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 Rollup and related plugins to their most recent versions #24916
base: main
Are you sure you want to change the base?
Update Rollup and related plugins to their most recent versions #24916
Conversation
…'9.3.4'' to '22.0.1'
…mp from '2.1.1'' to '13.3.0'
….2.0'' to '4.0.0'
…reeshake.moduleSideEffects" option should be used instead."
…o false. It is recommended to set this option to `true`, as the next major version will default this option to `true`."
Added a few comments/pointers to help explain some of the changes made + point to relevent docs/changelog/etc.
It's definitely also worth looking at the commits, as I tried to keep all of the changes neatly isolated, so it may help tell the story of the changes better:
| @@ -156,6 +156,7 @@ function getBabelConfig( | |||
| configFile: false, | |||
| presets: [], | |||
| plugins: [...babelPlugins], | |||
| babelHelpers: 'bundled', | |||
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.
Added as per advice in https://github.com/rollup/plugins/blob/master/packages/babel/CHANGELOG.md#500
Removed
externalHelpers&runtimeHelpersoptions. There is now a singlebabelHelpersoption which can take one of'bundled','inline','runtime'and'external'as a value. The default is'bundled'which matches 4.x behavior, but it is recommended to configure this option explicitly.
| skip: externals, | ||
| // skip: externals, // TODO: options.skip was removed in @rollup/plugin-node-resolve 3.0.0 |
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.
TODO: options.skip was removed in @rollup/plugin-node-resolve 3.0.0
Is there a different value we should be using here now in it's place? (external?)
| __DEV__: isProduction ? 'false' : 'true', | ||
| __PROFILE__: isProfiling || !isProduction ? 'true' : 'false', | ||
| __UMD__: isUMDBundle ? 'true' : 'false', | ||
| 'process.env.NODE_ENV': isProduction ? "'production'" : "'development'", | ||
| __EXPERIMENTAL__, | ||
| // Enable forked reconciler. | ||
| // NOTE: I did not put much thought into how to configure this. | ||
| __VARIANT__: bundle.enableNewReconciler === true, | ||
| preventAssignment: true, | ||
| values: { | ||
| __DEV__: isProduction ? 'false' : 'true', | ||
| __PROFILE__: isProfiling || !isProduction ? 'true' : 'false', | ||
| __UMD__: isUMDBundle ? 'true' : 'false', | ||
| 'process.env.NODE_ENV': isProduction ? "'production'" : "'development'", | ||
| __EXPERIMENTAL__, | ||
| // Enable forked reconciler. | ||
| // NOTE: I did not put much thought into how to configure this. | ||
| __VARIANT__: bundle.enableNewReconciler === true, | ||
| } |
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.
| @@ -540,7 +544,7 @@ async function createBundle(bundle, bundleType) { | |||
| const rollupConfig = { | |||
| input: resolvedEntry, | |||
| treeshake: { | |||
| pureExternalModules, | |||
| moduleSideEffects: pureExternalModules, | |||
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.
- https://github.com/rollup/rollup/blob/master/CHANGELOG.md#200
-
The
treeshake.pureExternalModulesoption will now show a deprecation warning when used: usetreeshake.moduleSideEffects: 'no-external'instead
-
- https://rollupjs.org/guide/en/#treeshakepureexternalmodules
| @@ -323,7 +323,11 @@ async function bundle() { | |||
| plugins: [ | |||
| rollupResolve(), | |||
| commonjs(), | |||
| babel({presets: ['@babel/preset-react'], sourceMap: true}), | |||
| babel({ | |||
| babelHelpers: 'bundled', | |||
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.
- https://github.com/rollup/plugins/blob/master/packages/babel/CHANGELOG.md#500
-
Removed
externalHelpers&runtimeHelpersoptions. There is now a singlebabelHelpersoption which can take one of'bundled','inline','runtime'and'external'as a value. The default is'bundled'which matches4.xbehavior, but it is recommended to configure this option explicitly.
-
- https://github.com/rollup/plugins/tree/master/packages/babel#babelhelpers
|
The build is currently failing due to the following error: I wonder if there could be some |
Summary
Update Rollup and related plugins to their most recent versions + resolve any breaking changes/deprecations/etc along the way. I made each change piece by piece, so the commit history tells a pretty good story of what was changed where/how/why.
fixes #24894
For the full deepdive/context, see:
The inspiration for this came from @jasonwilliams 's PR for attempting to add sourcemap output support to React's builds:
But I figured that it would be useful to minimise the scope of changes in that PR, and to modernise the build tooling along the way.
If any of these updates rely on a node version later than
10.x, then the following PR may have to land first, otherwise things might break on AppVeyor:How did you test this change?
See some of my testing notes RE: figuring compatible package versions + their associated changes in #24894 (comment) Of particular note is the difference in the
react.development.jsoutput between Rollup1.xand rollup2.x:I ran the tests with
yarn test --prod, and opened a separate issue for the few tests that seem to fail on Node16.x:I'm not 100% certain if this is the most thorough/complete way to test things, so I am definitely open to suggestions on how to improve confidence in these changes.