-
Notifications
You must be signed in to change notification settings - Fork 47.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal: Replace NODE_ENV with REACT_ENV for __DEV__ replacement #6582
Comments
Seems like Makes no sense for redux to use If you must not conflict with |
Maybe |
I like the idea of people being able to set |
Definitely want to avoid that.
Yea, this is interesting and could be an approach to take. There's also the concern that sometimes you are running in Node and do want Thanks for the quick feedback all! |
Babel is an interesting case because people use it both on the client and in production, and |
Historically, I run code in both "qa" and "staging" NODE_ENVs. For minification to work and accurately reflect what will soon be on "production", the builds run with NODE_ENV as "production", but client-side code checks against STAGE for the true environment (as our loggers and reporters call differently based on the actually environment). Point being, I think it's exceedingly difficult to solve this on the vendor side when the NODE_ENV pattern is so prevalent. There's a point where the lookup will always be faked to meet the needs of the build. I wonder if there's something more we can do in user land to resolve this? |
NODE_ENV is a really well established standard across the entire JavaScript ecosystem. As I see it, there's nothing wrong with people having something like There may be cases where it is useful to put one specific library/part of your code into development mode, while everything else is in prod mode (or visa versa) but I think it's a corner case. This is not what's being asked for in any of the three linked issues. I think we should wait until someone actually wants the feature before we build it, and if we build it it should be as @stubailo (an optional variable to override the default NODE_ENV behaviour). |
I think it's worth noting that it should be possible to write a browserify transform/webpack loader which applies a different environment variable to a specific library. This way it will be useful to all without needing each library implement a separate environment variable. |
And babel-plugin for that matter (which would then work server side). I really like that suggestion. |
It seems like |
It sounds like people are (cautiously) positive to an optional, overriding env var. Since we are only toggling between a release/optimized/minified build and a debug/dev build, could we maybe have that reflected in the name & values of that var? The pattern Our variable
With those requirements, I would suggest something along these lines:
or maybe
Using strings for a "boolean" value is problematic; you might expect On the hand, using specific keywords easily devolves into bikeshedding ( Edit: accidentally posted early, revised text a bit. |
I just feel like this ship has sailed. A ton of libraries in the React ecosystem already rely on |
Also, as I mention in #7512 (comment) regarding
So there is a workaround. |
This has come up a couple times lately as being an issue (#6479, #6581, #6511), I think perhaps because we added the minification warning and people are ending up seeing they aren't getting prod code when they expected it. But there's also the argument that you want React to be production and still use NODE_ENV for other purposes.
There would be a few things to figure out to make sure envify works, and deciding what we do for other projects which currently also use the NODE_ENV pattern (eg, Relay, fbjs, third-party code, etc).
This might not be a good idea at all though and definitely isn't happening immediately, but wanted to start the discussion.
The text was updated successfully, but these errors were encountered: