Core: Replaced process.env override in DefinePlugin config

[vdh]

Issue: #15927

Overriding process.env in DefinePlugin causes issues and greatly bloats the compiled output by inserting a large JSON blob into anywhere code uses process.env.

E.g. any small bit of code like this:

if (process.env.NODE_ENV !== "production") {
  /* … */
}

Is now horribly bloated into:

if ({
/* the entire process.env stringified into a JSON object */
}.NODE_ENV !== "production") {
      /* … */
}

Even if UglifyJS / Terser / etc is able to collapse that cleanly, it's easily possible for this repeated long JSON string to expend all the available RAM during a build. I've learnt from this mistake in the past when I personally misconfigured DefinePlugin in this same way and had horrible build issues due to it.

The current config causes Conflicting values for 'process.env' errors in the build logs, and was mentioned previously in:
#14257 (comment)

It was raised before in #569, and the original PR #565 only made the change to enable console.log(process.env), which is not a good trade-off for the build errors and build-time bloat that such a dramatic config change caused. This sort of config has also caused issues for create-react-app: facebook/create-react-app#915

The DefinePlugin docs already include a warning about overriding process, and includes a recommendation for 'process.env.NODE_ENV': JSON.stringify('production'), so it is implied that overriding process.env is also a problematic approach as well.

What I did

Replaced stringifyEnvs with a similar stringifyProcessEnvs util that sets a stringified process.env.${key} directly for each env key. This is the recommended way to use variable substitution with DefinePlugin. The existing NODE_ENV keys were left untouched in case of legacy code, although I would recommend using process.env.NODE_ENV where possible.

I included an empty fallback for process.env.XSTORYBOOK_EXAMPLE_APP for the variable used in lib/ui/src/index.tsx so that the lack of a replacement doesn't cause a build failure. If there are any other potentially undefined variables used in the future, it would also be wise to set fallbacks for those too, to defuse potential stealth time bombs. It doesn't seem wise to rely on the accidental saving grace of a JSON blob inside JIT-compiled code to narrowly avoid a runtime error due to a missing variable.

As a trade-off, it does remove access to process.env accessed as an object, but a Node API like process shouldn't be so deeply dependent on in browser environments. All the instances of libraries I've observed have followed the process.env.NODE_ENV (or similar) patterns, and those patterns have been targeted for substitution in most DefinePlugin setup recommendations I've observed. Is there even a legitimate non-console usage of process.env that wouldn't cause serious bloat as this misconfiguration does?

In summary, replacing process.env with process.env.* configs helps reduce build times and reduce conflicting build errors for what seems like a minimal trade-off, and helps to reduce dangerous stealth env dependencies.

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit 17cbc66. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	nx run-many --target=prepare --all --parallel --max-parallel=15

---

Sent with 💌 from NxCloud.

[usrrname]

Thanks for starting this... searching the internet far and wide for something to fix this... hope you get it merged :D

[shilman]

Looking good and works well! Thanks @vdh !!! 🙏

[7rulnik]

I also tried to do it in #13949 😄
Glad to see it merged!

[shilman]

Aha thanks @7rulnik !! sorry we didn't get yours in 😭

[vdh]

@7rulnik for your use-case, I would recommend setting scoped keys in your own setup as well, since DefinePlugin works best the more specific your replacements are, e.g. to reference this example, instead of:

new webpack.DefinePlugin({
	'process.env': 'some_global_variable_that_stores_values',
}),

use something like:

new webpack.DefinePlugin({
	'process.env.FOO': 'some_global_variable_that_stores_values.FOO',
	// as many additional variables as needed…
}),

Unless you have those variables available during build time (and they wont change during runtime), in which case it's better to JSON.stringify them so they become constants. e.g.

if (process.env.FOO === "foo") { doFoo(); }
if (process.env.FOO === "bar") { doBar(); }
// after DefinePlugin({ 'process.env.FOO': '"foo"' }):
if ("foo" === "foo") { doFoo(); }
if ("foo" === "bar") { doBar(); }
// which are equivalent to:
if (true) { doFoo(); }
if (false) { doBar(); }
// which would get trimmed by the compression (e.g. Terser) to something like:
doFoo();