Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
22 changes: 22 additions & 0 deletions packages/vite/src/node/plugins/define.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -157,7 +158,28 @@ 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)))
}
}

for (const marker in replacementMarkers) {
// TODO: this could also break sourcemap?
result.code = result.code.replaceAll(marker, replacementMarkers[marker])
hi-ogawa marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
39 changes: 38 additions & 1 deletion playground/js-sourcemap/__tests__/js-sourcemap.spec.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -156,4 +158,39 @@ describe.runIf(isBuild)('build tests', () => {
/^\/\/# sourceMappingURL=after-preload-dynamic.*\.js\.map$/,
)
})

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)),
})
})
})
1 change: 1 addition & 0 deletions playground/js-sourcemap/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ <h1>JS Sourcemap</h1>
<script type="module" src="./after-preload-dynamic.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>
38 changes: 38 additions & 0 deletions playground/js-sourcemap/test-ssr-dev.js
Original file line number Diff line number Diff line change
@@ -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()
6 changes: 6 additions & 0 deletions playground/js-sourcemap/vite.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,14 @@ export default defineConfig({
if (name.includes('after-preload-dynamic')) {
return 'after-preload-dynamic'
}
if (name.includes('with-define-object')) {
return 'with-define-object'
}
},
},
},
},
define: {
__testDefineObject: '{ "hello": "test" }',
},
})
8 changes: 8 additions & 0 deletions playground/js-sourcemap/with-define-object-ssr.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export function error() {
errorInner()
}

function errorInner() {
// @ts-expect-error "define"
throw new Error('with-define-object: ' + JSON.stringify(__testDefineObject))
}
12 changes: 12 additions & 0 deletions playground/js-sourcemap/with-define-object.ts
Original file line number Diff line number Diff line change
@@ -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()