[Beta] Dotenv expand is failing the build - consider reverting processing process. env

[AlexandreBonaventure]

Describe the bug

Hi,
One of the breaking changes in vite late alpha is preventing us to run the beta:
d5fe92c

Parsing / expanding with dotenv process.env variable is highly disruptive and certainly can lead to unexpected behaviour. Here we are testing Warp terminal, it is injecting this WARP_PS1 containing $. I can see how this could randomly bite other users and some side effects might be hard to debug (if it actually expand some variables from process.env)

Please consider reverting this change, this issue was already reported in the past but now that process.env is parsed we don't have any control on these vars for escaping the $ so the workaround cannot be applied see #6858 (comment)

Reproduction

Not relevant

Steps to reproduce

No response

System Info

Not relevant

Used Package Manager

npm

Logs

No response

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.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• 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.

[bluwy]

process.env shouldn't be expanded. Expand should only work in .env files, so this is a bug to me. It seems to happen because of

vite/packages/vite/src/node/env.ts

Lines 40 to 43 in 8b83089

	parsed: {
	...(process.env as any),
	...parsed,
	},

Seems like we might need to fork dotenv-expand, or have expand not use process.env until a minor 🤔 cc @Dunqing @sapphi-red

[Dunqing]

I think we still need expand process.env, which is intentional and allows us to use ci's environment variables in env files, which is very useful

[Dunqing]

Seems like we might need to fork dotenv-expand

I prefer to workaround the bug in this way

[sapphi-red]

Maybe we could escape process.env before passing to dotenv-expand? That way I think we could avoid forking.

[bluwy]

I think we still need expand process.env, which is intentional and allows us to use ci's environment variables in env files, which is very useful

Yeah we should expand .env using process.env, but we shouldn't expand the process.env itself.

Maybe we could escape process.env before passing to dotenv-expand? That way I think we could avoid forking.

I think that brings perf implications compared to forking it. Maybe looping in @motdotla if this could be implemented upstream? Summary so far is that we want to read process.env when expanding, but we don't want to write back to process.env (skips this code).

[patak-dev]

Maybe we can have a patch in the vite monorepo instead of forking until this is implemented upstream?

[Dunqing]

It is worth mentioning that If we fork dotenv-expand, we can also fix TypeError: Cannot read properties of undefined (reading 'split') and remove #11141 .