fix(optimizer): keep NODE_ENV as-is when keepProcessEnv is true

[sapphi-red]

Description

process.env.NODE_ENV was replaced by the optimizer even if keepProcessEnv was set to true. But the optimizer should respect keepProcessEnv to make it consistent with build.

[dario-piotrowicz]

Suggested change

	'process.env.NODE_ENV': environment.config.keepProcessEnv
	? // define process.env.NODE_ENV even for keepProcessEnv === true
	// as esbuild will replace it automatically when `platform` is `'browser'`
	'process.env.NODE_ENV'
	'process.env.NODE_ENV': environment.config.keepProcessEnv
	? 'process.env.NODE_ENV'

is the comment really helpful? 🤔

[sapphi-red]

I think it isn't obvious that we cannot write it like below without a comment.

const define = environment.config.keepProcessEnv
    ? {}
    : {
        'process.env.NODE_ENV': JSON.stringify(
          process.env.NODE_ENV || environment.config.mode,
        ),
      }

[dario-piotrowicz]

I see, yeah that's not obvious 😅

I actually didn't understand this before, I thought that your comment was practically saying that process.env.NODE_ENV can be defined even for keepProcessEnv === true (since esbuild just replaces it) and not that it needs to

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/vite@18899

commit: adcb59e

[sapphi-red]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI on adcb59e: Open

suite	result	latest scheduled
histoire	❌ failure	❌ failure
nuxt	❌ failure	❌ failure
redwoodjs	❌ failure	❌ failure
remix	❌ failure	❌ failure
vike	❌ failure	❌ failure
waku	❌ failure	❌ failure

✅ analogjs, astro, ladle, laravel, marko, previewjs, quasar, qwik, rakkas, storybook, sveltekit, unocss, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest, vuepress