Flat bundle using Rollup

[keyz]

This implements #6351 using Rollup. It could be used to replace our Browserify build step (i.e., it takes pre-built files from ./build/modules) and it's pretty similar to #4230.

Firstly I moved all our non-top-level requires to the top level and __DEV__ on their usage (we only have 5 files that have that). Since we no longer have dynamic parts in our require/exports, they are pretty much ES6-compliant. Then there's a Babel pass for Rollup to firstly rewrite all require/exports to ES6 import/exports, and finally it bundles to a flat file (in UMD format). It sounds weird to transpile files from ES5 to ES6 but using this babel pass results in smaller file sizes comparing with using Rollup's CommonJS plugin.

This decreases our gzipped minified (prod) file size by ~10% and some of the benchmarks look pretty good.

cc @facebook/react-core

Size Comparison

Running "compare_size:files" (compare_size) task
   raw     gz Sizes
  1199    641 build/react-dom-server.js
   734    445 build/react-dom-server.min.js
  1180    632 build/react-dom.js
   715    436 build/react-dom.min.js
668011 157714 build/react-with-addons.js
157020  46842 build/react-with-addons.min.js
601268 143463 build/react.js
134356  40475 build/react.min.js

   raw     gz Compared to master @ e5513eceff9ce8ae24f1d4de934408e2cc47311b
     =      = build/react-dom-server.js
     =      = build/react-dom-server.min.js
     =      = build/react-dom.js
     =      = build/react-dom.min.js
-112191-16858 build/react-with-addons.js
 -4003   -942 build/react-with-addons.min.js
-100276-13997 build/react.js
-15062  -4011 build/react.min.js

Benchmarks

Comparing min.txt (control) vs rollup.txt (test)
Significant differences marked by ***
% change from control to test, with 99% CIs:

* factory_ms_jsc_jit
    % change: -32.37% [-34.96%, -29.78%]  ***
    means: 16.6517 (control), 11.263 (test)
* factory_ms_jsc_nojit
    % change: -32.88% [-35.85%, -29.91%]  ***
    means: 15.6549 (control), 10.509 (test)
* factory_ms_node
    % change: -33.34% [-35.78%, -30.90%]  ***
    means: 50.0445 (control), 33.3632 (test)
* ssr_pe_cold_ms_jsc_jit
    % change:  -1.71% [ -5.26%,  +1.84%]
    means: 26.7829 (control), 26.3289 (test)
* ssr_pe_cold_ms_jsc_nojit
    % change:  +2.23% [ -3.06%,  +7.52%]
    means: 28.3938 (control), 29.0334 (test)
* ssr_pe_cold_ms_node
    % change:  +1.36% [ -1.29%,  +4.02%]
    means: 51.2295 (control), 51.9317 (test)

TODO:

• Let Babel throw if there are non-top-level requires/exports
• Instead of having Rollup config files, make it a script that can be executed by gulp/grunt

[sebmarkbage]

This is pretty awesome! What kind of features does rollup have? Are we missing out on anything that doesn't get applied due to some pattern we use?

[STRML]

That's really great @keyanzhang. Considering the __DEV__ require()s are now hoisted, shouldn't we be seeing a bit of a bump in production size? Or is it smart enough to recognize that it's only used in an unreachable branch and eventually tree-shakes it out?

[keyz]

@sebmarkbage I think if we switch to ES6 modules in our code base and have more named imports/exports, Rollup could shake the tree with a better granularity. My Babel pass can only change module.exports = ... to export default ... and var m = require('m'); to import m from 'm';; from this example it seems like Rollup only prunes on a module basis when we don't have named exports.

[keyz]

@STRML I'm no Rollup expert but seems it does both DCE and tree shaking (which is great). If you take a look at here you'll see that add1 didn't get bundled since the branch got folded.

In our code base, if (__DEV__) {...} becomes if (process.env.NODE_ENV === 'production') {...} after going through the first (gulp) build pipeline; by replacing process.env.NODE_ENV with a string literal Rollup is able to do constant folding and prune unused imports.

[STRML]

from this example it seems like Rollup only prunes on a module basis.

Yeah, it can't know for sure if the exported object is modified at some point or uses this. Perhaps a transformation could transform an exported object into individual exported members, but I don't believe React's codebase generally splits modules pretty well so there wouldn't be a lot of gain there.

