fix!: make NODE_ENV more predictable

[benmccann]

Description

Make the value of NODE_ENV more predictable

Additional context

This was discussed in #9274

The one intricacy in implementing was related to running tests. When the first test runs Vite will set a value for NODE_ENV and then Vite will no longer reset the value on subsequent tests, so I had to handle that in the tests themselves. That's unlikely to affect our users though, so I think this is still a good trade-off. E.g. when Playwright runs the Vite server in the tests, it will do so in a sub-process, which I think would isolate each server process from one another.

There was a comment in one of the tests about plugin-legacy that I deleted as it appears to be outdated. Had it not been outdated, I think fixing plugin-legacy would have been the better approach than making Vite adhere to the behavior of a single plugin, but that appears no longer relevant.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI: Open

suite	result
astro	✅ success
histoire	❌ failure
iles	❌ failure
ladle	✅ success
laravel	❌ failure
marko	✅ success
nuxt-framework	❌ failure
rakkas	⏹️ cancelled
svelte	❌ failure
vite-plugin-ssr	❌ failure
vite-setup-catalogue	✅ success
vitepress	✅ success
vitest	❌ failure
windicss	✅ success

[patak-dev]

I think we should move forward with this one to be able to test it during the alpha window. @bluwy feel free to merge after you review it.

[bluwy]

Thanks for making the changes! This LGTM too.

Looks like overall this implements #9274, except:

• Doesn't remove NODE_ENV support in .env files. Maybe ok for now to minimize the breaking change, but it does mean staging mode still builds in development NODE_ENV by default.
• NODE_ENV is still mutated in resolveConfig. Maybe it's also ok as the rule is simpler now.

Merging since we can improve the above later.

[benmccann]

Doesn't remove NODE_ENV support in .env files. Maybe ok for now to minimize the breaking change, but it does mean staging mode still builds in development NODE_ENV by default.

@bluwy can you clarify? I believe the default would be production:

vite/packages/vite/src/node/build.ts

Line 459 in 7d24b5f

'production'

If you don't define NODE_ENV in an .env file I think it shouldn't have any effect (I haven't tested though). Is there a reason we don't want users to be able to set it there?

[bluwy]

Ah you're right I missed that. Nevermind then 😄

If you don't define NODE_ENV in an .env file I think it shouldn't have any effect (I haven't tested though). Is there a reason we don't want users to be able to set it there?

It's mostly the concern that resolveConfig has another way to mutate NODE_ENV that would be nice to avoid

vite/packages/vite/src/node/config.ts

Lines 499 to 505 in 7d24b5f

	const isProduction =
	(process.env.NODE_ENV || process.env.VITE_USER_NODE_ENV || mode) ===
	'production'
	if (isProduction) {
	// in case default mode was not production and is overwritten
	process.env.NODE_ENV = 'production'
	}

Now that I think about it, since builds are production NODE_ENV by default, the above code doesn't allow someone to build in development NODE_ENV. I can fix that with a follow up PR.