-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
Move @babel/core
to peerDependencies
to resolve peer dependency warnings and errors
#452
Move @babel/core
to peerDependencies
to resolve peer dependency warnings and errors
#452
Conversation
I have 8 failures when I run the tests -- but those 8 failures also exist on |
You need a devDependency too. (This is an example of the confusing fallout from making v1 addons be their own test-app too, in a single package. The package.json has two meanings. It needs to do the work that a consuming app would do (put |
devDependency is there |
Oops, sorry, somehow I looked right at it and didn't see it. 😛 |
I think we can make the argument that this is a bugfix instead of breaking change because people were already experiencing peer dep warnings and/or failures depending on their package manager, so documenting the fact that you already needed this peer dep to avoid those issues is only a bug fix. |
This seems ready to go? |
It was pointed out to me that this really is probably breaking, because it's not the addition to |
I still think we should do it, we can make this ember-cli-babel 8. I know a traditional reason not to do a major release here was to save it for a babel major release, but this change means we're decoupled from the babel version anyway. |
Sounds good to me -- as long as we can specify in the release notes that |
Discussed with ember-cli core team and agreed to merge this, once we see it run CI. For some reason that hasn't happened yet. Investigating. Also, the plan to release this is tracked in #453 which I will fill out with more details. |
a239c07
to
83fa078
Compare
@NullVoxPopuli I added a test commit and dropped it again to trigger CI, hope you don't mind! |
@babel/core
to peerDependencies
to resolve peer dependency warnings and errors
CI is green, so I'm merging! |
…confusion
Following the advice here: ember-cli/ember-cli#9934 (comment)
The eventual perceived fix for users is that when using strict package managers (npm 8, yarn2+, pnpm, etc) is that when they are told to specify a missing peer,
@babel/core
, the@babel/core
they specify will be the one used by ember-cli-babel.@babel/core
will eventually be added to the app and addon blueprints in ember-cli/ember-cli#9934