From a9e3a8ce18ea0332835e5351c5af37e723026705 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 21 Feb 2025 15:00:34 +0100 Subject: [PATCH 1/4] fix(browser): improve source map handling for bundled files --- packages/browser/src/node/projectParent.ts | 50 +++++++++++++++++--- packages/utils/src/source-map.ts | 54 +++++++++++++++------- 2 files changed, 81 insertions(+), 23 deletions(-) diff --git a/packages/browser/src/node/projectParent.ts b/packages/browser/src/node/projectParent.ts index d189e5122a8f..4c6298b31f7d 100644 --- a/packages/browser/src/node/projectParent.ts +++ b/packages/browser/src/node/projectParent.ts @@ -10,9 +10,10 @@ import type { Vitest, } from 'vitest/node' import type { BrowserServerState } from './state' +import { readFileSync } from 'node:fs' import { readFile } from 'node:fs/promises' import { parseErrorStacktrace, parseStacktrace, type StackTraceParserOptions } from '@vitest/utils/source-map' -import { join, resolve } from 'pathe' +import { dirname, join, resolve } from 'pathe' import { BrowserServerCDPHandler } from './cdp' import builtinCommands from './commands/index' import { distRoot } from './constants' @@ -41,6 +42,9 @@ export class ParentBrowserProject { public config: ResolvedConfig + // cache for non-vite source maps + private sourceMapCache = new Map() + constructor( public project: TestProject, public base: string, @@ -50,17 +54,33 @@ export class ParentBrowserProject { this.stackTraceOptions = { frameFilter: project.config.onStackTrace, getSourceMap: (id) => { + if (this.sourceMapCache.has(id)) { + return this.sourceMapCache.get(id) + } + const result = this.vite.moduleGraph.getModuleById(id)?.transformResult + if (result && !result.map) { + const sourceMapUrl = this.retrieveSourceMapURL(result.code) + if (!sourceMapUrl) { + return null + } + const filepathDir = dirname(id) + const sourceMapPath = resolve(filepathDir, sourceMapUrl) + const map = JSON.parse(readFileSync(sourceMapPath, 'utf-8')) + this.sourceMapCache.set(id, map) + return map + } return result?.map }, - getFileName: (id) => { + getUrlId: (id) => { const mod = this.vite.moduleGraph.getModuleById(id) - if (mod?.file) { - return mod.file + if (mod) { + return id } - const modUrl = this.vite.moduleGraph.urlToModuleMap.get(id) - if (modUrl?.file) { - return modUrl.file + const resolvedId = resolve(project.config.root, id.slice(1)) + const modUrl = this.vite.moduleGraph.getModuleById(resolvedId) + if (modUrl) { + return resolvedId } return id }, @@ -235,4 +255,20 @@ export class ParentBrowserProject { const decodedTestFile = decodeURIComponent(testFile) return { sessionId, testFile: decodedTestFile } } + + private retrieveSourceMapURL(source: string): string | null { + const re + = /\/\/[@#]\s*sourceMappingURL=([^\s'"]+)\s*$|\/\*[@#]\s*sourceMappingURL=[^\s*'"]+\s*\*\/\s*$/gm + // Keep executing the search to find the *last* sourceMappingURL to avoid + // picking up sourceMappingURLs from comments, strings, etc. + let lastMatch, match + // eslint-disable-next-line no-cond-assign + while ((match = re.exec(source))) { + lastMatch = match + } + if (!lastMatch) { + return null + } + return lastMatch[1] + } } diff --git a/packages/utils/src/source-map.ts b/packages/utils/src/source-map.ts index 8e2a630fe28c..cc286e96c08d 100644 --- a/packages/utils/src/source-map.ts +++ b/packages/utils/src/source-map.ts @@ -16,7 +16,7 @@ export type { SourceMapInput } from '@jridgewell/trace-mapping' export interface StackTraceParserOptions { ignoreStackEntries?: (RegExp | string)[] getSourceMap?: (file: string) => unknown - getFileName?: (id: string) => string + getUrlId?: (id: string) => string frameFilter?: (error: ErrorWithDiff, frame: ParsedStack) => boolean | void } @@ -62,7 +62,9 @@ function extractLocation(urlLike: string) { } if (url.startsWith('http:') || url.startsWith('https:')) { const urlObj = new URL(url) - url = urlObj.pathname + urlObj.searchParams.delete('import') + urlObj.searchParams.delete('browserv') + url = urlObj.pathname + urlObj.hash + urlObj.search } if (url.startsWith('/@fs/')) { const isWindows = /^\/@fs\/[a-zA-Z]:\//.test(url) @@ -195,20 +197,16 @@ export function createStackString(stacks: ParsedStack[]): string { export function parseStacktrace( stack: string, - options: StackTraceParserOptions = {}, + options: StackTraceParserOptions, ): ParsedStack[] { const { ignoreStackEntries = stackIgnorePatterns } = options - let stacks = !CHROME_IE_STACK_REGEXP.test(stack) + const stacks = !CHROME_IE_STACK_REGEXP.test(stack) ? parseFFOrSafariStackTrace(stack) : parseV8Stacktrace(stack) - if (ignoreStackEntries.length) { - stacks = stacks.filter( - stack => !ignoreStackEntries.some(p => stack.file.match(p)), - ) - } + return stacks.map((stack) => { - if (options.getFileName) { - stack.file = options.getFileName(stack.file) + if (options.getUrlId) { + stack.file = options.getUrlId(stack.file) } const map = options.getSourceMap?.(stack.file) as @@ -216,15 +214,39 @@ export function parseStacktrace( | null | undefined if (!map || typeof map !== 'object' || !map.version) { - return stack + return shouldFilter(ignoreStackEntries, stack.file) ? null : stack } + const traceMap = new TraceMap(map) - const { line, column } = originalPositionFor(traceMap, stack) + const { line, column, source, name } = originalPositionFor(traceMap, stack) + + let file: string = stack.file + // TODO: respect map.sourceRoot + if (source) { + const fileUrl = stack.file.startsWith('file://') + ? stack.file + : `file://${stack.file}` + file = new URL(source, fileUrl).pathname + } + + if (shouldFilter(ignoreStackEntries, file)) { + return null + } + if (line != null && column != null) { - return { ...stack, line, column } + return { + line, + column, + file, + method: name || stack.method, + } } return stack - }) + }).filter(s => s != null) +} + +function shouldFilter(ignoreStackEntries: (string | RegExp)[], file: string): boolean { + return ignoreStackEntries.some(p => file.match(p)) } function parseFFOrSafariStackTrace(stack: string): ParsedStack[] { @@ -243,7 +265,7 @@ function parseV8Stacktrace(stack: string): ParsedStack[] { export function parseErrorStacktrace( e: ErrorWithDiff, - options: StackTraceParserOptions = {}, + options: StackTraceParserOptions, ): ParsedStack[] { if (!e || isPrimitive(e)) { return [] From 692f4205a9702ad0da1aa50c5e405b95db48af3c Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 21 Feb 2025 15:03:21 +0100 Subject: [PATCH 2/4] chore: cleanup --- packages/utils/src/source-map.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/source-map.ts b/packages/utils/src/source-map.ts index cc286e96c08d..9632e87ae025 100644 --- a/packages/utils/src/source-map.ts +++ b/packages/utils/src/source-map.ts @@ -197,7 +197,7 @@ export function createStackString(stacks: ParsedStack[]): string { export function parseStacktrace( stack: string, - options: StackTraceParserOptions, + options: StackTraceParserOptions = {}, ): ParsedStack[] { const { ignoreStackEntries = stackIgnorePatterns } = options const stacks = !CHROME_IE_STACK_REGEXP.test(stack) @@ -265,7 +265,7 @@ function parseV8Stacktrace(stack: string): ParsedStack[] { export function parseErrorStacktrace( e: ErrorWithDiff, - options: StackTraceParserOptions, + options: StackTraceParserOptions = {}, ): ParsedStack[] { if (!e || isPrimitive(e)) { return [] From cdd3374e0abdf84f897f2e4f9ae43e5aaeca8e19 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 21 Feb 2025 16:04:04 +0100 Subject: [PATCH 3/4] chore: comment --- packages/browser/src/node/projectParent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/node/projectParent.ts b/packages/browser/src/node/projectParent.ts index 4c6298b31f7d..9041af23f194 100644 --- a/packages/browser/src/node/projectParent.ts +++ b/packages/browser/src/node/projectParent.ts @@ -57,8 +57,8 @@ export class ParentBrowserProject { if (this.sourceMapCache.has(id)) { return this.sourceMapCache.get(id) } - const result = this.vite.moduleGraph.getModuleById(id)?.transformResult + // this can happen for bundled dependencies in node_modules/.vite if (result && !result.map) { const sourceMapUrl = this.retrieveSourceMapURL(result.code) if (!sourceMapUrl) { From c04929f5d2c13a5a4723639a3da8198142d3fe5c Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Tue, 25 Feb 2025 16:40:40 +0100 Subject: [PATCH 4/4] test: add a test for source map bundling --- packages/browser/src/node/projectParent.ts | 12 +++++++++--- packages/utils/src/source-map.ts | 6 ++++-- pnpm-lock.yaml | 3 +++ test/browser/bundled-lib/package.json | 6 ++++++ test/browser/bundled-lib/src/a.js | 3 +++ test/browser/bundled-lib/src/b.js | 3 +++ test/browser/bundled-lib/src/index.d.ts | 1 + test/browser/bundled-lib/src/index.js | 6 ++++++ test/browser/package.json | 1 + test/browser/specs/runner.test.ts | 12 ++++++++---- test/browser/test/failing.test.ts | 5 +++++ test/browser/vitest.config.mts | 2 +- 12 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 test/browser/bundled-lib/package.json create mode 100644 test/browser/bundled-lib/src/a.js create mode 100644 test/browser/bundled-lib/src/b.js create mode 100644 test/browser/bundled-lib/src/index.d.ts create mode 100644 test/browser/bundled-lib/src/index.js diff --git a/packages/browser/src/node/projectParent.ts b/packages/browser/src/node/projectParent.ts index 9041af23f194..e582c048247c 100644 --- a/packages/browser/src/node/projectParent.ts +++ b/packages/browser/src/node/projectParent.ts @@ -77,10 +77,16 @@ export class ParentBrowserProject { if (mod) { return id } - const resolvedId = resolve(project.config.root, id.slice(1)) - const modUrl = this.vite.moduleGraph.getModuleById(resolvedId) + const resolvedPath = resolve(project.config.root, id.slice(1)) + const modUrl = this.vite.moduleGraph.getModuleById(resolvedPath) if (modUrl) { - return resolvedId + return resolvedPath + } + // some browsers (looking at you, safari) don't report queries in stack traces + // the next best thing is to try the first id that this file resolves to + const files = this.vite.moduleGraph.getModulesByFile(resolvedPath) + if (files && files.size) { + return files.values().next().value!.id! } return id }, diff --git a/packages/utils/src/source-map.ts b/packages/utils/src/source-map.ts index 9632e87ae025..af256e96431e 100644 --- a/packages/utils/src/source-map.ts +++ b/packages/utils/src/source-map.ts @@ -221,12 +221,14 @@ export function parseStacktrace( const { line, column, source, name } = originalPositionFor(traceMap, stack) let file: string = stack.file - // TODO: respect map.sourceRoot if (source) { const fileUrl = stack.file.startsWith('file://') ? stack.file : `file://${stack.file}` - file = new URL(source, fileUrl).pathname + const sourceRootUrl = map.sourceRoot + ? new URL(map.sourceRoot, fileUrl) + : fileUrl + file = new URL(source, sourceRootUrl).pathname } if (shouldFilter(ignoreStackEntries, file)) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9420de310ad2..f31680e57554 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1057,6 +1057,9 @@ importers: '@vitest/browser': specifier: workspace:* version: link:../../packages/browser + '@vitest/bundled-lib': + specifier: link:./bundled-lib + version: link:bundled-lib '@vitest/cjs-lib': specifier: link:./cjs-lib version: link:cjs-lib diff --git a/test/browser/bundled-lib/package.json b/test/browser/bundled-lib/package.json new file mode 100644 index 000000000000..5d36a655294c --- /dev/null +++ b/test/browser/bundled-lib/package.json @@ -0,0 +1,6 @@ +{ + "name": "@vitest/bundled-lib", + "type": "module", + "private": true, + "main": "./src/index.js" +} diff --git a/test/browser/bundled-lib/src/a.js b/test/browser/bundled-lib/src/a.js new file mode 100644 index 000000000000..41baecd8e67f --- /dev/null +++ b/test/browser/bundled-lib/src/a.js @@ -0,0 +1,3 @@ +export function a() { + return 'a' +} diff --git a/test/browser/bundled-lib/src/b.js b/test/browser/bundled-lib/src/b.js new file mode 100644 index 000000000000..7593b7b810f5 --- /dev/null +++ b/test/browser/bundled-lib/src/b.js @@ -0,0 +1,3 @@ +export function b() { + throw new Error('error from b') +} diff --git a/test/browser/bundled-lib/src/index.d.ts b/test/browser/bundled-lib/src/index.d.ts new file mode 100644 index 000000000000..486226656427 --- /dev/null +++ b/test/browser/bundled-lib/src/index.d.ts @@ -0,0 +1 @@ +export declare function index(): string diff --git a/test/browser/bundled-lib/src/index.js b/test/browser/bundled-lib/src/index.js new file mode 100644 index 000000000000..b8a165302f6e --- /dev/null +++ b/test/browser/bundled-lib/src/index.js @@ -0,0 +1,6 @@ +import { a } from './a.js' +import { b } from './b.js' + +export function index() { + return a() + b() +} diff --git a/test/browser/package.json b/test/browser/package.json index 0acbc3c1aefb..05c2c2bf523c 100644 --- a/test/browser/package.json +++ b/test/browser/package.json @@ -26,6 +26,7 @@ "@types/react": "^18.2.79", "@vitejs/plugin-basic-ssl": "^1.0.2", "@vitest/browser": "workspace:*", + "@vitest/bundled-lib": "link:./bundled-lib", "@vitest/cjs-lib": "link:./cjs-lib", "@vitest/injected-lib": "link:./injected-lib", "react": "^18.3.1", diff --git a/test/browser/specs/runner.test.ts b/test/browser/specs/runner.test.ts index 7786cdfebd4f..80d5a69fd8a0 100644 --- a/test/browser/specs/runner.test.ts +++ b/test/browser/specs/runner.test.ts @@ -163,15 +163,19 @@ error with a stack test(`stack trace points to correct file in every browser`, () => { // depending on the browser it references either `.toBe()` or `expect()` - expect(stderr).toMatch(/test\/failing.test.ts:10:(12|17)/) + expect(stderr).toMatch(/test\/failing.test.ts:11:(12|17)/) // column is 18 in safari, 8 in others expect(stderr).toMatch(/throwError src\/error.ts:8:(18|8)/) expect(stderr).toContain('The call was not awaited. This method is asynchronous and must be awaited; otherwise, the call will not start to avoid unhandled rejections.') - expect(stderr).toMatch(/test\/failing.test.ts:18:(27|36)/) - expect(stderr).toMatch(/test\/failing.test.ts:19:(27|33)/) - expect(stderr).toMatch(/test\/failing.test.ts:20:(27|39)/) + expect(stderr).toMatch(/test\/failing.test.ts:19:(27|36)/) + expect(stderr).toMatch(/test\/failing.test.ts:20:(27|33)/) + expect(stderr).toMatch(/test\/failing.test.ts:21:(27|39)/) + + expect(stderr).toMatch(/bundled-lib\/src\/b.js:2:(8|18)/) + expect(stderr).toMatch(/bundled-lib\/src\/index.js:5:(15|17)/) + expect(stderr).toMatch(/test\/failing.test.ts:25:(2|8)/) }) test('popup apis should log a warning', () => { diff --git a/test/browser/test/failing.test.ts b/test/browser/test/failing.test.ts index 96495127452e..2f88b0d1fb21 100644 --- a/test/browser/test/failing.test.ts +++ b/test/browser/test/failing.test.ts @@ -1,4 +1,5 @@ import { page } from '@vitest/browser/context' +import { index } from '@vitest/bundled-lib' import { expect, it } from 'vitest' import { throwError } from '../src/error' @@ -19,3 +20,7 @@ it('several locator methods are not awaited', () => { page.getByRole('button').click() page.getByRole('button').tripleClick() }) + +it('correctly prints error from a bundled file', () => { + index() +}) diff --git a/test/browser/vitest.config.mts b/test/browser/vitest.config.mts index 89331be8c974..7f7e784d3e14 100644 --- a/test/browser/vitest.config.mts +++ b/test/browser/vitest.config.mts @@ -42,7 +42,7 @@ export default defineConfig({ }, }, optimizeDeps: { - include: ['@vitest/cjs-lib', 'react/jsx-dev-runtime'], + include: ['@vitest/cjs-lib', '@vitest/bundled-lib', 'react/jsx-dev-runtime'], }, test: { include: ['test/**.test.{ts,js,tsx}'],