Great to hear DCE + tree shaking means getting rid of the "require in a constant branch" hack. Really impressive stuff.

[keyz]

Yeah, its DCE + tree shaking is really impressive.

Here's an unminified production build bundled by Rollup: https://gist.github.com/keyanzhang/1dff99f6f54bcf9d4e3cc92ab407c41e (Note: ReactElementValidator and ReactComponentTreeDevtool still show up but they will be pruned by UglifyJS while minifying).

[ericf]

@keyanzhang if you haven't seen it already, there's a Rollup plugin that will handle require(): https://github.com/rollup/rollup-plugin-commonjs

[keyz]

Thanks @ericf. I've tried that but it wraps modules and keeps individual module.exports, which results in a larger bundle size. Our code is very disciplined (no dynamic requires/exports) so the Babel hack works better.

[keyz]

I added 2 commits that DEV on some dev-only usage and side-effects. By doing this we can further remove ReactComponentTreeDevtool, ReactDOMDebugTool, and their deps from our prod bundle, which decreases another ~1800 bytes.

@sebmarkbage I think we can have a mechanism that checks for these things first. We could consider adding more target flags like NODE_ENV: production | development | profile, REACT_ENV: server | browser, then annotating modules and making sure that they don't get included in wrong target bundles.

Moreover, 5ee5a97 is an example that shows the benefit of using named exports. All the usages of LinkedValueUtils.checkPropTypes are under __DEV__; if we can use named exports then checkPropTypes will be pruned automatically by Rollup.

With these changes we can keep our production file size under 40k.

Running "compare_size:files" (compare_size) task
     raw     gz Sizes
    1199    641 build/react-dom-server.js
     734    445 build/react-dom-server.min.js
    1180    632 build/react-dom.js
     715    436 build/react-dom.min.js
  774615 173498 build/react-with-addons.js
  176031  51851 build/react-with-addons.min.js
  695941 157031 build/react.js
+ 597033 142815 build/react.rollup.dev.js
  156814  46530 build/react.min.js
+ 132667  39971 build/react.rollup.prod.js

[keyz]

Been playing with it for a bit, I think there are 2 approaches that we could choose from. Comments and feedback are more than welcome!

1. Switch to use ES6 modules in React's code base

• Instead of exporting a default object that contains all methods, we switch to named exports when possible. Rollup is able to track and prune unused things when we do import * as ReactComponentTreeDevtool from 'ReactComponentTreeDevtool';, so we'll have a better tree-shaking granularity
• Pros:
  • We only need to branch on usage (like if (__DEV__) { doThis(); }) but not on imports/exports. Usage === Source of truth
  • Presumably this makes having more target flags (like browser, server, profiling, etc.) easier since all we need is to branch on usage
  • ES6 modules export bindings but not values. This means we can refactor more modules to use named exports. For instance, in ReactComponentTreeDevtool, this line (ReactComponentTreeDevtool._preventPurging) can be changed to a flag variable that lives in the module, therefore we no longer need to make ReactComponentTreeDevtool an object
• Cons:
  • Compatibility with Facebook's code base, React Native, etc.

2. Use the Babel transform trick as presented in this PR

• This means we need to branch on both usage and exports (like 5ee5a97)
• Pros:
  • Compatibility
• Cons:
  • More manual work. We'll need to find all target branches by usage, trace back to their dependencies, and add branch on exports. This is more error-prone; we might accidentally bundle more than we need if we forget to branch on an export
  • The Babel trick relies on the fact that our code and runtime dependencies (object-assign and fbjs) are very disciplined (although this probably won't change)

[vjeux]

FYI, React Native supports es6 modules :)

[sophiebits]

Doesn't really matter if RN supports es6 modules since we'd need to compile to CommonJS for npm regardless, which is where RN gets React from anyway.

[keyz]

@spicyj @vjeux does RN require React internals (like require('react/lib/*'))? I know Enzyme relies on this and will be broken if we switch to flat bundle completely.

Another question is how far do we want to go on this. A low hanging fruit is to replace Browserify with Rollup -- that means we only make react/dist/* smaller and it won't make a big difference for people who bundle React themselves; although you can alias in Webpack to redirect requires in production to the react/dist/*.min files. This approach requires minimal changes to our code base and I personally prefer this as a good first move.

Or, we use our bundled file in npm -- as @zpao mentioned in #6351 (comment), we'll have "a single entry point that does the dev check, requiring & re-exporting either the min or dev version".

[zpao]

Another question is how far do we want to go on this.

I think there are 2 distinct steps (as you noted) and we should consider them separately for now.

[keyz]

@zpao How about we use the babel trick for now and only replace Browserify with Rollup?

[keyz]

Note to self: revert #7188 and #7189 when this gets finished

[sebmarkbage]

@keyanzhang Can you try this on top of #7164 to see the effect?

[keyz]

@sebmarkbage Sure. However I'm not sure what's a good way to lazily load (i.e., those getters) modules that depend on DOM internals since we can't have non-top-level requires. Could you give me some suggestions?

[sebmarkbage]

@keyanzhang One thing you could do initially, is just ignore them for now or fallback to browserify. Those are only used in DEV and only the production build is really interesting.

[keyz]

This PR is ready for review!

Introducing a regression for webpack/Browserify users isn't great. After talking to @spicyj and @gaearon we decided to find all dev-only modules and hide their implementations under __DEV__ blocks. In this way, only a placeholder (like emptyFunction, null, or {}) will be imported for webpack/Browserify builds and it effectively reduces the build size.

We also added a Dev postfix to all dev-only modules, which will make future work easier. As a result, 14 modules are refactored and marked as dev-only:

• ReactElementValidator.js -> ReactElementValidatorDev.js
• ReactComponentTreeDevtool.js -> ReactComponentTreeDevtoolDev.js
• validateDOMNesting.js -> validateDOMNestingDev.js
• ReactDOMDebugTool.js -> ReactDOMDebugToolDev.js
• ReactDOMNullInputValuePropDevtool.js -> ReactDOMNullInputValuePropDevtoolDev.js
• ReactDOMUnknownPropertyDevtool.js -> ReactDOMUnknownPropertyDevtoolDev.js
• ReactDebugTool.js -> ReactDebugToolDev.js
• ReactInstrumentation.js -> ReactInstrumentationDev.js
• ReactPerf.js -> ReactPerfDev.js
• ReactChildrenMutationWarningDevtool.js -> ReactChildrenMutationWarningDevtoolDev.js
• ReactHostOperationHistoryDevtool.js -> ReactHostOperationHistoryDevtoolDev.js
• ReactInvalidSetStateWarningDevTool.js -> ReactInvalidSetStateWarningDevToolDev.js
• canDefineProperty.js -> canDefinePropertyDev.js
• ReactDOMInstrumentation.js -> ReactDOMInstrumentationDev.js

As a side note, src/test/* modules should be dev-only but they are currently supported in production by accident. Removing this support would be a breaking change; we can ship them as normal and break them reconsider for 16.0.

Size comparision:

Running "compare_size:files" (compare_size) task
    raw     gz Sizes
   1199    641 build/react-dom-server.js
    734    445 build/react-dom-server.min.js
   1180    632 build/react-dom.js
    715    436 build/react-dom.min.js
+674209 158589 build/react-with-addons.js
-782976 175169 build/react-with-addons.js
+150724  44871 build/react-with-addons.min.js
-161325  47892 build/react-with-addons.min.js
+605887 144004 build/react.js
-704268 158059 build/react.js
+134184  40402 build/react.min.js
-149721  44593 build/react.min.js

I tested with https://github.com/erikras/react-redux-universal-hot-example again and the webpack build was 2K smaller comparing with master (564K -> 562K).

[keyz]

@gaearon would you take a look at this? ReactPerf is the only dev-only module that we publicly expose and it was refactored to make sure the public API doesn't break.

[zpao]

Sorry for the delay 😓 A few things…

1. We'll need to rebase this on top of the hooks rename. Have fun :)
2. I still don't really like the Dev suffix. It feels really brutish and still error prone. Is this the best idea we have?
3. If we have to keep that suffix… Do we really need it for the hooks? They are already implicitly dev-only.
4. Renaming a bunch of files means we're likely going to have some things in www that break. We should prep a fix for that once it's all settled.

Otherwise, still looks fine.

[sebmarkbage]

Will need to be revised. @trueadm will likely pick this up.

[gaearon]

Superseded by #9147.