From 445c4f21583334edb37c7b32a1474903a0852b01 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Wed, 3 Apr 2024 19:19:35 +0900 Subject: [PATCH] fix: fix sourcemap when using object as `define` value (#15805) --- packages/vite/src/node/plugins/define.ts | 21 ++++++++++ .../__tests__/js-sourcemap.spec.ts | 39 ++++++++++++++++++- playground/js-sourcemap/index.html | 1 + playground/js-sourcemap/test-ssr-dev.js | 38 ++++++++++++++++++ playground/js-sourcemap/vite.config.js | 6 +++ .../js-sourcemap/with-define-object-ssr.ts | 8 ++++ playground/js-sourcemap/with-define-object.ts | 12 ++++++ 7 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 playground/js-sourcemap/test-ssr-dev.js create mode 100644 playground/js-sourcemap/with-define-object-ssr.ts create mode 100644 playground/js-sourcemap/with-define-object.ts diff --git a/packages/vite/src/node/plugins/define.ts b/packages/vite/src/node/plugins/define.ts index 3eaf2def0efde3..786a1038505c00 100644 --- a/packages/vite/src/node/plugins/define.ts +++ b/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 source entries + // since they would confuse source map remapping/collapsing which expects a single source + if (result.map.includes('= 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]) } diff --git a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts b/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts index f21ace14adab60..1566549293efdb 100644 --- a/playground/js-sourcemap/__tests__/js-sourcemap.spec.ts +++ b/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)), + }) + }) }) diff --git a/playground/js-sourcemap/index.html b/playground/js-sourcemap/index.html index 10afcc6c2d0807..7a91852f4ebe18 100644 --- a/playground/js-sourcemap/index.html +++ b/playground/js-sourcemap/index.html @@ -10,3 +10,4 @@

JS Sourcemap

+ diff --git a/playground/js-sourcemap/test-ssr-dev.js b/playground/js-sourcemap/test-ssr-dev.js new file mode 100644 index 00000000000000..c414f058517283 --- /dev/null +++ b/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() diff --git a/playground/js-sourcemap/vite.config.js b/playground/js-sourcemap/vite.config.js index 0c6d09b8d8906f..f47c89eff07ebf 100644 --- a/playground/js-sourcemap/vite.config.js +++ b/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" }', + }, }) diff --git a/playground/js-sourcemap/with-define-object-ssr.ts b/playground/js-sourcemap/with-define-object-ssr.ts new file mode 100644 index 00000000000000..9ff85230025e2d --- /dev/null +++ b/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)) +} diff --git a/playground/js-sourcemap/with-define-object.ts b/playground/js-sourcemap/with-define-object.ts new file mode 100644 index 00000000000000..5a9f8e2ddd43d9 --- /dev/null +++ b/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()