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: use relative paths in source map's "sources" field #3177

Merged
merged 3 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions packages/vite-node/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ export class ViteNodeServer {
return this.fetchPromiseMap.get(id)!
}

async transformRequest(id: string) {
async transformRequest(id: string, filepath = id) {
// reuse transform for concurrent requests
if (!this.transformPromiseMap.has(id)) {
this.transformPromiseMap.set(id,
this._transformRequest(id)
this._transformRequest(id, filepath)
.finally(() => {
this.transformPromiseMap.delete(id)
}),
Expand Down Expand Up @@ -177,7 +177,7 @@ export class ViteNodeServer {
}
else {
const start = performance.now()
const r = await this._transformRequest(id, transformMode)
const r = await this._transformRequest(id, filePath, transformMode)
duration = performance.now() - start
result = { code: r?.code, map: r?.map as any }
}
Expand All @@ -191,15 +191,15 @@ export class ViteNodeServer {
return result
}

protected async processTransformResult(id: string, result: TransformResult) {
const mod = this.server.moduleGraph.getModuleById(id)
protected async processTransformResult(filepath: string, result: TransformResult) {
const mod = this.server.moduleGraph.getModuleById(filepath)
return withInlineSourcemap(result, {
filepath: mod?.file || id,
filepath: mod?.file || filepath,
root: this.server.config.root,
})
}

private async _transformRequest(id: string, customTransformMode?: 'web' | 'ssr') {
private async _transformRequest(id: string, filepath: string, customTransformMode?: 'web' | 'ssr') {
debugRequest(id)

let result: TransformResult | null = null
Expand All @@ -225,7 +225,7 @@ export class ViteNodeServer {

const sourcemap = this.options.sourcemap ?? 'inline'
if (sourcemap === 'inline' && result && !id.includes('node_modules'))
result = await this.processTransformResult(id, result)
result = await this.processTransformResult(filepath, result)

if (this.options.debug?.dumpModules)
await this.debugger?.dumpFile(id, result)
Expand Down
20 changes: 11 additions & 9 deletions packages/vite-node/src/source-map.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { TransformResult } from 'vite'
import { dirname, isAbsolute, join, resolve } from 'pathe'
import { dirname, isAbsolute, relative, resolve } from 'pathe'
import type { EncodedSourceMap } from '@jridgewell/trace-mapping'
import { install } from './source-map-handler'

Expand All @@ -24,17 +24,19 @@ export function withInlineSourcemap(result: TransformResult, options: {
if (!map || code.includes(VITE_NODE_SOURCEMAPPING_SOURCE))
return result

// sources path from `ViteDevServer` may be not a valid filesystem path (eg. /src/main.js),
// so we try to convert them to valid filesystem path
map.sources = map.sources?.map((source) => {
if (!source)
return source
// make source absolute again, it might not be relative to the root, but to the "source root"
// https://github.com/bmeurer/vite/blob/172c3e36226ec4bdf2c9d5f8fa84310bde3fec54/packages/vite/src/node/server/transformRequest.ts#L281
if (!isAbsolute(source))
return resolve(dirname(options.filepath), source)
if (!source.startsWith(options.root))
return join(options.root, source)
// sometimes files here are absolute,
// but they are considered absolute to the server url, not the file system
// this is a bug in Vite
// all files should be either absolute to the file system or relative to the source map file
if (isAbsolute(source)) {
const actualPath = (!source.startsWith(options.root) && source.startsWith('/'))
? resolve(options.root, source.slice(1))
: source
return relative(dirname(options.filepath), actualPath)
}
return source
})

Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/typecheck/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export interface FileInformation {
}

export async function collectTests(ctx: WorkspaceProject, filepath: string): Promise<null | FileInformation> {
const request = await ctx.vitenode.transformRequest(filepath)
const request = await ctx.vitenode.transformRequest(filepath, filepath)
if (!request)
return null
const ast = parseAst(request.code, {
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/typecheck/typechecker.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { rm } from 'node:fs/promises'
import type { ExecaChildProcess } from 'execa'
import { execa } from 'execa'
import { extname, resolve } from 'pathe'
import { basename, extname, resolve } from 'pathe'
import { SourceMapConsumer } from 'source-map'
import { getTasks } from '../utils'
import { ensurePackageInstalled } from '../node/pkg'
Expand Down Expand Up @@ -132,7 +132,7 @@ export class Typechecker {
const processedPos = mapConsumer?.generatedPositionFor({
line: originalError.line,
column: originalError.column,
source: path,
source: basename(path),
}) || originalError
const line = processedPos.line ?? originalError.line
const column = processedPos.column ?? originalError.column
Expand Down
2 changes: 1 addition & 1 deletion test/vite-node/test/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ describe('server works correctly', async () => {
const sourceMap = extractSourceMap(fetchResult.code!)

// expect got sourcemap source in a valid filesystem path
expect(sourceMap?.sources[0]).toBe(resolve(__dirname, '../src/foo.js'))
expect(sourceMap?.sources[0]).toBe('foo.js')
})
})