[🐞BUG]: `import.meta.env.PROD` has the wrong value, even if stubbed.

[xsjcTony]

Describe the bug

import.meta.env.PROD will always have the value true.
It will only be false if I stub it to empty string '', but that's definitely not ideal.
Seems there's an issue when converting the string into a boolean (maybe !! is used straight away without checking for special value e.g. string 'false')
I don't have enough time to find the regression commit (it's working as expected before) and the exact logic in the code base, but please get this fixed, thanks🙏

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-ckneff?file=test%2Fbasic.test.ts

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
    Memory: 5.85 GB / 31.69 GB
  Binaries:
    Node: 20.11.0 - C:\Program Files\nodejs\node.EXE
    npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.6.9 - ~\AppData\Local\pnpm\pnpm.EXE
  Browsers:
    Edge: Chromium (123.0.2420.81)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @vitejs/plugin-react-swc: ^3.6.0 => 3.6.0
    @vitest/coverage-istanbul: ^1.5.0 => 1.5.0
    @vitest/coverage-v8: ^1.5.0 => 1.5.0
    @vitest/ui: ^1.5.0 => 1.5.0
    vite: ^5.2.8 => 5.2.8
    vitest: ^1.5.0 => 1.5.0

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[xsjcTony]

I'm aware that in the environment variable it's always with type string, but I'm assuming it should follow Vite's behavior, as Vite is currently handling it to convert to 'false' to false

[sheremet-va]

This is technically a bug because setting a boolean to import.meta.env.PROD directly works correctly:

import.meta.env.PROD = true

[hi-ogawa]

It looks like the default value of import.meta.env.PROD has changed in v1.5.0 and probably this is not intended?

https://stackblitz.com/edit/vitest-dev-vitest-vbi9xu?file=test%2Fbasic.test.ts

Show sample code

import { test, expect } from "vitest";

test("repro", () => {
  // v1.4.0
  [
    false,
    true,
    true,
    "",
    "1",
    "1"
  ];

  // v1.5.0
  [
    true,
    true,
    true,
    "false",
    "true",
    "1"
  ];

  console.log([
    import.meta.env.PROD,
    import.meta.env.DEV,
    import.meta.env.SSR,
    process.env.PROD,
    process.env.DEV,
    process.env.SSR,
  ]);
});

I'm assuming it should follow Vite's behavior, as Vite is currently handling it to convert to 'false' to false

I'm not sure if anything changed on the stubbing behavior (namely assigning "false" becoming true). This is implemented in #3944, so it's probably a different issue.

Btw, what do you mean by "Vite's behavior"? In Vite, import.meta.env is probably a constant and I thought people cannot do assignment.

[xsjcTony]

I might be misleading, but what I mean is Vite seems correctly treating 'false' as false, since process.env is always of type string. But Vitest is not following the same way. But stubbing to 'false', it should return false instead of true

[hi-ogawa]

Vite seems correctly treating 'false' as false

I still fail to understand what/when Vite is "treating" anything in terms of import.meta.env and process.env.
Are you referring to something like PROD=false vite ...? or NODE_ENV=production vite ...? Probably it would be easier if you can provide an example.

[xsjcTony]

Sorry, maybe just forget that, the ultimate goal of this issue is to make sure Vitest treats import.met.env.PROD as false when it stubbed to 'false', since the stub function can only accept string value, which makes sense since process.env are all strings.

[hi-ogawa]

Sorry, maybe just forget that, the ultimate goal of this issue is to make sure Vitest treats import.met.env.PROD as false when it stubbed to 'false', since the stub function can only accept string value, which makes sense since process.env are all strings.

Vitest currently handles this boolean/string problem by making '1' for true and empty string '' for false.

vitest/packages/vitest/src/node/plugins/index.ts

Lines 166 to 169 in 643bed6

	// process.env can have only string values and will cast string on it if we pass other type,
	// so we are making them truthy
	process.env.PROD ??= PROD ? '1' : ''
	process.env.DEV ??= DEV ? '1' : ''

If there is something Vite already does, then Vitest aligning with it would totally makes sense, so that's why I was curious about that. Otherwise, current recommendation would be to stick with our internal representation, namely using an empty string vi.stubEnv('PROD', '') to make import.meta.env.PROD as false.

[xsjcTony]

Yeah stubbing to empty string will definitely work, but that doesn't make sense. Imagine you are setting your env by cli, for example using cross-env, then you will have something like PROD=false, that way 'false' will also be the string version of false, if Vitest treat it as true, it's just so weird. I don't think things should working like that way.

[sheremet-va]

PROD/DEV/SSR/MODE is not something that should be stubbed using PROD=false - this variable is set by Vite itself. If you are using cross-env, then you won't do that. PROD variable is based on the mode: https://vitejs.dev/guide/env-and-mode.html#env-variables

[xsjcTony]

Yeah that makes sense, and I'm absolutely fine to change my test file to vi.stubEnv('PROD', '') instead of vi.stubEnv('PROD', 'false').
But it was working before, so it should be considered as a breaking change. I didn't see this is mentioned in the release note.