embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
334 stars 136 forks source link

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

Open anehx opened 1 year ago

anehx commented 1 year ago

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..

Windvis commented 1 year ago

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 commented 1 year ago

We just fixed a very similar case in https://github.com/ef4/ember-auto-import/pull/530 by moving an error from build time to runtime.

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