Skip to content

Commit

Permalink
fix: fix sourcemap when using object as define value (#15805)
Browse files Browse the repository at this point in the history
  • Loading branch information
hi-ogawa authored and patak-dev committed Apr 5, 2024
1 parent c7efec4 commit 9699ba3
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 1 deletion.
21 changes: 21 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,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])
}
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 @@ -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)),
})
})
})
1 change: 1 addition & 0 deletions playground/js-sourcemap/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
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 @@ -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')) {
Expand All @@ -30,4 +33,7 @@ export default defineConfig({
},
},
},
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()

0 comments on commit 9699ba3

Please sign in to comment.