Unset env variables are not properly embedded

[cauthmann]

Describe the bug

Variables like REACT_APP_FOO are hard-baked into the code if they were present in the environment during build-time. Referencing a variable that did not exist during build time results in rather useless code that cannot be minified:

process.env.REACT_APP_FOO

->

Object({
    NODE_ENV: "production",
    PUBLIC_URL: "",
    WDS_SOCKET_HOST: void 0,
    WDS_SOCKET_PATH: void 0,
    WDS_SOCKET_PORT: void 0
}).REACT_APP_FOO

This will neither cause an error, not is it minified into a constant expression.

Full example further below.

Did you try recovering your dependencies?

Problem is reproducible on a fresh install.
> npm --version: 6.14.4

Which terms did you search for in User Guide?

This behaviour is not documented on
https://create-react-app.dev/docs/adding-custom-environment-variables/

Environment

Environment Info:

  current version of create-react-app: 3.4.1

  [Tested on multiple systems, everything else is irrelevant]

Steps to reproduce

> npx create-react-app test
> cd test
> $EDITOR src/index.js

index.js:

if (process.env.REACT_APP_FOO) {
	console.log('Foo is enabled');
}
else {
	console.log('Foo is disabled');
}

> npm run build
> cat build/static/js/main*.js

main*.js (prettified):

(this.webpackJsonptest = this.webpackJsonptest || []).push([
    [0],
    [function(o, s, e) {
        o.exports = e(1)
    }, function(o, s) {
        Object({
            NODE_ENV: "production",
            PUBLIC_URL: "",
            WDS_SOCKET_HOST: void 0,
            WDS_SOCKET_PATH: void 0,
            WDS_SOCKET_PORT: void 0
        }).REACT_APP_FOO ? console.log("Foo is enabled") : console.log("Foo is disabled")
    }],
    [
        [0, 1]
    ]
]);

Expected behavior

• Preferred: the build emits a warning or an error when referencing a missing variable. It's most likely either a typo in the variable name, or a broken build configuration.
• Alternatively, the variable is replaced with undefined or void 0 or '' or some other sensible default value.
• Alternatively, this behaviour is at least documented with a warning on the corresponding page.

The workaround is to define a default value for every used variable in .env and commit the file, but that doesn't protect against mistyped identifiers in the code.

Actual behavior

See above for actual output. The output contains both '"Foo is enabled"' and "Foo is disabled", even though one of them is dead code.

Reproducible demo

See above.

I am not sure how best to resolve the issue.

Replacing with undefined or '' isn't easy:

• It's not a bug in terser - the code could evaluate to something other than undefined if someone sets Object.prototype.REACT_APP_FOO, so terser is not allowed to minify here. Playing around with terser's repl, I was unable to find a different construct for process.env that would allow minification here.
• webpack's DefinePlugin does not allow regexps for replacements; substituting REACT_APP_.* is not possible.

Emitting an error isn't easy:

• scanning the build output for remaining process.env.REACT_APP_ won't work, and scanning for the whole code snippet above seems really fragile.

This could be solved by moving the replacement logic from DefinePlugin into a custom babel plugin (similar to this one). Considering that cra only supports replacing with strings, and not with arbitrary javascript code, that might work. Is babel guaranteed to be run on every source file, including typescript?

If all else fails, I think it deserves at least a warning in the documentation.

[abumalick]

I pushed a repo that is showing this behavior here:
https://github.com/abumalick/react_app_env_variables

[cauthmann]

@abumalick: That looks like a different issue. I'm not concerned that a list of ENV variables appears in the output - it's likely intended behaviour to account for code like:

let x = process.env;
let foo = x.REACT_APP_FOO;

Those variables shouldn't contain anything sensitive, and I'm sure some code somewhere relies on this working.

What concerns me (and what I consider a bug) is that referencing a variable that wasn't defined does not result in a constant expression. If env vars are used for feature gating, then this will prevent dead code elimination of the gated features.

As you can see above, both "Foo is enabled" and "Foo is disabled" appear in the output; both branches would presumably contain a lot of code, and one of them is dead code.

