Browserify production build is not optimized by default

[thibaudcolas]

Hey hey,

(formal bug report below, without as much context on my solution)

TL;DR; Like react, use loose-envify dependency / browserify transform to prevent browserify users from shipping development code in production.
(Warning: big file) L248 of https://github.com/ThibWeb/react-router-browserify-build/blob/master/bundle.js#L248.

When using browserify to bundle a project that uses react-router, the default configuration of browserify transforms causes around 3kb of minified/gzipped warning messages to end up in the production bundles.

The source of the problem is that by default browserify transforms do not run on packages in node_modules. Configuring those to run on the whole bundle is simple, but I believe this is counter-intuitive to the common "best practices", and people might overlook this.

I imagine this hasn't been reported before because React users seem to use Webpack more than Browserify, and Webpack's DefinePlugin targets the whole bundle.

facebook/react handles this problem by depending on loose-envify, and declaring the transform it in its package.json. Since React itself does this I believe that react-router should follow the lead and not rely on users to do the right thing. That said, it's another burden on the package authors for something that can also be handled with appropriate configuration by users. It also needs to be done in https://github.com/mjackson/history, and a big number of other React-related projects I imagine.

The other option is to document this issue in this project and others, by specifying the consequences (dev code in production, dead weight in the JS bundles) and fix (global flag for a transform that replaces NODE_ENV and potentially other variables).

---

Version

2.3.0, 2.4.1, and history@2.1.2 (probably all of react-router@2 & other versions of history as well)

Test Case

https://github.com/ThibWeb/react-router-browserify-build
(Warning: big file) L248 of https://github.com/ThibWeb/react-router-browserify-build/blob/master/bundle.js#L248.

Steps to reproduce

Compile a project using react-router with browserify, without running an "envify" transform over all node_modules (default behavior).

Example: NODE_ENV=production browserify index.js -t [ babelify ] -t [ envify ] > bundle.js

Expected Behavior

Messages intended for developers should be surrounded by 'production' !== 'production' (or other env values for that matter) checks so that minification removes them.

Actual Behavior

±3kb (gzipped) of development aid messages are kept in the bundle and make their way to production.

[taion]

Sure, but to pull up a couple more examples, neither react-dom nor relay do this. I think in practice it'd just be better to accept the caveat for Browserify builds (the same way that webpack builds need to use DefinePlugin), rather than to end up with a suboptimal bundle if any given package fails to add explicit Browserify stuff.

[thibaudcolas]

Completely agree, and personally I'll definitely be running those transforms globally. I still think this solution is worth discussing, first because react does it and second because react-router is one of the biggest 3rd-party "React" projects.

IMHO the broader documentation solution is also necessary, regardless of what this particular project does of this PR. I've been using react and react-router with browserify for over a year now, think I'm quite proficient with those – and never noticed this problem before because react is taking care of it silently.

For react-router and other projects that publish code with development aids as a default, I think a "looks like you are using development code in production" warning would be a useful additional aid that would solve or at least help with that kind of problem.

[timdorr]

👍 from me. I ran into this same thing with Redux: reduxjs/redux#1301

[taion]

Okay fair enough

[mjackson]

Do we really need to include loose-envify in dependencies just so Browserify users can use it? Feels like a dev-only dependency to me.