From 909c6dacfde06b87fa22f2e8506c47124cf982b5 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Fri, 24 Mar 2023 14:08:41 -0400 Subject: [PATCH] Update Rollup to 3.x (#26442) ## Summary This PR: - Updates Rollup from 2.x to latest 3.x, and updates associated plugins - Updates deprecated / altered config settings in the Rollup plugin pipeline - Fixes some file extension and import issues related to use of ESM in `react-dom-webpack-server` - Removes a now-obsolete `strip-unused-imports` Rollup plugin - Fixes an _existing_ bug with the Rollup 2.x plugin pipeline on `main` that was causing parts of `DOMProperty.js` to get left out of the `react-dom-webpack-server` JS bundles, by adding a new plugin to tell Rollup to treat that file as if it as side effects This PR should be functionally identical to the other existing "Rollup 3 upgrade" PR at #26078 . I'm filing this as a near-duplicate because I'm ready to push this change through ASAP so that I can follow it up with a PR that adds sourcemap support, that PR's artifact diffing seems like it's possibly stuck and I want to compare the build results, and I've got this set up against latest `main`. This gets React's build setup updated to the latest Rollup version, which is generally a good practice, but also ensures that any further Rollup config tweaks can be done using the current Rollup docs as a reference. ## How did you test this change? - Made builds from the latest `main` - Updated Rollup package versions and cross-compared the changes I needed to make locally to get successful builds vs #26078 - Diffed the output folders between `main` and this PR, and confirmed that the bundle contents are identical (with the exception of version strings and the `react-dom-webpack-server` bundle fix re-adding missing `DOMProperty.js` content) --- package.json | 12 +- .../react-devtools-extensions/package.json | 4 - .../react-server-dom-webpack/package.json | 2 +- scripts/rollup/build.js | 33 +++- scripts/rollup/bundles.js | 2 +- .../rollup/plugins/strip-unused-imports.js | 29 --- scripts/rollup/wrappers.js | 16 +- yarn.lock | 171 ++++++++++-------- 8 files changed, 142 insertions(+), 127 deletions(-) delete mode 100644 scripts/rollup/plugins/strip-unused-imports.js diff --git a/package.json b/package.json index 964bdd936ea07..a23ea2c5145e4 100644 --- a/package.json +++ b/package.json @@ -36,10 +36,10 @@ "@babel/preset-flow": "^7.10.4", "@babel/preset-react": "^7.10.4", "@babel/traverse": "^7.11.0", - "@rollup/plugin-babel": "^5.3.1", - "@rollup/plugin-commonjs": "^22.0.1", - "@rollup/plugin-node-resolve": "^13.3.0", - "@rollup/plugin-replace": "^4.0.0", + "@rollup/plugin-babel": "^6.0.3", + "@rollup/plugin-commonjs": "^24.0.1", + "@rollup/plugin-node-resolve": "^15.0.1", + "@rollup/plugin-replace": "^5.0.2", "abortcontroller-polyfill": "^1.7.5", "art": "0.10.1", "babel-plugin-syntax-trailing-function-commas": "^6.5.0", @@ -89,9 +89,9 @@ "random-seed": "^0.3.0", "react-lifecycles-compat": "^3.0.4", "rimraf": "^3.0.0", - "rollup": "^2.76.0", + "rollup": "^3.17.1", "rollup-plugin-prettier": "^3.0.0", - "rollup-plugin-strip-banner": "^2.0.0", + "rollup-plugin-strip-banner": "^3.0.0", "semver": "^7.1.1", "targz": "^1.0.1", "through2": "^3.0.1", diff --git a/packages/react-devtools-extensions/package.json b/packages/react-devtools-extensions/package.json index ff41a5d3d32dd..e4044629f5dec 100644 --- a/packages/react-devtools-extensions/package.json +++ b/packages/react-devtools-extensions/package.json @@ -27,9 +27,6 @@ "@babel/plugin-transform-modules-commonjs": "^7.10.4", "@babel/plugin-transform-react-jsx-source": "^7.10.5", "@babel/preset-react": "^7.10.4", - "@rollup/plugin-babel": "^5.3.1", - "@rollup/plugin-commonjs": "^22.0.1", - "@rollup/plugin-node-resolve": "^13.3.0", "acorn-jsx": "^5.2.0", "archiver": "^3.0.0", "babel-core": "^7.0.0-bridge", @@ -58,7 +55,6 @@ "os-name": "^3.1.0", "parse-filepath": "^1.0.2", "raw-loader": "^3.1.0", - "rollup": "^2.76.0", "source-map-js": "^0.6.2", "sourcemap-codec": "^1.4.8", "style-loader": "^0.23.1", diff --git a/packages/react-server-dom-webpack/package.json b/packages/react-server-dom-webpack/package.json index 3f09719bb06e7..f32d8cdeefd7d 100644 --- a/packages/react-server-dom-webpack/package.json +++ b/packages/react-server-dom-webpack/package.json @@ -66,7 +66,7 @@ "./server.node.unbundled": "./server.node.unbundled.js", "./node-loader": "./esm/react-server-dom-webpack-node-loader.production.min.js", "./node-register": "./node-register.js", - "./src/*": "./src/*", + "./src/*": "./src/*.js", "./package.json": "./package.json" }, "main": "index.js", diff --git a/scripts/rollup/build.js b/scripts/rollup/build.js index bb33ea02c4e36..f7ce2760fb7bf 100644 --- a/scripts/rollup/build.js +++ b/scripts/rollup/build.js @@ -18,7 +18,6 @@ const Stats = require('./stats'); const Sync = require('./sync'); const sizes = require('./plugins/sizes-plugin'); const useForks = require('./plugins/use-forks-plugin'); -const stripUnusedImports = require('./plugins/strip-unused-imports'); const dynamicImports = require('./plugins/dynamic-imports'); const Packaging = require('./packaging'); const {asyncRimRaf} = require('./utils'); @@ -174,6 +173,31 @@ function getBabelConfig( return options; } +let getRollupInteropValue = id => { + // We're setting Rollup to assume that imports are ES modules unless otherwise specified. + // However, we also compile ES import syntax to `require()` using Babel. + // This causes Rollup to turn uses of `import SomeDefaultImport from 'some-module' into + // references to `SomeDefaultImport.default` due to CJS/ESM interop. + // Some CJS modules don't have a `.default` export, and the rewritten import is incorrect. + // Specifying `interop: 'default'` instead will have Rollup use the imported variable as-is, + // without adding a `.default` to the reference. + const modulesWithCommonJsExports = [ + 'JSResourceReferenceImpl', + 'error-stack-parser', + 'art/core/transform', + 'art/modes/current', + 'art/modes/fast-noSideEffects', + 'art/modes/svg', + ]; + + if (modulesWithCommonJsExports.includes(id)) { + return 'default'; + } + + // For all other modules, handle imports without any import helper utils + return 'esModule'; +}; + function getRollupOutputOptions( outputPath, format, @@ -188,7 +212,7 @@ function getRollupOutputOptions( format, globals, freeze: !isProduction, - interop: false, + interop: getRollupInteropValue, name: globalName, sourcemap: false, esModule: false, @@ -420,9 +444,6 @@ function getPlugins( assume_function_wrapper: !isUMDBundle, renaming: !shouldStayReadable, }), - // HACK to work around the fact that Rollup isn't removing unused, pure-module imports. - // Note that this plugin must be called after closure applies DCE. - isProduction && stripUnusedImports(pureExternalModules), // Add the whitespace back if necessary. shouldStayReadable && prettier({ @@ -616,7 +637,7 @@ async function createBundle(bundle, bundleType) { output: { externalLiveBindings: false, freeze: false, - interop: false, + interop: getRollupInteropValue, esModule: false, }, }; diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index e4ff1cf8bf0f7..f29fe5e17a433 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -442,7 +442,7 @@ const bundles = [ { bundleTypes: [NODE_ES2015], moduleType: RENDERER_UTILS, - entry: 'react-server-dom-webpack/src/ReactFlightWebpackNodeRegister.js', + entry: 'react-server-dom-webpack/src/ReactFlightWebpackNodeRegister', name: 'react-server-dom-webpack-node-register', global: 'ReactFlightWebpackNodeRegister', minifyWithProdErrorCodes: false, diff --git a/scripts/rollup/plugins/strip-unused-imports.js b/scripts/rollup/plugins/strip-unused-imports.js deleted file mode 100644 index a8b74142d811f..0000000000000 --- a/scripts/rollup/plugins/strip-unused-imports.js +++ /dev/null @@ -1,29 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ -'use strict'; - -module.exports = function stripUnusedImports(pureExternalModules) { - return { - name: 'scripts/rollup/plugins/strip-unused-imports', - renderChunk(code) { - pureExternalModules.forEach(module => { - // Ideally this would use a negative lookbehind: (?