Setting NODE_ENV to test has unintended side effects

[steverice]

In 3a38ddf, code was added to default the NODE_ENV variable to test if one is not set (see the CHANGELOG entry). The rationale given was twofold: to be able to specify the Jest preset in Babel config based on the env, and to follow a convention used in Relay.
The former is no longer specified in this way, and the latter is now specified explicitly by Relay so there's no dependency on Jest setting the default.

This came up in #1654 where Babel config for development was not read because Jest would change the environment away from the default — the workaround was to more clearly document this behavior in 968ca38 and b1e7af2.

I believe that library code like Jest changing NODE_ENV (or indeed, any shared environment variables) is an anti-pattern and leads to unexpected behavior, like the Babel issue raised above. Essentially, it's modifying a global variable — see this great explanation on why doing so is considered harmful. If any other libraries are being used that check the value of NODE_ENV, their behavior could change simply by adding Jest — a strong violation of module-based encapsulation.

At this point, there seems to be no requirement that it be defaulted in this manner, given that other NODE_ENV values can be explicitly set and Jest is expected to continue working properly.
If an environment-type value needs to be explicitly set, and to be set to test if nothing else is provided, that should be set in a value local to Jest and not global to the environment — see, for example, this simple env variable in express-js.

I'm happy to create a PR to remove the modifying-NODE_ENV behavior, but I wanted to check that there isn't something intentional or required about this behavior first or if there are strong opinions from the maintainers that they want to keep it this way.

[steverice]

Hi folks, any thoughts on this?

[sdomagala]

+1, user shouldn't be forced to pick ENV for tests.
For example, when combined with config npm package, every test case prints out:

   console.error node_modules/config/lib/config.js:1709
      WARNING: NODE_ENV value of 'test' did not match any deployment config file names.
    console.error node_modules/config/lib/config.js:1710
      WARNING: See https://github.com/lorenwest/node-config/wiki/Strict-Mode

[cpojer]

This is a breaking change but I'm not opposed to it.

[sdomagala]

Maybe there could be flag introduced, like: --no-env so we don't break the compatibility.
Right now you could do:

NODE_ENV= jest

but it doesn't look right to the person that don't know about this issue

[Hurtak]

Also if we decide to make this change, we should introduce some new env variable that will indicate file is running in Jest. Something like NODE_JEST=1.

[kwyn]

So is there a way currently to stop Jest from setting the NODE_ENV environment variable?

[iki]

+1

For example, I found this issue, because we want to use different default env (test-develop) for the API tests, and (test-localhost) for the integration tests.

Btw, for reference, mocha and ava don't do that either: mochajs/mocha#185, avajs/ava#1470.

[adrian-moisa]

From react index.js:

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react.production.min.js');
} else {
  module.exports = require('./cjs/react.development.js');
}

This means that the jest env variable will prevent react from loading the minified version. This might actually add a performance penalty during testing.

[SimenB]

You're free to set it to whatever you want. Jest only sets it to test if it's not set to something already

[ufku]

Jest only sets it to test if it's not set to something already

This is true if it is set as an OS variable. If you are setting it in .env file it is still being overwritten by jest.

[cappslock]

You're free to set it to whatever you want. Jest only sets it to test if it's not set to something already

Does setting it to be a different value affect Jest in some way? If not, why does Jest do this at all?

[cappslock]

Answering my own question, from a quick scan of the code, this looks like the only reference to it, outside of a mention in the configuration docs:

https://github.com/facebook/jest/blob/b7cb5221bb06b6fe63c1a5e725ddbc1aaa82d306/packages/jest-haste-map/src/index.ts#L372

[SimenB]

It makes e.g. process.env.NODE_ENV === 'production' checks be false, and babel config to pick up env: { test: { config } }. Idk about .env files - we check process.env - if it's not there it's not set for the node process, which is what we check

[yan-foto]

You're free to set it to whatever you want. Jest only sets it to test if it's not set to something already

