-
Notifications
You must be signed in to change notification settings - Fork 37
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.
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
{ type: "module" } breaks under embroider #403
Comments
Thanks for the issue report! I had thought I had taken care of this already. It seems that
|
Everything works when |
But... just... sigh Can we please debug things like this instead of racing ahead with the wrong thing because it happens to work in the one test app here?
Using renovate doesn't address this problem because you can't control how quickly or slowly apps update their own dependency on |
yeah, but I did not believe this to be the wrong thing -- just a different thing -- based on my understanding of the below:
How would this happen? I verified the dist output, and test-waiters is not pulled in to the compiled output
right, the setting I used is to start out with all of |
It has nothing to do with whether it will get inlined into your package. When you put something in But in the case of test-waiters, that would defeat the point of using test-waiters. If the app uses test-waiters and calls The only way to guarantee that you're getting the same copy is a peer dependency. You might get the same copy without a peer dependency, but that optimization can be defeated by a bunch of things outside your control. Like, some other addon could change its dependencies and suddenly cause ember-resources's promises to stop being seen by the app's |
See explanation: #403 (comment) tldr: peerDeps are the only way to guarantee that modern packagers only load one version (specified by the consumer)
## [4.3.4](v4.3.3...v4.3.4) (2022-02-24) ### Bug Fixes * `@ember/test-waiters` must be a peer dependency ([01866c7](01866c7)), closes [/github.com//issues/403#issuecomment-1048404119](https://github.com//github.com/NullVoxPopuli/ember-resources/issues/403/issues/issuecomment-1048404119)
ember-resource 4.3.1 has
"type": "module"
in package.json. This means that imports without file extensions are only legal if the package you're importing from declaresexports
.But
@glimmer/tracking
does not declareexports
, so imports likeimport { getValue } from '@glimmer/tracking/primitives/cache';
are not legal.In general, you can't unilaterally adopt
"type": "module"
without waiting for your dependencies to be compatible with it.This probably only breaks under embroider, when webpack is doing the full module analysis. Under ember-auto-import there is still less strictness at the boundary between a v2 addon like this one as a v1 addon like
@glimmer/tracking
. This repo's embroider test coverage is turned off:ember-resources/.github/workflows/ci.yml
Lines 79 to 80 in 9c0fd4c
The text was updated successfully, but these errors were encountered: