warn user when using minified code outside of NODE_ENV 'production'

[conorhastings]

@gaearon my attempt at #1029 . Likely still needs a bit of work but I wanted to get your opinion before i continued. thanks!

[gaearon]

This looks correct. (Have you checked whether it works?)
Let's just move this check inline and keep comments to the minimum—it’s not part of a public API.
Something like a two-line comment would do because we explain the issue in the warning anyway.

[conorhastings]

@gaearon updated, is this what you meant by moving the check inline?

Yes, I tested with this branch in a small app using redux and webpack with DefinePlugin with the following combinations:

1. NODE_ENV='production' , minified - no warning.
2. NODE_ENV='development' minified - warning
3. NODE_ENV='development' not minified, no warning
4. NODE_ENV='production' not minified, no warning.
5. NODE_ENV='production' , minified, removed DefinePlugin , warning.

[gaearon]

We're only using warningMessage once, let's put it right inside the console.warn call?

[conorhastings]

yeah, that makes sense, updated.

[gaearon]

Thanks!
Let's leave this hanging for now—I'll come back to this after I have some time to cut a new release.

[conorhastings]

@gaearon sounds good thanks for such a great project and for the quick review/feedback!

[sapegin]

utilzing → utilizing. Or much better: using.

[gaearon]

I'll tweak the error message anyway before merging, no worries!

[timdorr]

@gaearon I'm going to merge this in and tweak the message a tad on master. Please flog me appropriately if you disagree or don't like my tweaks :)

[gaearon]

Thanks, please do!

[gaearon]

Out in 3.0.6, thanks!

[marcelowa]

Hi
I have other environments like staging that is not production but is suppose to behave like production
this should be configurable somehow

[timdorr]

I would use another ENV for swapping your configuration based on environment (CONFIG_ENV, for instance). Even better, I would configure your application based on various ENVs specific to those parts, such as things like DATABASE_URL or IMAGE_HOST or whatever you've got in your app.

NODE_ENV is heavily used by many projects (not just Redux) to check if developer features should be enabled. I wouldn't rely on it for defining a staging context.

[gaearon]

NODE_ENV is heavily used by many projects (not just Redux) to check if developer features should be enabled. I wouldn't rely on it for defining a staging context.

Yes, this is the reason we went with it. For example, React behaves much slower with NODE_ENV being something other than 'production'. Since it’s often used with Redux it makes sense for us to converge on the same behavior.

[marcelowa]

Searched the source code and saw this now
It seems to be consistent
Not sure about this convention though

[gaearon]

Well, it's being used in tons of projects now so we're not changing it anytime soon :-)

[gaearon]

If you don't want to deal with it you can use UMD builds. The minified build is envified for prod environment.

[marcelowa]

I will use my own environment as suggested :)
Thanks

[sirreal]

This line seems to behave differently in ie9 vs. modern browsers. Possible cause of #1311?

[gaearon]

Yup, fixed in 3.1.4.

[xhawk]

We are using react with Java backend and this same error pops up. Why is that?

[gaearon]

It means you are minifying the code, but not envifying the CommonJS build. What do you build your JavaScript assets with?

[xhawk]

We are using Webpack, is this what we should do new webpack.DefinePlugin({ "process.env": { NODE_ENV: JSON.stringify("production") } })

[gaearon]

We are using Webpack, is this what we should do new webpack.DefinePlugin({ "process.env": { NODE_ENV: JSON.stringify("production") } })

Yes. JSON.stringify("production") just makes sure the string is quoted. It’s essentially '"production"'.

[possibilities]

This doesn't seem right, if process.env.NODE_ENV is not defined that doesn't necessarily indicate that I'm using minified code outside of production, or am I missing something? In our codebase we define NODE_ENV without the "process.env" prefix but I can imagine lots of apps have no need to define it.

[possibilities]

From POV it was pretty surprising to see this pop up in production builds. Doesn't seem like something a library should try to enforce IMO.

[timdorr]

That comes from the Node universe, where process.env.NODE_ENV is defined by default. process is a global in Node. It is used in many libraries (React included) to define where development code is. Our module bundler (and most common setups) replaces the text process.env.NODE_ENV with "production", which gets eliminated by an optimizer as dead code.

So, it would be a good idea to emulate the same behavior, even if you don't like it. There are some Babel plugins to make this easier (letting you use a global DEV). You're probably getting development builds of many other things for the same reason.

[gaearon]

The CommonJS build of Redux uses Node conventions. You can explicitly use the UMD builds in dist folder if you’d rather avoid them.

We just went ahead with the same pattern React is using. If you use the CommonJS build of React and don’t envify process.env.NODE_ENV, you are running a slow development version of React in production.

From http://facebook.github.io/react/docs/getting-started.html#using-react-from-npm:

We just follow the same convention.

[possibilities]

Fair enough. I suppose it makes sense for libraries to follow a convention for this and being loud about an inefficient build is good service. Thanks to both for your responses!

[possibilities]

Out of curiosity though I would expect to see conditionals with process.env.NODE_ENV or similar in the react codebase, but I don't. If I'm using webpack and requiring react how does defining it make the react build faster?

[timdorr]

They use DEV in their codebase, which is converted to the process.env format when they build the CommonJS version for publishing.

[possibilities]

Oh interesting, got it!