Convert current build system to Rollup and adopt flat bundles (#9327) * WIP * fbjs support * WIP * dev/prod mode WIP * More WIP * builds a cjs bundle * adding forwarding modules * more progress on forwarding modules and FB config - #11
Conversation
…9327) * WIP * fbjs support * WIP * dev/prod mode WIP * More WIP * builds a cjs bundle * adding forwarding modules * more progress on forwarding modules and FB config * improved how certain modules get inlined for fb and cjs * more forwarding modules * added comments to the module aliasing code * made ReactPerf and ReactTestUtils bundle again * Use -core suffix for all bundles This makes it easier to override things in www. * Add a lazy shim for ReactPerf This prevents a circular dependency between ReactGKJSModule and ReactDOM * Fix forwarding module for ReactCurrentOwner * Revert "Add a lazy shim for ReactPerf" This reverts commit 723b402. * Rename -core suffix to -fb for clarity * Change forwarding modules to import from -fb This is another, more direct fix for ReactPerf circular dependency * should fix fb and cjs bundles for ReactCurrentOwner * added provides module for ReactCurrentOwner * should improve console output * fixed typo with argument passing on functon call * Revert "should improve console output" This breaks the FB bundles. This reverts commit 65f11ee. * Work around internal FB transform require() issue * moved ReactInstanceMap out of React and into ReactDOM and ReactDOMFiber * Expose more internal modules to www * Add missing modules to Stack ReactDOM to fix UFI * Fix onlyChild module * improved the build tool * Add a rollup npm script * Rename ReactDOM-fb to ReactDOMStack-fb * Fix circular dependencies now that ReactDOM-fb is a GK switch * Revert "Work around internal FB transform require() issue" This reverts commit 0a50b6a. * Bump rollup-plugin-commonjs to include a fix for rollup/rollup-plugin-commonjs#176 * Add more forwarding modules that are used on www * Add even more forwarding modules that are used on www * Add DOMProperty to hidden exports * Externalize feature flags This lets www specify them dynamically. * Remove forwarding modules with implementations Instead I'm adding them to react-fb in my diff. * Add all injection necessary for error logging * Add missing forwarding module (oops) * Add ReactART builds * Add ReactDOMServer bundle * Fix UMD build of ReactDOMFiber * Work in progress: start adding ReactNative bundle * tidied up the options for bundles, so they can define what types they output and exclude * Add a working RN build * further improved and tidied up build process * improved how bundles are built by exposing externals and making the process less "magical", also tidied up code and added more comments * better handling of bundling ReactCurrentOwner and accessing it from renderer modules * added NODE_DEV and NODE_PROD * added NPM package creation and copying into build chain * Improved UMD bundles, added better fixture testing and doc plus prod builds * updated internal modules (WIP) * removed all react/lib/* dependencies from appearing in bundles created on build * added react-test-renderer bundles * renamed bundles and paths * fixed fixture path changes * added extract-errors support * added extractErrors warning * moved shims to shims directory in rollup scripts * changed pathing to use build rather than build/rollup * updated release doc to reflect some rollup changes * Updated ReactNative findNodeHandle() to handle number case (react#9238) * Add dynamic injection to ReactErrorUtils (react#9246) * Fix ReactErrorUtils injection (react#9247) * Fix Haste name * Move files around * More descriptive filenames * Add missing ReactErrorUtils shim * Tweak reactComponentExpect to make it standalone-ish in www * Unflowify shims * facebook-www shims now get copied over correctly to build * removed unnecessary resolve * building facebook-www/build is now all sync to prevent IO issues plus handles extra facebook-www src assets * removed react-native-renderer package and made build make a react-native build dir instead * 😭😭😭 * Add more SSR unit tests for elements and children. (react#9221) * Adding more SSR unit tests for elements and children. * Some of my SSR tests were testing for react-text and react-empty elements that no longer exist in Fiber. Fixed the tests so that they expect correct markup in Fiber. * Tweaked some test names after @gaearon review comment react#9221 (comment) . Also realized that one of the tests was essentially a direct copy of another, so deleted it. * Responding to code review react#9221 (review) . Thanks @spicyj! * ReactElementValidator uses temporary ReactNative View propTypes getter (react#9256) * Updating packages for 16.0.0-alpha.6 release * Revert "😭😭😭" This reverts commit 7dba33b. * Work around Jest issue with CurrentOwner shared state in www * updated error codes * splits FB into FB_DEV and FB_PROD * Remove deps on specific builds from shims * should no longer mangle FB_PROD output * Added init() dev block to ReactTestUtils * added shims for DEV only code so it does not get included in prod bundles * added a __DEV__ wrapping code to FB_DEV * added __DEV__ flag behind a footer/header * Use right haste names * keeps comments in prod * added external babel helpers plugin * fixed fixtures and updated cjs/umd paths * Fixes Jest so it run tests correctly * fixed an issue with stubbed modules not properly being replaced due to greedy replacement * added a WIP solution for ReactCurrentOwner on FB DEV * adds a FB_TEST bundle * allows both ReactCurrentOwner and react/lib/ReactCurrentOwner * adds -test to provides module name * Remove TEST env * Ensure requires stay at the top * added basic mangle support (disbaled by default) * per bundle property mangling added * moved around plugin order to try and fix deadcode requires as per rollup/rollup#855 * Fix flow issues * removed gulp and grunt and moved tasks to standalone node script * configured circleci to use new paths * Fix lint * removed gulp-extract-errors * added test_build.sh back in * added missing newline to flow.js * fixed test coverage command * changed permissions on test_build.sh * fixed test_html_generations.sh * temp removed html render test * removed the warning output from test_build, the build should do this instead * fixed test_build * fixed broken npm script * Remove unused ViewportMetrics shim * better error output * updated circleci to node 7 for async/await * Fixes * removed coverage test from circleci run * circleci run tets * removed build from circlci * made a dedicated jest script in a new process * moved order around of circlci tasks * changing path to jest in more circleci tests * re-enabled code coverage * Add file header to prod bundles * Remove react-dom/server.js (WIP: decide on the plan) * Only UMD bundles need version header * Merge with master * disabled const evaluation by uglify for <script></script> string literal * deal with ART modules for UMD bundles * improved how bundle output gets printed * fixed filesize difference reporting * added filesize dep * Update yarn lockfile for some reason * now compares against the last run branch built on * added react-dom-server * removed un-needed comment * results only get saved on full builds * moved the rollup sized plugin into a plugins directory * added a missing commonjs() * fixed missing ignore * Hack around to fix RN bundle * Partially fix RN bundles * added react-art bundle and a fixture for it * Point UMD bundle to Fiber and add EventPluginHub to exported internals * Make it build on Node 4 * fixed eslint error with resolve being defined in outer scope * Tweak how build results are calculated and stored * Tweak fixtures build to work on Node 4 * Include LICENSE/PATENTS and fix up package.json files * Add Node bundle for react-test-renderer * Revert "Hack around to fix RN bundle" We'll do this later. This reverts commit 59445a6. * Revert more RN changes We'll do them separately later * Revert more unintentional changes * Revert changes to error codes * Add accidentally deleted RN externals * added RN_DEV/RN_PROD bundles * fixed typo where RN_DEV and RN_PROD were the wrong way around * Delete/ignore fixture build outputs * Format scripts/ with Prettier * tidied up the Rollup build process and split functions into various different files to improve readability * Copy folder before files * updated yarn.lock * updated results and yarn dependencies to the latest versions
There was a problem hiding this comment.
Pull request overview
This PR converts React's build system from Gulp/Grunt/Browserify to Rollup, adopting flat bundles. This is a major infrastructure change that replaces the old multi-step build process with a unified Rollup-based pipeline. The old system compiled individual modules into lib/ directories and then bundled them with Browserify; the new system produces flat CJS, UMD, and Facebook-specific bundles directly from source files using Rollup.
Changes:
- Replaces Gulp/Grunt/Browserify build infrastructure with Rollup-based build system (
scripts/rollup/), including bundle configuration, module resolution/aliasing, packaging, and size tracking. - Updates all package entry points (
packages/*/index.js) to use dev/prod CJS bundle switching viaprocess.env.NODE_ENV, and creates shim modules for Facebook (www) and React Native builds that re-export internals via__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED. - Updates CI scripts, fixtures, and documentation to reference the new build output paths and commands (e.g.,
npm run buildinstead ofgrunt build).
Reviewed changes
Copilot reviewed 208 out of 240 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/rollup/bundles.js | Defines all bundle configurations (entry, paths, bundle types, externals) |
| scripts/rollup/modules.js | Module aliasing, resolution, and shim logic for Rollup |
| scripts/rollup/packaging.js | Post-build file copying and package assembly |
| scripts/rollup/stats.js | Bundle size tracking and comparison |
| scripts/rollup/header.js | License/banner header generation |
| scripts/rollup/mangle.js | Property mangling whitelist |
| scripts/rollup/results.json | Baseline bundle size data |
| scripts/rollup/plugins/sizes-plugin.js | Rollup plugin for capturing bundle sizes |
| scripts/rollup/shims/** | Shim modules for Rollup, Facebook www, and React Native |
| scripts/tasks/{jest,flow,eslint,version-check}.js | Standalone task scripts replacing Gulp tasks |
| scripts/circleci/* | CI scripts updated to use new build commands |
| scripts/jest/preprocessor.js | Jest preprocessor updated with new babel plugin |
| packages/*/index.js, package.json | Package entry points switched to dev/prod CJS bundles |
| src/fb/{ReactFBEntry,ReactDOMFBEntry,ReactDOMFiberFBEntry}.js | Facebook-specific entry points with secret internals |
| src/test/reactComponentExpect.js | Hardcoded HostText constant for build compatibility |
| src/umd/ReactDOMUMDEntry.js | Simplified UMD entry, now uses ReactDOMFiber |
| src/renderers/art/ReactARTStack.js | Replaced object-assign with Object.assign, added batching fix |
| src/renderers/noop/ReactNoop.js | Exposed ReactFiberInstrumentation via secret internals |
| fixtures/** | Updated build paths, added dev/prod fixture variants |
| package.json | Added Rollup deps, updated scripts, removed Gulp/Grunt deps |
| Gruntfile.js, gulpfile.js, grunt/** | Removed old build system |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
|
|
||
| function getReactCurrentOwnerModuleAlias(bundleType, isRenderer) { | ||
| if (bundleType === FB_DEV || bundleType === FB_DEV) { |
There was a problem hiding this comment.
Bug: The condition checks FB_DEV twice instead of checking FB_DEV and FB_PROD. The second condition should be bundleType === FB_PROD instead of bundleType === FB_DEV. This means the function will never return an empty object for FB_PROD bundle types, causing incorrect module aliasing for Facebook production builds.
| </div> | ||
| <div class="frame"> | ||
| <h2>systemjs (prod)</h2> | ||
| <iframe src="/fixtures/packaging/systemjs/dev.html"></iframe> |
There was a problem hiding this comment.
The systemjs (prod) iframe incorrectly points to systemjs/dev.html instead of systemjs/prod.html. This means both the dev and prod SystemJS iframes will show the dev version.
| const { | ||
| __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED, | ||
| } = require('ReactDOM-fb'); | ||
|
|
||
| module.exports = __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactDOMInputSelection; |
There was a problem hiding this comment.
Inconsistency: The @providesModule name is ReactDOMInputSelection but the FB entry files (ReactDOMFBEntry.js and ReactDOMFiberFBEntry.js) expose this internal as ReactInputSelection. This shim will be provided as ReactDOMInputSelection module but references ReactInputSelection from the internals. If code imports ReactDOMInputSelection, it would get undefined since the secret internals key is ReactInputSelection.
| "'react-dom/lib/ReactTestUtils'", | ||
| ]; | ||
|
|
||
| // this function builds up a very niave Haste-like moduleMap |
There was a problem hiding this comment.
Spelling: "niave" should be "naïve" (or more commonly "naive").
| function getExternalModules(externals, bundleType, isRenderer) { | ||
| // external modules tell Rollup that we should not attempt | ||
| // to bundle these modules and instead treat them as | ||
| // external depedencies to the bundle. so for CJS bundles |
There was a problem hiding this comment.
Spelling: "depedencies" should be "dependencies".
| * `build/dist/react.dev.js` | ||
| * `build/dist/react.min.prod.js` | ||
| * `build/dist/react-dom.dev.js` | ||
| * `build/dist/react-dom.prod.min.js` |
There was a problem hiding this comment.
The UMD build filenames referenced in the Readme (react.dev.js, react.min.prod.js, react-dom.dev.js, react-dom.prod.min.js) appear inconsistent. Based on the results.json and fixture references elsewhere in the PR, the actual filenames follow the pattern react.development.js, react.production.min.js, react-dom.development.js, react-dom.production.min.js. These filenames should be corrected to match.
| const { | ||
| __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED, | ||
| } = require('ReactDOM-fb'); | ||
|
|
||
| module.exports = __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactTestUtils; |
There was a problem hiding this comment.
The ReactTestUtils is exposed in __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED only inside the __DEV__ block (line 52), but the ReactTestUtils shim file unconditionally accesses it via __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactTestUtils. In production, this would be undefined, which could cause confusing errors if the shim is ever loaded outside of dev mode.
| jest.on('close', code => { | ||
| if (code === 1) { | ||
| console.error('Jest failed!'); | ||
| } else { | ||
| console.log('Jest passed!'); | ||
| } | ||
| process.exit(0); |
There was a problem hiding this comment.
Bug: The jest task always exits with code 0 regardless of whether Jest passed or failed. When code === 1 (Jest failed), the script logs the error but still calls process.exit(0), so CI will never fail on test failures. This should be process.exit(code) to properly propagate the exit code.
| var { | ||
| HostText, | ||
| } = ReactTypeOfWork; | ||
| var HostText = 6; // ReactTypeOfWork |
There was a problem hiding this comment.
Hardcoding the HostText constant as 6 with only a comment referencing ReactTypeOfWork is fragile. If the enum value in ReactTypeOfWork changes, this will silently break. Consider importing or requiring the constant from its source, or at least adding an assertion to verify the value matches at runtime during tests.
| function percentChange(prev, current) { | ||
| const change = Math.floor((current - prev) / prev * 100); | ||
|
|
||
| if (change > 0) { | ||
| return chalk.red.bold(`+${change} %`); | ||
| } else if (change <= 0) { | ||
| return chalk.green.bold(change + ' %'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: In percentChange, when prev is 0, this will result in division by zero producing Infinity or NaN. The function also has a potential issue: Math.floor on a negative number like -0.5 gives -1, which would match change <= 0 and be shown in green, but Math.floor(Infinity) returns Infinity which is > 0 but the formatted output would look odd. Consider adding a guard for prev === 0.
No description provided.