fix: avoid replacing defines and NODE_ENV in optimized deps (fix #8593) (#8606)

Author: patak-dev

packages/vite/src/node/optimizer/index.ts

@@ -385,6 +385,7 @@ export async function runOptimizeDeps(
   resolvedConfig: ResolvedConfig,
   depsInfo: Record<string, OptimizedDepInfo>
 ): Promise<DepOptimizationResult> {
+  const isBuild = resolvedConfig.command === 'build'
   const config: ResolvedConfig = {
     ...resolvedConfig,
     command: 'build'
@@ -471,30 +472,24 @@ export async function runOptimizeDeps(
     flatIdToExports[flatId] = exportsData
   }
 
-  const define: Record<string, string> = {
-    'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV || config.mode)
-  }
-  for (const key in config.define) {
-    const value = config.define[key]
-    define[key] = typeof value === 'string' ? value : JSON.stringify(value)
-  }
-
   const start = performance.now()
 
   const result = await build({
     absWorkingDir: process.cwd(),
     entryPoints: Object.keys(flatIdDeps),
     bundle: true,
+    // Ensure resolution is handled by esbuildDepPlugin and
+    // avoid replacing `process.env.NODE_ENV` for 'browser'
+    platform: 'neutral',
     format: 'esm',
     target: config.build.target || undefined,
     external: config.optimizeDeps?.exclude,
     logLevel: 'error',
     splitting: true,
     sourcemap: true,
     outdir: processingCacheDir,
-    ignoreAnnotations: resolvedConfig.command !== 'build',
+    ignoreAnnotations: !isBuild,
     metafile: true,
-    define,
     plugins: [
       ...plugins,
       esbuildDepPlugin(flatIdDeps, flatIdToExports, config)
@@ -887,7 +882,6 @@ export function getDepHash(config: ResolvedConfig): string {
     {
       mode: process.env.NODE_ENV || config.mode,
       root: config.root,
-      define: config.define,
       resolve: config.resolve,
       buildTarget: config.build.target,
       assetsInclude: config.assetsInclude,

playground/define/__tests__/define.spec.ts

@@ -40,4 +40,7 @@ test('string', async () => {
   // html would't need to define replacement
   expect(await page.textContent('.exp-define')).toBe('__EXP__')
   expect(await page.textContent('.import-json')).toBe('__EXP__')
+  expect(await page.textContent('.define-in-dep')).toBe(
+    defines.__STRINGIFIED_OBJ__
+  )
 })

playground/define/commonjs-dep/index.js

@@ -0,0 +1 @@
+module.exports = { defined: __STRINGIFIED_OBJ__ }

playground/define/commonjs-dep/package.json

@@ -0,0 +1,6 @@
+{
+  "name": "commonjs-dep",
+  "private": true,
+  "version": "1.0.0",
+  "type": "commonjs"
+}

playground/define/index.html

@@ -16,6 +16,7 @@ <h1>Define</h1>
 <p>no identifier substring: <span class="no-identifier-substring"></span></p>
 <p>define variable in html: <code class="exp-define">__EXP__</code></p>
 <p>import json: <code class="import-json"></code></p>
+<p>define in dep: <code class="define-in-dep"></code></p>
 
 <script type="module">
   const __VAR_NAME__ = true // ensure define doesn't replace var name
@@ -50,6 +51,9 @@ <h1>Define</h1>
   function text(el, text) {
     document.querySelector(el).textContent = text
   }
+
+  import { defined } from 'dep'
+  text('.define-in-dep', JSON.stringify(defined))
 </script>
 
 <style>

playground/define/package.json

@@ -7,5 +7,8 @@
     "build": "vite build",
     "debug": "node --inspect-brk ../../packages/vite/bin/vite",
     "preview": "vite preview"
+  },
+  "dependencies": {
+    "dep": "file:./commonjs-dep"
   }
 }

playground/define/vite.config.js

@@ -19,6 +19,7 @@ module.exports = {
     'process.env.SOMEVAR': '"SOMEVAR"',
     $DOLLAR: 456,
     ÖUNICODE_LETTERɵ: 789,
-    __VAR_NAME__: false
+    __VAR_NAME__: false,
+    __STRINGIFIED_OBJ__: JSON.stringify({ foo: true })
   }
 }