React: Minification warning with production environment.

[dangreen]

I have minification warning with NODE_ENV=production. React v15.

[lcxfs1991]

Same here with react15. It works perfectly fine with react0.14.7

[zpao]

It's a new warning so you wouldn't see it in 0.14.x.

@dangreen Is process.env that being set at runtime some point after ReactDOM is required? I think that's the only way you would see this.

[dangreen]

@zapo I use envify with browserify (also uglify), as written in fb.me/react-minification

[dangreen]

@zapo and envify is used as global transformer

[zpao]

Without steps to reproduce this is going to be a bit tricky to track down. Could you perhaps reproduce your set up minimally? You probably want to be running browserify with your NODE_ENV set (which will trigger the envify part). Here's a simple set up I made that you can use as a model. No transforms (so no JSX) or anything else. Just browserify and uglify. You could even skip the uglify step - browerifying React would be enough, would just leave a bunch of dead code in there. This warning you're seeing should be dead code.

https://gist.github.com/zpao/2b545c0fc5c2fdc8a2911fd49b8b92a4

[dangreen]

@zpao I find loose-envify transform in package.json of react. This transformer replace NODE_ENV before my envify transform. And in result I don't have way to set NODE_ENV manually!

[lambert-velir]

I'm using a gulp build and browserify almost exactly like this: https://github.com/hughsk/envify#module-usage Here is my actual code:

// will expose externalPackages, eg. "react"
var b = browserify({
    require: externalPackages
});

// set the node environment variable.
// https://facebook.github.io/react/downloads.html#npm
b.transform(envify({
    NODE_ENV: (isProduction) ? "production" : "development"
}));

return b.bundle()

I am getting the same warning, and upon further investigation, I found that envify wasn't working at all.

Instead, I found that I can just set the environment variable right there instead of using the transform like this:

// will expose externalPackages, eg. "react"
var b = browserify({
    require: externalPackages
});

// set the node environment variable.
process.env.NODE_ENV = (isProduction) ? "production" : "development";

return b.bundle()

Now, it seems to replace instances of process.env.NODE_ENV with "production", so I'm confused what envify is even for. Was I using it incorrectly?

[zpao]

@lambert-velir 2 things for your case specifically

1. require('envify') will use process.env.* values. If you look at the example in the readme again you'll actually see that you need to require('envify/custom')
2. You may also need to pass {global:true} as the first argument to b.transform so it's applied to dependencies. I think by default browserify won't transform files from node_modules.

@dangreen Again, without seeing how you're running things I can't really help. Yes, React uses loose-envify but I think only when you compile from the command line and presumably you can control that in your build system. If I do it via the API it looks like this is working just fine.

var browserify = require('browserify');
var envify = require('envify/custom');

// where index.js is from the gist above
var b = browserify('./index.js');

b.transform({global: true}, envify({
  NODE_ENV: 'production'
}));

b.bundle(function(err, buf) {
  console.log(buf.toString());
});

I leave off the envify transform, process.env is still in the React code.

[lambert-velir]

Ahh, the {global: true} made envify work. I don't really see that documented anywhere (browserify, https://github.com/substack/node-browserify#btransformtr-opts ?, envify, )...

Is there a reason to use envify vs just setting process.env.NODE_ENV?

[zpao]

It's at the bottom of that section of the browserify docs, but not super obvious. Also is a command line option too.

As to why? The upside is that if you avoid runtime process lookups AND if you then pass your code through uglify or some other minifiier, you'll get the dead code removed.

[lambert-velir]

Sorry, I should have clarified. These two blocks of code seem to result in the same bundle:

b.transform({global: true}, envify({
  NODE_ENV: "production"
}));

process.env.NODE_ENV = "production";

E.g. if(process.env.NODE_ENV !== 'production') was transformed to if ("production" !== 'production') in both cases.

[dangreen]

@zpao I use sails.js framework, in my sails app i have configs for docker env, and this app starts gulp tasks on lifting.

// set NODE_ENV
process.env.NODE_ENV = "docker";

// build
var browserify = require('browserify'),
    fs         = require('fs');

browserify('./index.js').bundle(function(err, buffer) {
    fs.writeFileSync('./bundle.js', buffer.toString(), 'utf8');
});

but I think only when you compile from the command line

No, after this build script i have "docker" instead process.env.NODE_ENV. As i already sad:

This transformer (loose-envify) replace NODE_ENV before my envify transform. And in result I don't have way to set NODE_ENV manually!

[zpao]

@dangreen That's really helpful because now I have steps to reproduce your issue.

Unfortunately I don't think it's possible to disable the transform we specify in our package.json, not recursively anyway. Perhaps if you were browserifying our package directly (eg, instead of ./index.js you specify react) you could use the browserField: false option.

So my best suggestion right now is to either not set NODE_ENV = 'docker', or set it after doing the browserify step (and then adjusting your browserify step to use envify directly). Alternatively, don't use NODE_ENV for values besides 'development', 'production', 'test', and undefined and use some other environment variable to indicate that you're in docker. NODE_ENV isn't a standard but from what I can tell it's become more common practice to expect those values and make adjustments accordingly (eg, even Sails does this in a couple places: https://github.com/balderdashy/sails/search?utf8=%E2%9C%93&q=NODE_ENV&type=Code). Personally that's what I would do, but I don't know everything that went into your decision or what constraints Sails imposes that wouldn't make that possible.

I'm going to close because there's nothing else we can do here. The benefit of automatically transforming outweighs the cost of the occasional difficult integration point. Sorry that you happen to be in one of the cases that isn't easy.