fix: improve DefinePlugin config

[sdemjanenko]

This configuration is used in webpack's DefinePlugin which prefers keys
that look like process.env.REACT_APP_MY_ENV_VAR instead of a nested
object: https://webpack.js.org/plugins/define-plugin/

Prior to this change, webpack would replace all occurrences of
process.env with a JS object containing every key in the environment
that begins with REACT_APP.

The expected behavior is that a reference to
process.env.REACT_APP_MY_ENV_VAR is replaced with the value of just
that environment variable.

[ianschmitz]

I'm not sure i understand this change. Was there a bug you were experiencing?

[sdemjanenko]

In the build output, webpack's DefinePlugin was replacing process.env
with a large object containing every key on process.env. This led to
code that looked like:

({REACT_APP_SENTRY_DSN: ..., ...}).REACT_APP_SENTRY_DSN

instead of just replacing process.env.REACT_APP_SENTRY_DSN with the
value.

The fix is to build the config to DefinePlugin such that the keys look
like process.env.REACT_APP_SENTRY_DSN. The plugin will then find and
replace occurrences in the output.

[ianschmitz]

Ah i'm with you now. Thanks for clarifying.

[sdemjanenko]

Technically this is a breaking change since some apps might be directly referencing the full process.env object.

[Timer]

I'm not sure we want to make this change. Next.js used to use the process.env.${key} approach instead of replacing the entirety of process.env, and we recently switched to replacing the full process.env object.

There's two primary reasons:

1. Replacing individual properties breaks Object Destructuring. That means users won't be able to write const { REACT_APP_MY_VAR } = process.env and have it work anymore.
2. When instrumenting Create React App to perform server-side rendering, it's dangerous to not replace the entire process.env object as you'd accidentally allow variables without the REACT_APP_ prefix to leak into your server-rendered markup.

The latter point is the most serious.

---

The JavaScript minification step (Terser) should be properly handling replacing the object access and removing all of the unrelated values, so this should not result in a net bundle win.

If this change is actually making the bundled code smaller, it means Terser is misconfigured and we should fix that instead.

[iansu]

Based on @Timer's comments above I think it's best that we leave this behaviour as is and close this PR.

[Timer]

@sdemjanenko Thanks for spending the time to send this PR! If you open an issue describing the specific problem you're trying to solve in detail we'd be more than happy to suggest a better approach or implement a patch.

If the only problem is what you said above (#9171 (comment)), be sure you're not customizing any webpack configuration using react-app-rewired or a similar tool.

[idler8]

我支持这个改动。
当我加载styled-components，并未定义REACT_APP_SC_ATTR和REACT_APP_SC_DISABLE_SPEEDY，我的文件总会包含整个process.env对象，并且无法被压缩，这是一个缺陷。
正常情况下，开发者并不会注意到，REACT_APP_SC_ATTR和REACT_APP_SC_DISABLE_SPEEDY这两个变量。