Custom environment variables: unset variables cause unexpected minification

[mfonsen]

Can you reproduce the problem with latest npm?

yes (with latest yarn)

Description

If a custom environment variable is not set minification will not strip out dead code related to it.

This could cause unwanted development assets to go into the production build if the environment variables are not properly set.

If this is works as planned perhaps I could try to create a PR to mention this in the documentation.

Please consider the following example when running yarn run build:

if (process.env.REACT_APP_FOO === 'bar') {
  console.log('REACT_APP_FOO: bar')
} else {
  console.log('REACT_APP_FOO: something else');
}

Expected behavior

string REACT_APP_FOO: bar is not included in the bundle.

Actual behavior

REACT_APP_FOO: bar is included in the bundle.

Environment

1. npm ls react-scripts (if you haven’t ejected): └── react-scripts@0.9.4

2. node -v: v6.10.0

3. npm -v: 3.10.10

4. Operating system: macOS

5. Browser and version: Chrome 56.0.2924.87 (64-bit)

Reproducible Demo

Repo: https://github.com/mfonsen/create-react-app-undefined-env-vars
Steps:

cd my-app
yarn
yarn run build
cat build/static/js/main.*.js | grep -Eo "REACT_APP_FOO: bar"
# returns REACT_APP_FOO: bar
REACT_APP_FOO="foobar" yarn run build
# does not return anything

[Timer]

This looks suspicious...

"bar" === {
    NODE_ENV: "production",
    PUBLIC_URL: ""
}.REACT_APP_FOO ? console.log("REACT_APP_FOO: bar") : console.log("REACT_APP_FOO: something else")

Looks like webpack doesn't inline undefined. Could you please open an issue with them and link it back here?
This might be the intended behavior.

[mfonsen]

If I understood this right Webpack does not do dead code elimination on it's own. Would this be a missing feature in UglifyJS (used in webpack config)?

DefinePlugin does not seem to support auto discovery.

I could create an issue for UglifyJS. Should I also create PR to mention this limitation in the docs?

[Timer]

Webpack should inline variables like this, which then should be eliminated by Uglify.
I do not believe our config is missing a option, it might be.
I think we should create issues for Uglify and webpack and see who bites.

Uglify could note that there's no REACT_APP_FOO in given object.
Webpack could inline undefined instead of REACT_APP_FOO.

Best to probably start with Uglify, though. Could you please link the issue here if you create one?

[Timer]

@mfonsen is this still a problem?

[gaearon]

I think this is the expected behavior.

For differentiating between production and development builds, I recommend checking process.env.NODE_ENV which is hardcoded and will always be specified.

For custom environment variables, you can employ a trick like:

if (process.env.REACT_APP_FOO === 'bar') {
  console.log('REACT_APP_FOO: bar')
} else if (process.env.REACT_APP_FOO === 'baz') {
  console.log('REACT_APP_FOO: baz');
} else {
  throw new Error('You forgot to specify REACT_APP_FOO in the build.');
}

Then at least you’d get a completely crashing bundle and presumably would find the mistake soon enough.

But technically accessing process.env.WHATEVER should be safe so IMO we shouldn’t introduce non-obvious semantics there (like suddenly failing if it is missing).

[WickyNilliams]

I've just noticed this same issue. If you reference an unset env var like this:

console.log(process.env.I_DO_NOT_EXIST);

Then the output of the build from CRA is:

console.log(Object({NODE_ENV:"production",PUBLIC_URL:""}).I_DO_NOT_EXIST);

I have tested with just webpack (i.e. no CRA), and it outputs (as expected) this:

console.log(process.env.I_DO_NOT_EXIST);

When encountering an unknown env var, webpack will leave the code verbatim, but define process.env = {} somewhere else, so the code evaluates to undefined.

I'm not sure what is causing this in CRA, but it should be fixed. It's wasteful to output the whole process.env object just to get something that evaluates to undefined, you lose some dead code elimination as originally stated here, and it is also leaking some information in the bundle (though env var names are arguably insignificant, they could unwittingly reveal something you would not want exposed).

@gaearon should i create a new issue for this? It's the underlying cause of what was originally reported here

[WickyNilliams]

Here's a gist which will show raw webpack doesn't behave the same as CRA https://gist.github.com/WickyNilliams/992292328366f6fcfe0df3862051ade8