Having NODE_ENV not set is IMO semantically different from not caring if it's set to test! Yes, setting values explicitly might be a good idea, but I don't think that out of all things it should be your testing framework to (forcefully) teach you that lesson.

[mscottx88]

Workaround:

node -r ts-node/register -r ./src/__tests__/setup.ts ./node_modules/jest/bin/jest.js --config \"./jest.config.js\" --testMatch \"<rootDir>/src/**/__tests__/*.integration.spec.ts\" --silent --verbose

Instead of using jest CLI, use Node and inject your own script which defaults process.env.NODE_ENV to what you want.

You could even do jest-only environment stuff here since you could discern the result of NODE_ENV.

process.env.NODE_ENV = process.env.NODE_ENV ?? 'dev';

// set by jest
if (process.env.NODE_ENV === 'test') {
  require('@rbi-ctg/mocks/logger');
}

[ggascoigne]

Somewhat related to this.

I spent quite a while trying to track down errors like this on a couple of projects:

/Users/ggp/dev/git/new-app/jest.setup.js:5
    import "@testing-library/jest-dom/extend-expect";
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

It turned out that this was caused by having NODE_ENV=development set globally. Unsetting it or setting it to test caused things to work as expected. I might expect NODE_ENV to effect how things are compiled, optimization changes etc. for instance, but I honestly never expected that it might change if things could be compiled.

edit to add: this was with Jest 25.4.0 and 26.1.0

[Keyes]

Somewhat related to this.

I spent quite a while trying to track down errors like this on a couple of projects:

/Users/ggp/dev/git/new-app/jest.setup.js:5
    import "@testing-library/jest-dom/extend-expect";
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

It turned out that this was caused by having NODE_ENV=development set globally. Unsetting it or setting it to test caused things to work as expected. I might expect NODE_ENV to effect how things are compiled, optimization changes etc. for instance, but I honestly never expected that it might change if things could be compiled.

edit to add: this was with Jest 25.4.0 and 26.1.0

Had the same issue today. NODE_ENV=development is my default setting for development, to (de-) activate some optimizations. Im my opinion it feels weird if Jest just stops working in that case :/

[x80486]

You're free to set it to whatever you want. Jest only sets it to test if it's not set to something already

Usually developers don't have NODE_ENV set anywhere in the system, because it's better to have that handled by dotenv within the project in question. The trick with this is that dotenv doesn't overwrite values for anything that's already set, so it leads you to NODE_ENV = test all the time. In my case I have this Jiu Jitsu using cross-env all the time in order to set NODE_ENV before Jest...so it would be really nice to not set it to anything or have a setting to bypass it.

[fabioespinosa]

For those using dotenv and wanting to have .env override variables which were set before (like jest's NODE_ENV=test):

dotenv.config({
  // We set override to true to make .env override variables already set (like NODE_ENV which jest sets it to test if it's not already defined)
  override: true,
});

[odd1ty-dev]

Stumbled on this today trying to figure out how to make tests for the first time and honestly, though the feature is not without its merits, I feel like it causes more issues than intended.

If not removing the feature or adding an option to opt out of NODE_ENV being set to "test" by default, at least a more descriptive entry on the "Environment Variables" documentation stating that the "if its not set already" part doesn't apply to dotenv would be nice.

[337Gslime]

Text me 3379367844

…

On Fri, May 13, 2022, 10:54 PM Andres Aldape ***@***.***> wrote: Stumbled on this today trying to figure out how to make tests for the first time and honestly, though the feature is not without its merits, I feel like it causes more issues than intended. If not removing the feature or adding an option to opt out of NODE_ENV being set to "test" by default, at least a more descriptive entry on the "Environment Variables" documentation stating that the "if its not set already" part doesn't apply to dotenv would be nice. — Reply to this email directly, view it on GitHub <#3370 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXSL3Z5MJIXMLMX65G3LPLLVJ4PVZANCNFSM4DJBNXCA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[github-actions]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.