Skip to content
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

Upgrade to modern version of Rollup and related plugins #24894

Closed
0xdevalias opened this issue Jul 12, 2022 · 2 comments · Fixed by #24916
Closed

Upgrade to modern version of Rollup and related plugins #24894

0xdevalias opened this issue Jul 12, 2022 · 2 comments · Fixed by #24916

Comments

@0xdevalias
Copy link
Contributor

0xdevalias commented Jul 12, 2022

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.


There have been quite a few version releases for Rollup and the plugins that are currently used, as well as a number of the official Rollup plugins changing package name to use the @rollup/* package prefix.

Looking at package.json we can see all of the current Rollup packages:

// ..snip..
    "rollup": "^1.19.4",
    "rollup-plugin-babel": "^4.0.1",
    "rollup-plugin-commonjs": "^9.3.4",
    "rollup-plugin-node-resolve": "^2.1.1",
    "rollup-plugin-prettier": "^0.6.0",
    "rollup-plugin-replace": "^2.2.0",
    "rollup-plugin-strip-banner": "^0.2.0",
// ..snip..

And using grep, we can see some additional usages under packages/:

⇒  grep -r 'rollup' package.json packages/*/package.json
package.json:    "rollup": "^1.19.4",
package.json:    "rollup-plugin-babel": "^4.0.1",
package.json:    "rollup-plugin-commonjs": "^9.3.4",
package.json:    "rollup-plugin-node-resolve": "^2.1.1",
package.json:    "rollup-plugin-prettier": "^0.6.0",
package.json:    "rollup-plugin-replace": "^2.2.0",
package.json:    "rollup-plugin-strip-banner": "^0.2.0",
package.json:    "build": "node ./scripts/rollup/build.js",
package.json:    "build-combined": "node ./scripts/rollup/build-all-release-channels.js",
package.json:    "lint-build": "node ./scripts/rollup/validate/index.js",
packages/react-devtools-extensions/package.json:    "rollup": "^1.19.4",
packages/react-devtools-extensions/package.json:    "rollup-plugin-babel": "^4.0.1",
packages/react-devtools-extensions/package.json:    "rollup-plugin-commonjs": "^9.3.4",
packages/react-devtools-extensions/package.json:    "rollup-plugin-node-resolve": "^2.1.1",

The following is an attempt to capture all of the relevant information about package name changes, new versions, breaking changes, etc for these packages:

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:


My plan/intent is to work through all of this and create a PR updating Rollup and the related plugins; but I wanted to create this issue in the meantime to capture what I have done so far, and also check whether there would be any issues/concerns with a PR like this being made.

Edit: PR opened in:

@0xdevalias
Copy link
Contributor Author

0xdevalias commented Jul 12, 2022

It seems yarn 1.x's remove doesn't update the yarn.lock file particularly well.. is there a best practice way for doing this, or is it just a manual sort of thing to clean up?

eg. yarn remove --force --ignore-workspace-root-check rollup-plugin-babel only updated package.json, not yarn.lock


Edit: I wonder if this is because it's used in other packages as well?

⇒  grep -r 'rollup-plugin-babel' packages
packages/react-devtools-timeline/node_modules/json5/package.json:    "rollup-plugin-babel": "^3.0.3",
packages/react-devtools-extensions/package.json:    "rollup-plugin-babel": "^4.0.1",
packages/react-devtools-shared/src/node_modules/react-window/package.json:    "rollup-plugin-babel": "^4.3.2",
packages/react-devtools-shared/src/hooks/__tests__/updateMockSourceMaps.js:const babel = require('rollup-plugin-babel');
⇒  yarn add --force --dev --ignore-workspace-root-check @rollup/plugin-babel
yarn add v1.22.19
[1/4] 🔍  Resolving packages...
warning workspace-aggregator-06b1766b-e8f5-406b-87e7-0089d7a59d18 > react-devtools-extensions > rollup-plugin-babel@4.4.0: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-babel.
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...

...snip: a whole bunch of peer dependency warnings...

[4/4] 🔨  Rebuilding all packages...
success Saved lockfile.
success Saved 3 new dependencies.
info Direct dependencies
├─ @rollup/plugin-babel@5.3.1
└─ react-is@18.2.0
info All dependencies
├─ @rollup/plugin-babel@5.3.1
├─ react-is@18.2.0
└─ rollup-plugin-babel@4.4.0

Edit 2: I tried using yarn's workspace command to manipulate the other packages:

