fix: reset NODE_ENV on production build end

[bholmesdev]

Description

Resolves #9057

• Reset NODE_ENV to its initial value when a production build ends. This prevents NODE_ENV from leaking to subsequent Vite runs.

Additional context

It seems this issue was introduced by #8218 (cc @sapphi-red and @bluwy). I'd suggest investigating other options to flip isProduction beyond environment variables, since they can easily leak between Vite builds (ex. the parallel CI suites we use at Astro). Perhaps a new CLI flag, or exposing isProduction as an inline config option separate from mode?

---

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.

[netlify]

✅ Deploy Preview for vite-docs-main canceled.

Name	Link
🔨 Latest commit	4cfe9ce
🔍 Latest deploy log	https://app.netlify.com/sites/vite-docs-main/deploys/62cdf64aa412230008f2facc

[patak-dev]

@sapphi-red, @bluwy, let me know what you think here. This PR is small enough that we could merge it before the release tomorrow.
We could also consider reverting #8218, but it is not clear to me if that is the right path forward. We may need to review the NODE_ENV/mode interaction for v4 again.

[patak-dev]

Interesting looks like some tests are checking post-build. I think these tests could be skipped during build 🤔

[bluwy]

I think we may want to revise NODE_ENV handling in the future in general. In SvelteKit side we recently had sveltejs/kit#5456 too. It's not great now as the heuristic for production-like env in staging mode depends on after the config is resolved. Plugins might already need process.env.NODE_ENV on init, hence it's set at here.

I'm not really sure about resetting it like in this PR. It feels like it would open up a can of worms over an existing dilemma. Maybe a better way is to hard-set process.env.NODE_ENV as development when the command is serve? If we want to be extra cautious, we can do so only when process.env.NODE_ENV is production, because it's unlikely that production would ever work in dev mode.

In the future, we should probably avoid setting process.env.NODE_ENV in resolveConfig at all and let users specify that 🤔

[sapphi-red]

I think we may need to revise NODE_ENV handling for v4 in future.

As long as we set NODE_ENV, it is not possible to make it work parallelly. But it is needed to set NODE_ENV to make it work out of the box (Maybe this is not needed?).

But for now, I think we should focus on making the change less breaking. I think going with this PR or reverting #8218 is better. If we are going to change how to handle NODE_ENV in future, maybe it is better to revert #8218?

About this PR's test, I think replacing process.env.NODE_ENV with 'production' in test files will fix the test.

[patak-dev]

@sapphi-red you have a point, we shoupd avoid breaking twice. I think reverting #8218 may be the best. It should only break the issues it targeted, no? I'm out for a bit, would you like to send a PR and we check with ecosystem ci? Better to send it in the main vite repo instead of your fork so we can test before merging

[sapphi-red]

@patak-dev Yes, I think it only breaks the targeted issue and there is a workaround (set VITE_USER_NODE_ENV or use .env). I'll create it 👍

[bluwy]

I'm in the boat of not reverting #8218. Ideally we should respect user's NODE_ENV and it follows the idea in my comment above. I'd even say that the linked issue was a bug in Vite 2 and fixed in Vite 3 because we're running the dev server when process.env.NODE_ENV is production. I can implement my comment above and we could see how it works with the ecosystem ci

[patak-dev]

Ok, we could try both and run ecosystem-ci in both PR. I think we could actually release and then fix this in a patch with more time. Whatever we do now, we are chosing what edge cases to break

[sapphi-red]

On second thought, I now think @bluwy's idea (#9066) is better.
If someone want's to use a NODE_ENV which differs from the derived one from mode, they will/should set NODE_ENV and NODE_ENV should be 'development' during dev in any mode.

[bholmesdev]

Yep, @bluwy's fix should work for us! Happy to close this 👍