-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[ENHANCEMENT] Update ember-cli-babel
to v8 in app
and addon
blueprints
#10141
[ENHANCEMENT] Update ember-cli-babel
to v8 in app
and addon
blueprints
#10141
Conversation
embroider-build/embroider#1343 should fix the failing Embroider runs. |
25c04f1
to
35bef7d
Compare
35bef7d
to
d7db292
Compare
@@ -56,6 +56,11 @@ module.exports = { | |||
contents.dependencies['ember-cli-babel'] = contents.devDependencies['ember-cli-babel']; | |||
delete contents.devDependencies['ember-cli-babel']; | |||
|
|||
// Addons must bring in their own version of `@babel/core` | |||
// when using `ember-cli-babel` >= v8. | |||
contents.dependencies['@babel/core'] = contents.devDependencies['@babel/core']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why a dependencies
entry?
shouldn't this be a peer(+dev), letting the app control the version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bertdeblock provided some context here: https://github.com/babel/ember-cli-babel/blob/master/UPGRADING.md#upgrade-path-for-addons
tl;dr: v1 addons already don't care what the app is doing, as they compile themselves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added that link as well over in #10349.
|
Mainly opening to see how CI will respond.
Closes #9933.
Closes #9934.
Closes emberjs/ember-cli-babel#453.