But then I ran into an error about "Failed to install dependencies in workspace: expected workspace package to exist". After some googling, that lead me to the following issues, and the various proposed solutions within:

@0xdevalias
Copy link
Contributor Author

Playing around with things, this seems to be a working set of upgraded rollup version/plugins that still seems to produce the same build output for build/node_modules/react/umd/react.development.js.

Note that:

diff --git a/package.json b/package.json
index 2e8445521..51e284ff6 100644
--- a/package.json
+++ b/package.json
@@ -36,6 +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": "^10.0.0",
+    "@rollup/plugin-replace": "^4.0.0",
     "abort-controller": "^3.0.0",
     "art": "0.10.1",
     "babel-eslint": "^10.0.3",
@@ -83,13 +87,9 @@
     "random-seed": "^0.3.0",
     "react-lifecycles-compat": "^3.0.4",
     "rimraf": "^3.0.0",
-    "rollup": "^1.19.4",
-    "rollup-plugin-babel": "^4.0.1",
-    "rollup-plugin-commonjs": "^9.3.4",
-    "rollup-plugin-node-resolve": "^2.1.1",
-    "rollup-plugin-prettier": "^0.6.0",
-    "rollup-plugin-replace": "^2.2.0",
-    "rollup-plugin-strip-banner": "^0.2.0",
+    "rollup": "^1.32.1",
+    "rollup-plugin-prettier": "^2.2.2",
+    "rollup-plugin-strip-banner": "^2.0.0",
     "semver": "^7.1.1",
     "targz": "^1.0.1",
     "through2": "^3.0.1",

Moving to a 2.x version of Rollup seems to change the output of build/node_modules/react/umd/react.development.js. While it may be insignificant, I figured it was worth calling out the differences here for posterity (partial diff included below):