I've edited the issue to clarify.

[dburrows]

Ran across this today and it's hugely annoying - i'm trying to include some debug code in a specific build but this bug causes it to still be included if the env var isn't set which is the opposite of the expected behaviour.

I would expect this to "fail safe", it's a potential footgun for devs.

[cauthmann]

I believe the most pragmatic way to fix this is to create a new eslint rule, similar to no-process-env (docs, source), except with a whitelist of allowed values. Ideally this should warn in dev, but error on production builds.

Implementing that is on my list of things to do when I get bored, but as you can see it hasn't reached the top yet.

[johnlobster]

Saw the same thing, I think a note in documentation is a good start, and urgent.

In my case, the code made it into the browser code and then threw a type error because process.env.REACT_APP_VAR didn't exist. Not my favorite javascript feature

I agree that there should be a reasonable default, but I can't think of simple way. There has to be a single source of truth somewhere

I think that the underlying thought behind the documentation is confusing

Environment files are used for

1. Passing configuration information,for instance NODE_ENV (create-react-app explicitly passes NODE_ENV)
2. Passing secrets to server code

create_react_app creates browser code. If you need a secret, to access a maps api for instance, there is no way to hide that secret.

So passing variables into create-react-app through the . env file is probably the wrong approach anyway, particularly if you are developing a full stack app in the same directory where you really do need to pass secrets. This is where the documentation is confused. It says to check in the .env file.

[johnlobster]

A solution

• create a "cra-env-defaults" in the package.json
• create a new shell utility, say cra-cross-env that reads "cra-env-defaults" sets those environment variables or according to its arguments

Can be used in package.json scripts

"build:author": "npx cra-cross-env REACT_APP_BUILD_MODE='author' react-scripts build",

I already use cross-env in this way

• requires careful documentation
• no changes to create-react-app code
• backwards compatible

[dimaqq]

I think this also affects 3rd party libraries, e.g. styled-components/styled-components#3166

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[dimaqq]

Here's some recent activity for you, you bot! 😆

[Kudo]

I also had this problem today.
The final solution is pretty much like @johnlobster did.
But simply adding a default REACT_APP_FOO in .env, so that DefinePlugin could replace the variable as constant.
echo "REACT_APP_FOO=false" >> .env

In such case, the if statement should rewrite as
if (process.env.REACT_APP_FOO === 'true') {

Details

1. TerserPlugin's Dead Code Elimination could only eliminate simple code block:

        const foo = 'true';
        if (foo !== 'true')
          console.log('This code should be removed');

But not for this:

        const foo = {};
        foo.bar = 'true';
        if (foo.bar !== 'true')
          console.log('This code will NOT be removed');

So we should try to make the code block like the previous one.

2. DefinePlugin will replace some variables as constants.
   From if (process.env.REACT_FOO === 'true') to if ('false' === 'true')

3. Create React App will pass REACT_APP_* from .env to DefinePlugin.

[johnlobster]

I have updated the environment variables documentation

• re organization and summary
• added note in several places that REACT_APP_ vars must always be defined
• added use of cross-env for setting environment variables

Please could you take a look ? I would really appreciate some feedback. I moved quite a lot of the documentation around because I thought it had been added to in an inconsistent way

Clearly, this doesn't close this issue, but should stop people falling into the same hole that we did

If you wish to edit my version, I can add you as collaborator to my fork

My fork is at
https://github.com/johnlobster/create-react-app/tree/jw-env-vars
Edited file at
https://github.com/johnlobster/create-react-app/blob/jw-env-vars/docusaurus/docs/adding-custom-environment-variables.md

I deployed the docusaurus build on Netlify and you can find the built documentation at
https://eloquent-hermann-c200a6.netlify.app/docs/adding-custom-environment-variables

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

[stale]

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

[idler8]

In a server environment, process.env will be used as a whole
In a browser environment, process.env.${key} is used alone for part of the logic
We should distinguish the usage of this part in two contexts

[xMartin]

Still an issue with the latest version.