-
Notifications
You must be signed in to change notification settings - Fork 4.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
Update and align babel dependencies version #65949
Conversation
Size Change: 0 B Total Size: 1.77 MB ℹ️ View Unchanged
|
44efca7
to
2ea5744
Compare
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Flaky tests detected in 5a61b80. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11243188325
|
package.json
Outdated
"@babel/core": "7.25.7", | ||
"@babel/plugin-syntax-jsx": "7.25.7", | ||
"@babel/plugin-transform-async-generator-functions": "7.25.7", | ||
"@babel/plugin-transform-export-namespace-from": "7.25.7", |
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.
Do you know why the name of the package has changed? Maybe it is natively supported now and it could be dropped now?
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.
Both of these plugins (async-generator-functions
and export-namespace-from
) are part of the default preset (@babel/preset-env
) we're using, so we should be able to remove them as explicit dependencies.
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.
The @babel/plugin-proposal-export-namespace-from
and @babel/plugin-proposal-async-generator-function
are deprecated, and their docs reference the new packages that replaces them:
- https://www.npmjs.com/package/@babel/plugin-proposal-export-namespace-from
- https://www.npmjs.com/package/@babel/plugin-proposal-async-generator-functions
Both of these plugins (
async-generator-functions
andexport-namespace-from
) are part of the default preset (@babel/preset-env
) we're using, so we should be able to remove them as explicit dependencies.
@tyxla just to make sure I understand, are you suggesting the removal of @babel/plugin-transform-async-generator-functions
and @babel/plugin-transform-export-namespace-from
?
Should we add an explicit dependency from @babel/preset-env
, then ?
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 thought this might be related to deprecated warnings when running npm ci
like this:
npm warn deprecated @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
But I still see them on this branch. Not a blocker but it would be great to track those all down.
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.
Many babel plugins are referenced directly in the native test config:
gutenberg/test/native/babel.config.js
Lines 6 to 16 in 6be7f8f
'@babel/plugin-proposal-async-generator-functions', | |
'@babel/plugin-transform-runtime', | |
[ | |
'react-native-platform-specific-extensions', | |
{ | |
extensions: [ 'css', 'scss', 'sass' ], | |
}, | |
], | |
'react-native-reanimated/plugin', | |
'@babel/plugin-proposal-export-namespace-from', | |
'@babel/plugin-transform-dynamic-import', |
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.
@tyxla just to make sure I understand, are you suggesting the removal of @babel/plugin-transform-async-generator-functions and @babel/plugin-transform-export-namespace-from?
Ideally, yes, unless there's a good reason to keep them.
Should we add an explicit dependency from @babel/preset-env, then ?
There should already be one in the @wordpress/babel-preset-default
project. Worth verifying if all different type of tests use that preset though.
Not a blocker but it would be great to track those all down.
I agree this isn't a blocker and can be done separately 👍
Would be nice to check if we can ditch them for the react native tests and wherever else we're specifically referencing them.
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.
Yes, it's definitely a follow-up. I mostly wanted to raise it as I had some blurry memories that they standardized the naming to make it easier to distinguish that the proposal becomes the standard, so moving forward, it should be part of the @babel/preset-env
. The Babel config for React Native might have some differences, so that needs to be double checked if it is still required there.
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 removed the @babel/plugin-transform-async-generator-functions
and @babel/plugin-transform-export-namespace-from dependencies
from package.json
, the package-lock.json
still includes them:
So, if all CI checks pass, I guess we're probably better off omitting them anyway, as they should be already included with the dependencies of @wordpress/babel-preset-default
(search for "packages/babel-preset-default/node_modules/@babel/preset-env" in the package-lock.json
file).
If folks are ok with the current state of the PR, I'd like to stop here and get it merged to limit the scope.
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.
Sounds good to me. Doing some final testing now.
"@babel/preset-env": "^7.16.0", | ||
"@babel/preset-typescript": "^7.16.0", | ||
"@babel/runtime": "^7.16.0", | ||
"@babel/core": "7.25.7", |
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.
Let's add the changelog entry for this package so it's clear that these dependencies were updated.
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.
Done in 4c83b1c
(#65949)
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.
Nice one, I left some minor notes but it's looking great. Thank you for working on it.
package-lock.json
Outdated
"peerDependencies": { | ||
"@babel/core": "^7.0.0-0" | ||
} | ||
}, | ||
"node_modules/@babel/plugin-proposal-nullish-coalescing-operator": { |
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.
Kind of a bummer that some of the modules are still using the old proposal versions when there are respective transform plugin alternatives (https://babeljs.io/docs/babel-plugin-transform-nullish-coalescing-operator).
Not much we can do here, this one seems to be a dependency of a few React Native related packages.
package.json
Outdated
"@babel/core": "7.25.7", | ||
"@babel/plugin-syntax-jsx": "7.25.7", | ||
"@babel/plugin-transform-async-generator-functions": "7.25.7", | ||
"@babel/plugin-transform-export-namespace-from": "7.25.7", |
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.
Both of these plugins (async-generator-functions
and export-namespace-from
) are part of the default preset (@babel/preset-env
) we're using, so we should be able to remove them as explicit dependencies.
…l/plugin-transform-export-namespace-from` as they are included with other dependencies
5a61b80
to
4c83b1c
Compare
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.
LGTM 👍 Thanks @ciampo 🚀
I didn't notice that before. All dependencies for Babel packages are listed as specific versions instead of using ranges. Was there any reason behind it, or was it overlooked? |
The reason was to avoid misalignment between versions across the package. Previously we had a mix of required versions, both ranges and specific versions, which led to multiple versions being installed. Having said that, feel free to change it to whatever strategy you think it's best, I definitely don't have much experience on this aspect of the project. |
We might hear back from folks once they run into issues when they start trying to upgrade packages. Historically, the biggest issue we encountered with the WP release process was when one of the dependencies was pinned so I’m a bit worried about consequences… |
I understand — feel free to change from |
* Update @babel/* dependencies to 7.25.7 * Swap old proposal-* plugin to transform-* plugins * Update non-official, babel-* dependencies * Update package-lock.json * Remove `@babel/plugin-transform-async-generator-functions` and `@babel/plugin-transform-export-namespace-from` as they are included with other dependencies * Update platform docs package-lock.json * CHANGELOG -- Co-authored-by: ciampo <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: sirreal <[email protected]>
What?
Update and align babel-related dependencies
Why?
As flagged in #65913 (comment), currently babel-related dependencies use different versions.
This PR aligns all babel-related dependencies to use the latest stable available version.
It also swaps a couple of deprecated babel plugins for the respective stable version.
How?
package.json
files manuallypackage-lock.json
Testing Instructions