Details
diff --git a/build-currentMain/node_modules/react/umd/react.development.js b/build/node_modules/react/umd/react.development.js
index 74fb8652e..50a773692 100644
--- a/build-currentMain/node_modules/react/umd/react.development.js
+++ b/build/node_modules/react/umd/react.development.js
@@ -10,8 +10,8 @@
 (function (global, factory) {
   typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
   typeof define === 'function' && define.amd ? define(['exports'], factory) :
-  (global = global || self, factory(global.React = {}));
-}(this, (function (exports) { 'use strict';
+  (global = typeof globalThis !== 'undefined' ? globalThis : global || self, factory(global.React = {}));
+})(this, (function (exports) { 'use strict';
 
   // TODO: this is special because it gets imported during build.
   //
@@ -40,9 +40,12 @@
   var REACT_SUSPENSE_LIST_TYPE = Symbol.for('react.suspense_list');
   var REACT_MEMO_TYPE = Symbol.for('react.memo');
   var REACT_LAZY_TYPE = Symbol.for('react.lazy');
+  var REACT_SCOPE_TYPE = Symbol.for('react.scope');
   var REACT_DEBUG_TRACING_MODE_TYPE = Symbol.for('react.debug_trace_mode');
   var REACT_OFFSCREEN_TYPE = Symbol.for('react.offscreen');
+  var REACT_LEGACY_HIDDEN_TYPE = Symbol.for('react.legacy_hidden');
   var REACT_CACHE_TYPE = Symbol.for('react.cache');
+  var REACT_TRACING_MARKER_TYPE = Symbol.for('react.tracing_marker');
   var REACT_SERVER_CONTEXT_DEFAULT_VALUE_NOT_LOADED = Symbol.for('react.default_value');
   var MAYBE_ITERATOR_SYMBOL = Symbol.iterator;
   var FAUX_ITERATOR_SYMBOL = '@@iterator';
@@ -63,13 +66,14 @@
   /**
    * Keeps track of the current dispatcher.
    */
-  var ReactCurrentDispatcher = {
+  var ReactCurrentDispatcher$1 = {
     /**
      * @internal
      * @type {ReactComponent}
      */
     current: null
   };
+  var ReactCurrentDispatcher$2 = ReactCurrentDispatcher$1;
 
   /**
    * Keeps track of the current batch's configuration such as how long an update
@@ -78,6 +82,7 @@
   var ReactCurrentBatchConfig = {
     transition: null
   };
+  var ReactCurrentBatchConfig$1 = ReactCurrentBatchConfig;
 
   var ReactCurrentActQueue = {
     current: null,
@@ -85,6 +90,7 @@
     isBatchingLegacy: false,
     didScheduleLegacyUpdate: false
   };
+  var ReactCurrentActQueue$1 = ReactCurrentActQueue;
 
   /**
    * Keeps track of the current owner.
@@ -99,8 +105,9 @@
      */
     current: null
   };
+  var ReactCurrentOwner$1 = ReactCurrentOwner;
 
-  var ReactDebugCurrentFrame = {};
+  var ReactDebugCurrentFrame$1 = {};
   var currentExtraStackFrame = null;
   function setExtraStackFrame(stack) {
     {
@@ -109,16 +116,16 @@
   }
 
   {
-    ReactDebugCurrentFrame.setExtraStackFrame = function (stack) {
+    ReactDebugCurrentFrame$1.setExtraStackFrame = function (stack) {
       {
         currentExtraStackFrame = stack;
       }
     }; // Stack implementation injected by the current renderer.
 
 
-    ReactDebugCurrentFrame.getCurrentStack = null;
+    ReactDebugCurrentFrame$1.getCurrentStack = null;
 
-    ReactDebugCurrentFrame.getStackAddendum = function () {
+    ReactDebugCurrentFrame$1.getStackAddendum = function () {
       var stack = ''; // Add an extra top frame while an element is being validated
 
       if (currentExtraStackFrame) {
@@ -126,7 +133,7 @@
       } // Delegate to the injected renderer-specific implementation
 
 
-      var impl = ReactDebugCurrentFrame.getCurrentStack;
+      var impl = ReactDebugCurrentFrame$1.getCurrentStack;
 
       if (impl) {
         stack += impl() || '';
@@ -136,9 +143,12 @@
     };
   }
 
+  var ReactDebugCurrentFrame$2 = ReactDebugCurrentFrame$1;
+
   // -----------------------------------------------------------------------------
 
   var enableScopeAPI = false; // Experimental Create Event Handle API.
+  var enableCacheElement = true;
   var enableTransitionTracing = false; // No known bugs, but needs performance testing
 
   var enableLegacyHidden = false; // Enables unstable_avoidThisFallback feature in Fiber
@@ -146,24 +156,28 @@
   // issues in DEV builds.
 
   var enableDebugTracing = false; // Track which Fiber(s) schedule render work.
+  var enableServerContext = true; // Internal only.
 
-  var ContextRegistry = {};
+  var ContextRegistry$1 = {};
 
-  var ReactSharedInternals = {
-    ReactCurrentDispatcher: ReactCurrentDispatcher,
-    ReactCurrentBatchConfig: ReactCurrentBatchConfig,
-    ReactCurrentOwner: ReactCurrentOwner
+  var ReactSharedInternals$2 = {
+    ReactCurrentDispatcher: ReactCurrentDispatcher$2,
+    ReactCurrentBatchConfig: ReactCurrentBatchConfig$1,
+    ReactCurrentOwner: ReactCurrentOwner$1
   };
 
   {
-    ReactSharedInternals.ReactDebugCurrentFrame = ReactDebugCurrentFrame;
-    ReactSharedInternals.ReactCurrentActQueue = ReactCurrentActQueue;
+    ReactSharedInternals$2.ReactDebugCurrentFrame = ReactDebugCurrentFrame$2;
+    ReactSharedInternals$2.ReactCurrentActQueue = ReactCurrentActQueue$1;
   }
 
   {
-    ReactSharedInternals.ContextRegistry = ContextRegistry;
+    ReactSharedInternals$2.ContextRegistry = ContextRegistry$1;
   }
 
+  var ReactSharedInternals$3 = ReactSharedInternals$2;
+
+  var suppressWarning = false;

//..snip..

I'm not currently sure if these changes are significant/change the semantics of the output, or are just a different way of the bundler working, but I did notice that it increases the output file sizes a bit (most dramatically in react.development.js):

⇒  yarn build react/index,react-dom/index --type=UMD
yarn run v1.22.19
$ node ./scripts/rollup/build.js react/index,react-dom/index --type=UMD

//..snip..

┌─────────────────────────────────────────────┬───────────┬──────────────┬───────┬───────────┬──────────────┬───────┐
│ Bundle                                      │ Prev Size │ Current Size │ Diff  │ Prev Gzip │ Current Gzip │ Diff  │
├─────────────────────────────────────────────┼───────────┼──────────────┼───────┼───────────┼──────────────┼───────┤
│ react.development.js  (UMD_DEV)             │ 111.96 KB │ 117.59 KB    │ +5.0% │ 28.82 KB  │ 30.35 KB     │ +5.3% │
├─────────────────────────────────────────────┼───────────┼──────────────┼───────┼───────────┼──────────────┼───────┤
│ react.production.min.js  (UMD_PROD)         │ 11.45 KB  │ 11.51 KB     │ +0.5% │ 4.42 KB   │ 4.43 KB      │ +0.4% │
├─────────────────────────────────────────────┼───────────┼──────────────┼───────┼───────────┼──────────────┼───────┤
│ react.profiling.min.js  (UMD_PROFILING)     │ 11.45 KB  │ 11.51 KB     │ +0.5% │ 4.42 KB   │ 4.44 KB      │ +0.4% │
├─────────────────────────────────────────────┼───────────┼──────────────┼───────┼───────────┼──────────────┼───────┤
│ react-dom.development.js  (UMD_DEV)         │ 1.07 MB   │ 1.07 MB      │ +0.1% │ 236.54 KB │ 236.89 KB    │ +0.1% │
├─────────────────────────────────────────────┼───────────┼──────────────┼───────┼───────────┼──────────────┼───────┤
│ react-dom.production.min.js  (UMD_PROD)     │ 135.13 KB │ 135.91 KB    │ +0.6% │ 43.93 KB  │ 44.21 KB     │ +0.6% │
├─────────────────────────────────────────────┼───────────┼──────────────┼───────┼───────────┼──────────────┼───────┤
│ react-dom.profiling.min.js  (UMD_PROFILING) │ 143.66 KB │ 144.44 KB    │ +0.5% │ 46.14 KB  │ 46.41 KB     │ +0.6% │
└─────────────────────────────────────────────┴───────────┴──────────────┴───────┴───────────┴──────────────┴───────┘
✨  Done in 36.89s.

This was using the following versions for Rollup and related plugins, which I believe are the latest versions of all of them:

diff --git a/package.json b/package.json
index 2e8445521..eb68a8d60 100644
--- a/package.json
+++ b/package.json
@@ -36,6 +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",
     "abort-controller": "^3.0.0",
     "art": "0.10.1",
     "babel-eslint": "^10.0.3",
@@ -83,13 +87,9 @@
     "random-seed": "^0.3.0",
     "react-lifecycles-compat": "^3.0.4",
     "rimraf": "^3.0.0",
-    "rollup": "^1.19.4",
-    "rollup-plugin-babel": "^4.0.1",
-    "rollup-plugin-commonjs": "^9.3.4",
-    "rollup-plugin-node-resolve": "^2.1.1",
-    "rollup-plugin-prettier": "^0.6.0",
-    "rollup-plugin-replace": "^2.2.0",
-    "rollup-plugin-strip-banner": "^0.2.0",
+    "rollup": "^2.76.0",
+    "rollup-plugin-prettier": "^2.2.2",
+    "rollup-plugin-strip-banner": "^2.0.0",
     "semver": "^7.1.1",
     "targz": "^1.0.1",
     "through2": "^3.0.1",

sebmarkbage added a commit that referenced this issue Feb 20, 2023
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:

- #24894

The inspiration for this came from @jasonwilliams 's PR for attempting
to add sourcemap output support to React's builds:

- #20186
  - #21946

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:

- #24891
  - #24892

Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>
github-actions bot pushed a commit that referenced this issue Feb 20, 2023
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:

- #24894

The inspiration for this came from @jasonwilliams 's PR for attempting
to add sourcemap output support to React's builds:

- #20186
  - #21946

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:

- #24891
  - #24892

Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>

DiffTrain build for [6b6d061](6b6d061)
[View git log for this commit](https://github.com/facebook/react/commits/6b6d0617eff48860c5b4e3e79c74cbd3312cf45a)
jerrydev0927 added a commit to jerrydev0927/react that referenced this issue Jan 5, 2024
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 facebook/react#24894

For the full deepdive/context, see:

- facebook/react#24894

The inspiration for this came from @jasonwilliams 's PR for attempting
to add sourcemap output support to React's builds:

- facebook/react#20186
  - facebook/react#21946

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:

- facebook/react#24891
  - facebook/react#24892

Co-authored-by: Sebastian Markbage <sebastian@calyptus.eu>

DiffTrain build for [6b6d0617eff48860c5b4e3e79c74cbd3312cf45a](facebook/react@6b6d061)
[View git log for this commit](https://github.com/facebook/react/commits/6b6d0617eff48860c5b4e3e79c74cbd3312cf45a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant