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

Displaying wrong warning about process.env.NODE_ENV #4180

Closed
schmeter opened this issue Oct 3, 2021 · 15 comments
Closed

Displaying wrong warning about process.env.NODE_ENV #4180

schmeter opened this issue Oct 3, 2021 · 15 comments

Comments

@schmeter
Copy link

schmeter commented Oct 3, 2021

What is the current behavior?

Console output is the following: You are currently using minified code outside of NODE_ENV === "production"...

Steps to Reproduce

  • Set NODE_ENV or process.env.NODE_ENV variable to 'production' explicitly.
  • Don't use webpack.
  • Run code. See output.

What is the expected behavior?

No output.

Environment Details

4.1.1, all browsers.

@markerikson
Copy link
Contributor

@schmeter Afraid that's not enough details to understand what your setup is. Can you put together a repo that reproduces this issue? Where are you "setting process.env.NODE_ENV", especially if you're not using Webpack?

@schmeter
Copy link
Author

schmeter commented Oct 3, 2021

@markerikson Setup can be found here, see PR with related logs in setup phase and production output: https://github.com/schmeter/music/pull/20/files

@markerikson
Copy link
Contributor

Just defining process.env.NODE_ENV in grunt won't do anything. You need to configure your build tooling to effectively do a search-and-replace on the JS code itself. See Dan Abramov's post here for a longer explanation:

https://overreacted.io/how-does-the-development-mode-work/

@schmeter
Copy link
Author

schmeter commented Oct 3, 2021

@markerikson Closing might be quite overhasty here, the issue is as described: Displaying that a variable is not set AND minified code is in use, the output is still wrong. Setting via JS is exactly what $NODE_ENV puts into it, so other libraries know the read and redux doesn't. What would be setting it and passing through uglify otherwise cause?

@markerikson
Copy link
Contributor

@schmeter per my comment, the issue is not about setting the actual value of process.env.NODE_ENV in your Node runtime environment. It's about replacing that value in the JS code itself.

In other words, this code in a library in node_modules:

if (process.env.NODE_ENV !== 'production') {
  console.log('Some dev-only warning')
}

Must get transformed into this code during the build process:

if ('production' !== 'production') {
    console.log('Some dev-only warning')
}

which will then be eliminated by a minifier as the comparison results in dead code. This requires a literal string search-and-replace operation, completely separate from the actual value of process.env.NODE_ENV in your build environment.

Now, it's true that the value used in that search-and-replace is normally based on the value of process.env.NODE_ENV in your build environment, but the build step has to be set up to do that.

With Webpack, you'd normally do it using the DefinePlugin, like:

new webpack.DefinePlugin({
  PRODUCTION: JSON.stringify(true),
  VERSION: JSON.stringify('5fa3b9'),
  BROWSER_SUPPORTS_HTML5: true,
  TWO: '1+1',
  'typeof window': JSON.stringify('object'),
  'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),
})

or alternately the EnvironmentPlugin.

Ultimately, this is an issue with your build setup. I see per https://github.com/schmeter/music/blob/034e56554164489d1758b12ccb5e4ebe41f61682/Gruntfile.js#L38-L40 that you're defining NODE_ENV: 'production', but I don't see any evidence that you're running an equivalent of that search-and-replace step in your build pipeline. That is the problem here.

@schmeter
Copy link
Author

schmeter commented Oct 3, 2021

@markerikson
Copy link
Contributor

Ah, gotcha.

Per the release notes for 4.1.0 at https://github.com/reduxjs/redux/releases/tag/v4.1.0 , we removed the loose-envify tag from our own package.json, so you'll need to configure that yourself to specifically convert the redux lib code.

@schmeter
Copy link
Author

schmeter commented Oct 3, 2021

The message says it quite explicitly: You are currently using minified code outside of NODE_ENV === "production". This means that you are running a slower development build of Redux. You can use loose-envify (https://github.com/zertosh/loose-envify) for browserify or setting mode to production in webpack (https://webpack.js.org/concepts/mode/) to ensure you have the correct code for your production build.

I use loose-envify as announced an it fails? Envify doesn't work too.

@markerikson
Copy link
Contributor

I've never used Browserify or loose-envify myself. My assumption, based on the existence of that field in our package.json for many years, is that Browserify iterates over all library package.json fields to apply any necessary transforms listed. I would assume this used to work okay with Redux 4.0.5.

Because we removed that listed transform from our own package.json in 4.1.0, it's now up to you to ensure that Browserify actually transforms the code in the Redux library package.

That, ultimately, is something that's outside the scope of Redux itself.

@schmeter
Copy link
Author

schmeter commented Oct 3, 2021

So the issue here is still that this warning is wrong, isn't it? If loose-envify is removed, it should not be announced to be used, I guess.

@markerikson
Copy link
Contributor

The warning is correct. What appears to be happening is that loose-envify is no longer getting applied to the Redux library code, and thus the warning is coming into play.

Related: https://github.com/browserify/browserify#browserifytransform

@schmeter
Copy link
Author

schmeter commented Oct 3, 2021

The warning is correct.

Nah. Variables set and minification applied. If extraordinary transforming is enforced by specific implementions, a "help-yourself"-warning with indistinct expression is neither helpful nor correct. As you don't agree with that grammar, that's your playground then. Thanks for having a look and clarifying that.

@markerikson
Copy link
Contributor

markerikson commented Oct 3, 2021

@schmeter I'm not sure you're understanding what I'm saying here.

Per Browserify's docs, if a browserify.transform field exists in the package.json of a library, Browserify will apply any listed transforms to that library's source during the build process.

As far as I can see, Browserify doesn't automatically transform lib sources otherwise. I'm assuming that listing transforms in your Browserify config will, by default, only apply those to your own application's source code, but not to libraries.

We removed that entry from our package.json in 4.1. Therefore, it is up to you to ensure that library sources are also getting transformed as needed.

But no, it's not our responsibility to try to list exact instructions on how to configure external tools like that, and especially in a warning message. A pointer in the general direction is sufficient.

@schmeter
Copy link
Author

schmeter commented Oct 4, 2021

@markerikson Understanding is exactly what I'm talking about. If the warning would express clearly what is happening here, no questions needed to redux devs. As it is not, here's the thread: The message is not sufficient. You cannot argue as a person who sends a message, but I can as I receive it. Easy principle, not opinionated but proven in ages.

Besides: Sharing is caring. A future answer to someone pointing at things should be: Thanks for making things up here, as free QA to our work is always very welcome. We'll consider to have better message output for the future. Closing an issue because of an opinion is the opposite.

@klesun
Copy link

klesun commented Aug 9, 2023

tl;dr to all future googlers:

process.env.NODE_ENV = "production";
browserify()
    .transform(require('envify'), { global: true })

Like markerikson pointed out, envify will only process react code if you pass a specific {global: true} parameter to the transform. The lib is not for setting the environment variables (as I mistakenly inferred from the warning), it's for optimizing the minifier by taking existing variables from environment and inlining them.

Sucks to be a browserify user, this thread appears to be the only source of information about the true purpose of envify lib and how to properly use it.

There also appears to be a vagueness in the usage of the lib. Hope I was the last person to still keep looking for the solution from here.

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

No branches or pull requests

3 participants