Hoist __DEV__'s process.env use out of hot code.

[STRML]

trafficstars

See react/#812.

Naive benchmarks show nearly 100% performance increase in server rendering performance.

Benched and built on 0.14.3 tag.

Sizes:

   raw     gz Sizes
  1189    635 build/react-dom-server.js
   725    438 build/react-dom-server.min.js
  1170    627 build/react-dom.js
   706    431 build/react-dom.min.js
703188 160014 build/react-with-addons.js
147964  42965 build/react-with-addons.min.js
637385 145166 build/react.js
135574  39485 build/react.min.js

Benchmark (50000 creations of a small nested element):

50000 iterations of Baseline: 8457ms
50000 iterations of dist/react, unminified (dev): 6278ms
50000 iterations of Production: 3928ms
50000 iterations of Production (raw obj process.env): 2160ms
50000 iterations of Minified React (dist/react.min): 1446ms
50000 iterations of Proposed fix (hoist const): 1598ms
50000 iterations of Proposed fix (dist min.js): 1546ms

Repository for bench is at https://github.com/STRML/react-bench (soon to be updated).

In order for uglify to properly kill dead code for the browser builds, we need to blacklist es6.constants so the files compile with the const intact. UglifyJS will then optimize them away.

Re: non const-supporting browsers, we'll likely need to add a post-process step to the React grunt build in case of users browserifying React without varify. This could potentially cause some dead code to enter user's generated browserify/uglify builds as uglify will not properly optimize consts that aren't declared with const. This may be something that we could solve on the UglifyJS end as it makes library compatibility difficult when forcing the use of const. UglifyJS has a tracking issue.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[sophiebits]

I don't think it's practical for us to use const in what we ship unfortunately. This is interesting though…

[zpao]

This is an interesting idea… I think we thought about it before but jstransform made this really tricky & Babel is much nicer. Thanks for taking a stab at it.

cc @cpojer @voideanvalue who are much better at this whole transform stuff and can give a better review.

If we do go down this route we'll definitely want to make sure we update the big comment blocks describing how this works. We'll also have to do some more testing with minifiers. I don't think we want to use const yet - that would deviate from our generation of ES5 compatible code. Perhaps something with envify can work or even switching away from browserify to webpack for our browser builds. I'd like to make sure we remove dead code if we can.

[STRML]

Agreed re: shipping const.

IMO, there's not going to be any way, with current UglifyJS (perhaps Closure is better) to eliminate dead code only with envify (by setting NODE_ENV = 'production') without using const. UglifyJS, at least right now, cannot figure out that a var is not being reassigned because it would technically have to check eval and with.

Because of that, we'll risk an extra ~16kb in users' browser builds if they're using browserify or Webpack.

I have an idea, though, and I'll explore it in UglifyJS - we should be able to tag a var as a const so we get backwards-compatible consts that can be eliminated by Uglify. Then, all we'd have to do is update this PR to print a var instead of a const and we can :shipit:.

[zpao]

We could probably do another pass in the browserify step with another babel transform. Might not be ideal but it's workable.

[STRML]

Well, technically if we just changed our uglify step to replace __DEV__, we'd be golden. But the real problem is users installing the npm package and doing their own bundling/minification. We could issue a breaking change and say that you now have to add __DEV__ to global_defs, but that feels messy to me. If I can fix it in UglifyJS outright, all we have to say is that you should be on the latest version and it'll work.

[STRML]

UglifyJS PR is in, if it is merged I'll change this to use /** @const */ var __DEV__ instead of const __DEV__.

[jshow]

UglifyJS PR has been merged

[STRML]

I've rebased this in case anyone's still interested: a brief history of where this is at:

1. In early 2016, I introduced /** @const */ syntax that UglifyJS2 could recognize, which would allow it to eliminate more dead code: https://github.com/mishoo/UglifyJS2/pull/928
2. In September 2016, UglifyJS2 added the reduce_vars option (https://github.com/mishoo/UglifyJS2/pull/1301) which can find effectively-constant vars and do the same optimization.
3. In late Feb 2017, UglifyJS2 made reduce_vars a default (https://github.com/mishoo/UglifyJS2/pull/1498).
4. In early March, evaluate and reduce_vars were consolidated.

This means that we no longer have to repeat a constant condition in order for UglifyJS to replace it, like so:

if (process.env.NODE_ENV !== 'production') {
  // dev code, this will be eliminated
}

We can now just make __DEV__ a real var:

var __DEV__ = process.env.NODE_ENV !== 'production';
if (__DEV__) {
  // dev code, this will be eliminated
}

Because this dead code elimination is now happening properly on browser builds, it's safe to hoist __DEV__, which means the slow React-Server bug is actually fixed with this.

[gaearon]

FWIW it's probably not going to be important soon because with React 16 we plan to have separate precompiled bundles for development and production, and just one switch.

[STRML]

@gaearon And that includes SSR? Will all the files be individually compiled or will you be essentially require()ing a bundle?

[gaearon]

We don't have Fiber-compatible implementation of SSR but the plan is to figure it out for 16 release. By that time, we intend to have react-dom-server.node-dev.js and react-dom-server.node-prod.min.js loaded depending on process.env.NODE_ENV, and same with react.node-dev.js and react.node-prod.min.js. Those will be single-file CommonJS bundles.