fix(plugin-legacy): prevent global process.env.NODE_ENV mutation

packages/plugin-legacy/src/index.ts

@@ -205,6 +205,7 @@ function viteLegacyPlugin(options: Options = {}): Plugin[] {
             modernPolyfills
           )
         await buildPolyfillChunk(
+          config.mode,
           modernPolyfills,
           bundle,
           facadeToModernPolyfillMap,
@@ -237,6 +238,7 @@ function viteLegacyPlugin(options: Options = {}): Plugin[] {
           )
 
         await buildPolyfillChunk(
+          config.mode,
           legacyPolyfills,
           bundle,
           facadeToLegacyPolyfillMap,
@@ -615,6 +617,7 @@ function createBabelPresetEnvOptions(
 }
 
 async function buildPolyfillChunk(
+  mode: string,
   imports: Set<string>,
   bundle: OutputBundle,
   facadeToChunkMap: Map<string, string>,
@@ -626,6 +629,16 @@ async function buildPolyfillChunk(
   let { minify, assetsDir } = buildOptions
   minify = minify ? 'terser' : false
   const res = await build({
+    // Because Vite's build function defaults to calling resolveConfig with defaultMode = 'production'
+    // And as Vite's resolveConfig function will overwrite import.env.NODE_ENV if === 'production'
+    // We must pass the current value of process.env.NODE_ENV as mode, to prevent it from being overwritten
+    // by a default value, even if it has been previously set to something else by the user
+    // This problem manifests itself when programmatically running Vite's build function twice - client & server build
+    // with the legacy plugin enabled - it will first build the client version, then build the client fallback, and in the process,
+    // overwrite process.env.NODE_ENV. From there on, the server build will have the correct config.mode value, but the global
+    // import.env.NODE_ENV will assume the wrong value as mutated by resolveConfig
+    // If process.env.NODE_ENV is undefined, the mode and mutation will assume the default value and should not break people's projects
+    mode,
     // so that everything is resolved from here
     root: path.dirname(fileURLToPath(import.meta.url)),
     configFile: false,

playground/legacy/__tests__/client-and-ssr/client-legacy-ssr-sequential-builds.spec.ts

@@ -0,0 +1,17 @@
+import { describe, expect, test } from 'vitest'
+import { port } from './serve'
+import { isBuild, page } from '~utils'
+
+const url = `http://localhost:${port}`
+
+describe.runIf(isBuild)('client-legacy-ssr-sequential-builds', () => {
+  test('should work', async () => {
+    await page.goto(url)
+    expect(await page.textContent('#app')).toMatch('Hello')
+  })
+
+  test('import.meta.env.MODE', async () => {
+    // SSR build is always modern
+    expect(await page.textContent('#mode')).toMatch('test')
+  })
+})

playground/legacy/__tests__/client-and-ssr/serve.ts

@@ -0,0 +1,68 @@
+// this is automatically detected by playground/vitestSetup.ts and will replace
+// the default e2e test serve behavior
+import path from 'node:path'
+import { ports, rootDir } from '~utils'
+
+export const port = ports['legacy/client-and-ssr']
+
+export async function serve(): Promise<{ close(): Promise<void> }> {
+  const { build } = await import('vite')
+
+  // In a CLI app it is possible that you may run `build` several times one after another
+  // For example, you may want to override an option specifically for the SSR build
+  // And you may have a CLI app built for that purpose to make a more concise API
+  // An unexpected behaviour is for the plugin-legacy to override the process.env.NODE_ENV value
+  // And any build after the first client build that called plugin-legacy will misbehave and
+  // build with process.env.NODE_ENV=production, rather than your CLI's env: NODE_ENV=myWhateverEnv my-cli-app build
+  // The issue is with plugin-legacy's index.ts file not explicitly passing mode: process.env.NODE_ENV to vite's build function
+  // This causes vite to call resolveConfig with defaultMode = 'production' and mutate process.env.NODE_ENV to 'production'
+
+  await build({
+    mode: process.env.NODE_ENV,
+    root: rootDir,
+    logLevel: 'silent',
+    build: {
+      target: 'esnext',
+      outDir: 'dist/client'
+    }
+  })
+
+  await build({
+    mode: process.env.NODE_ENV,
+    root: rootDir,
+    logLevel: 'silent',
+    build: {
+      target: 'esnext',
+      ssr: 'entry-server-sequential.js',
+      outDir: 'dist/server'
+    }
+  })
+
+  const { default: express } = await import('express')
+  const app = express()
+
+  app.use('/', async (_req, res) => {
+    const { render } = await import(
+      path.resolve(rootDir, './dist/server/entry-server-sequential.mjs')
+    )
+    const html = await render()
+    res.status(200).set({ 'Content-Type': 'text/html' }).end(html)
+  })
+
+  return new Promise((resolve, reject) => {
+    try {
+      const server = app.listen(port, () => {
+        resolve({
+          // for test teardown
+          async close() {
+            await new Promise((resolve) => {
+              server.close(resolve)
+            })
+          }
+        })
+      })
+    } catch (e) {
+      reject(e)
+    }
+  })
+}

playground/legacy/entry-server-sequential.js

@@ -0,0 +1,7 @@
+// This counts as 'server-side' rendering, yes?
+export async function render() {
+  return /* html */ `
+    <div id="app">Hello</div>
+    <div id="mode">${import.meta.env.MODE}</div>
+  `
+}

playground/test-utils.ts

@@ -21,6 +21,7 @@ export const ports = {
   'legacy/ssr': 9520,
   lib: 9521,
   'optimize-missing-deps': 9522,
+  'legacy/client-and-ssr': 9523,
   'ssr-deps': 9600,
   'ssr-html': 9601,
   'ssr-pug': 9602,