Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

dependencySatisfies doesn't work properly with optional peer dependencies #1266

Open
anehx opened this issue Oct 20, 2022 · 2 comments · May be fixed by embroider-build/ember-auto-import#546
Open

dependencySatisfies doesn't work properly with optional peer dependencies #1266

anehx opened this issue Oct 20, 2022 · 2 comments · May be fixed by embroider-build/ember-auto-import#546

Comments

@anehx
Copy link

anehx commented Oct 20, 2022

Hi

I'm having problems with the dependencySatisfies macro. I have an addon that defines a peer dependency as optional. The addon code then uses dependencySatisfies to check whether I need to implement a function (including an import of that dependency).

However, if I use that addon in an app that does not have that dependency, the macro still resolves to true which causes an error of ember-auto-import:

ember-simple-auth-oidc tried to import "@apollo/client" in "ember-simple-auth-oidc/apollo.js" but the package was not resolvable

@apollo/client is not present in the dependencies, I verfied that with yarn why @apollo/client.

Here are the relevant code snippets of the addon:

Could you take a look at that? Let me know if I can do something to help..
This was referenced Oct 20, 2022
Open
Merged
anehx added a commit to anehx/ember-simple-auth-oidc that referenced this issue Oct 20, 2022
@anehx
fix(apollo): make apollo client a dependency
c917446 
This is necessary since embroider doesn't handle optional dependencies
properly as reported here: embroider-build/embroider#1266

This commit makes `@apollo/client` temporarly a non optional dependency
to fix the current issue we have in apps without that dependency.
However, we need to revert this as soon as the issue above is resolved
since we don't want to enforce this dependency.

Related to adfinis#597
anehx added a commit to adfinis/ember-simple-auth-oidc that referenced this issue Oct 20, 2022
@anehx
fix(apollo): make apollo client a dependency
31aca2e 
This is necessary since embroider doesn't handle optional dependencies
properly as reported here: embroider-build/embroider#1266

This commit makes `@apollo/client` temporarly a non optional dependency
to fix the current issue we have in apps without that dependency.
However, we need to revert this as soon as the issue above is resolved
since we don't want to enforce this dependency.

Related to #597
@Windvis
Copy link
Collaborator

Windvis commented Oct 20, 2022 •
edited
Loading

I believe this is an ember-auto-import issue. The error is thrown here.

dependencySatisfies does return false correctly but I think ember-auto-import simply analyzes all imports and doesn't check for optional peerDependencies yet. I'm not familiar enough with the code (and intended workings) to know what the correct solution is but this is at least a first pointer for the real issue 😄.

@ef4
Copy link
Contributor

ef4 commented Oct 20, 2022

We just fixed a very similar case in embroider-build/ember-auto-import#530 by moving an error from build time to runtime.

It would make sense to do the same for importSync for the same reason.

@richgt richgt linked a pull request Oct 21, 2022 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove built-time error for root-path module not found richgt/ember-auto-import
3 participants
@ef4 @Windvis @anehx