Skip to content
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

NODE_ENV should be used strictly #1565

Closed
Morriz opened this issue Apr 1, 2016 · 8 comments
Closed

NODE_ENV should be used strictly #1565

Morriz opened this issue Apr 1, 2016 · 8 comments

Comments

@Morriz
Copy link

Morriz commented Apr 1, 2016

I would suggest using explicit tests to see if development features are needed, also to accomodate other dev/test/acc/prod usage in other packages, like in loopback, so:

if (process.env.NODE_ENV !== 'production')
would become
if (process.env.NODE_ENV === 'development')

@Morriz
Copy link
Author

Morriz commented Apr 1, 2016

At the same time, please change the following:

    // set global vars
    new webpack.DefinePlugin({
      'process.env': {
        // Useful to reduce the size of client-side libraries, e.g. react
        NODE_ENV: JSON.stringify(process.env.NODE_ENV)
      }
    }),

This makes it possible to use NODE_ENV later on, i.e. to give context to the client, which is static, but needs to know sometimes what env it's running in, for example to point to statics in S3

@sompylasar
Copy link

sompylasar commented Apr 1, 2016 via email

@gaearon
Copy link
Contributor

gaearon commented Apr 1, 2016

We are using !== to be consistent with other packages in React ecosystem, including React itself, which use this check. I don’t think it would be wise for us to deviate from it at this point—it would be especially confusing for binding libraries like React Redux which use both React and Redux.

As for why we use NODE_ENV at all, please refer to the discussion in #1075. Even if we wanted to use a different variable, it would be problematic because of all other packages (including React) that use NODE_ENV for this. It’s just not worth it going against the convention, and indeed, you can always use a custom env variable for your app.

I hope this helps!

@gaearon gaearon closed this as completed Apr 1, 2016
@Morriz
Copy link
Author

Morriz commented Apr 1, 2016

But I would like an argument, not just status quo reasoning. My suggestion doesn't break anything, but allows for the other use case as well...

So no breaking anything, otherwise I wouldn't suggest it...so please consider your resistance and reason about it afresh

@gaearon
Copy link
Contributor

gaearon commented Apr 1, 2016

My argument is that Redux is overwhelmingly used in React projects, and introducing a different convention is going to cause people who don’t specify NODE_ENV to get inconsistent builds (e.g. dev React but prod Redux and React Redux).

@gaearon
Copy link
Contributor

gaearon commented Apr 1, 2016

If you insist on specifying something other than development or production on NODE_ENV and treating it like an “app-level” variable, please use UMD builds of Redux. They are prebuilt and don’t care about NODE_ENV at all. You can configure the build tool of your choice to resolve redux to redux/dist/redux or redux/dist/redux.min depending on which one you want to use.

@Morriz
Copy link
Author

Morriz commented Apr 1, 2016

I understand....too bad we are throwing away what was given to us long ago, just because some dumbasses didn't realize that a lot of tooling DOES use the NODE_ENV for dev/test/acc/prod...typically front end people who don't look beyond their scope.

Node.js was there looooooong before it was used for frontend tooling...shame

@reduxjs reduxjs locked and limited conversation to collaborators Apr 1, 2016
@gaearon
Copy link
Contributor

gaearon commented Apr 1, 2016

We don’t tolerate personal attacks and derogatory language in this repository.
Unfortunately we won’t be able to have a meaningful conversation for this reason.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants