fix: fix sourcemap when using object as define value (#15805)

Author: hi-ogawa

packages/vite/src/node/plugins/define.ts

@@ -1,4 +1,5 @@
 import { transform } from 'esbuild'
+import { TraceMap, decodedMap, encodedMap } from '@jridgewell/trace-mapping'
 import type { ResolvedConfig } from '../config'
 import type { Plugin } from '../plugin'
 import { escapeRegex, getHash } from '../utils'
@@ -157,6 +158,26 @@ export async function replaceDefine(
     sourcemap: config.command === 'build' ? !!config.build.sourcemap : true,
   })
 
+  // remove esbuild's <define:...> source entries
+  // since they would confuse source map remapping/collapsing which expects a single source
+  if (result.map.includes('<define:')) {
+    const originalMap = new TraceMap(result.map)
+    if (originalMap.sources.length >= 2) {
+      const sourceIndex = originalMap.sources.indexOf(id)
+      const decoded = decodedMap(originalMap)
+      decoded.sources = [id]
+      decoded.mappings = decoded.mappings.map((segments) =>
+        segments.filter((segment) => {
+          // modify and filter
+          const index = segment[1]
+          segment[1] = 0
+          return index === sourceIndex
+        }),
+      )
+      result.map = JSON.stringify(encodedMap(new TraceMap(decoded as any)))
+    }
+  }
+
   for (const marker in replacementMarkers) {
     result.code = result.code.replaceAll(marker, replacementMarkers[marker])
   }

playground/js-sourcemap/__tests__/js-sourcemap.spec.ts

@@ -1,4 +1,6 @@
-import { URL } from 'node:url'
+import { URL, fileURLToPath } from 'node:url'
+import { promisify } from 'node:util'
+import { execFile } from 'node:child_process'
 import { describe, expect, test } from 'vitest'
 import { mapFileCommentRegex } from 'convert-source-map'
 import { commentSourceMap } from '../foo-with-sourcemap-plugin'
@@ -170,4 +172,39 @@ describe.runIf(isBuild)('build tests', () => {
     const js = findAssetFile(/after-preload-dynamic-no-dep-[-\w]{8}\.js$/)
     expect(js).not.toMatch(/__vite__mapDeps/)
   })
+
+  test('sourcemap is correct when using object as "define" value', async () => {
+    const map = findAssetFile(/with-define-object.*\.js\.map/)
+    expect(formatSourcemapForSnapshot(JSON.parse(map))).toMatchInlineSnapshot(`
+      {
+        "mappings": "qBAEA,SAASA,GAAO,CACJC,GACZ,CAEA,SAASA,GAAY,CAEX,QAAA,MAAM,qBAAsBC,CAAkB,CACxD,CAEAF,EAAK",
+        "sources": [
+          "../../with-define-object.ts",
+        ],
+        "sourcesContent": [
+          "// test complicated stack since broken sourcemap
+      // might still look correct with a simple case
+      function main() {
+        mainInner()
+      }
+
+      function mainInner() {
+        // @ts-expect-error "define"
+        console.trace('with-define-object', __testDefineObject)
+      }
+
+      main()
+      ",
+        ],
+        "version": 3,
+      }
+    `)
+  })
+
+  test('correct sourcemap during ssr dev when using object as "define" value', async () => {
+    const execFileAsync = promisify(execFile)
+    await execFileAsync('node', ['test-ssr-dev.js'], {
+      cwd: fileURLToPath(new URL('..', import.meta.url)),
+    })
+  })
 })

playground/js-sourcemap/index.html

@@ -10,3 +10,4 @@ <h1>JS Sourcemap</h1>
 <script type="module" src="./after-preload-dynamic-no-dep.js"></script>
 <script type="module" src="./with-multiline-import.ts"></script>
 <script type="module" src="./zoo.js"></script>
+<script type="module" src="./with-define-object.ts"></script>

playground/js-sourcemap/test-ssr-dev.js

@@ -0,0 +1,38 @@
+import assert from 'node:assert'
+import { fileURLToPath } from 'node:url'
+import { createServer } from 'vite'
+
+async function runTest() {
+  const server = await createServer({
+    root: fileURLToPath(new URL('.', import.meta.url)),
+    configFile: false,
+    optimizeDeps: {
+      noDiscovery: true,
+    },
+    server: {
+      middlewareMode: true,
+      hmr: false,
+    },
+    define: {
+      __testDefineObject: '{ "hello": "test" }',
+    },
+  })
+  const mod = await server.ssrLoadModule('/with-define-object-ssr.ts')
+  const error = await getError(() => mod.error())
+  server.ssrFixStacktrace(error)
+  assert.match(error.stack, /at errorInner (.*with-define-object-ssr.ts:7:9)/)
+  await server.close()
+}
+
+async function getError(f) {
+  let error
+  try {
+    await f()
+  } catch (e) {
+    error = e
+  }
+  assert.ok(error)
+  return error
+}
+
+runTest()

playground/js-sourcemap/vite.config.js

@@ -21,6 +21,9 @@ export default defineConfig({
           if (name.endsWith('after-preload-dynamic-no-dep.js')) {
             return 'after-preload-dynamic-no-dep'
           }
+          if (name.includes('with-define-object')) {
+            return 'with-define-object'
+          }
         },
         banner(chunk) {
           if (chunk.name.endsWith('after-preload-dynamic-hashbang')) {
@@ -30,4 +33,7 @@ export default defineConfig({
       },
     },
   },
+  define: {
+    __testDefineObject: '{ "hello": "test" }',
+  },
 })

playground/js-sourcemap/with-define-object-ssr.ts

@@ -0,0 +1,8 @@
+export function error() {
+  errorInner()
+}
+
+function errorInner() {
+  // @ts-expect-error "define"
+  throw new Error('with-define-object: ' + JSON.stringify(__testDefineObject))
+}

playground/js-sourcemap/with-define-object.ts

@@ -0,0 +1,12 @@
+// test complicated stack since broken sourcemap
+// might still look correct with a simple case
+function main() {
+  mainInner()
+}
+
+function mainInner() {
+  // @ts-expect-error "define"
+  console.trace('with-define-object', __testDefineObject)
+}
+
+main()