diff --git a/.changeset/fix-source-map-source-names.md b/.changeset/fix-source-map-source-names.md new file mode 100644 index 000000000..1bc57c00c --- /dev/null +++ b/.changeset/fix-source-map-source-names.md @@ -0,0 +1,18 @@ +--- +"@callstack/repack": patch +"@callstack/repack-dev-server": patch +--- + +Fix source map source names and development stack frames for files whose path +URLs would encode. + +- `babel-loader` and `babel-swc-loader` no longer set `sourceRoot`. Webpack, and + Rspack whenever Babel produced the map, prepended it to the absolute source + path, so project files were named `src//` in development + and release source maps. +- The dev server now returns symbolicated file names exactly as the bundler + emitted them. Before, `source-map` percent-encoded them, so a frame outside the + project root came back as `[projectRoot%5E2]/...`, and a path with a space or + non-ASCII character came back encoded. The encoded name broke opening the file + from LogBox and reading its source when the map has no embedded content. +- Development asset requests with an encoded directory name no longer 404. diff --git a/packages/dev-server/src/plugins/symbolicate/Symbolicator.ts b/packages/dev-server/src/plugins/symbolicate/Symbolicator.ts index a69053743..5acfa42c0 100644 --- a/packages/dev-server/src/plugins/symbolicate/Symbolicator.ts +++ b/packages/dev-server/src/plugins/symbolicate/Symbolicator.ts @@ -3,9 +3,11 @@ import { codeFrameColumns } from '@babel/code-frame'; import type { FastifyBaseLogger } from 'fastify'; import { SourceMapConsumer } from 'source-map'; import { + decodeSourceName, + escapeSourceName, isGeneratedBundleFrame, isSymbolicatableFrame, - normalizeInvalidWebpackSourceUrls, + prepareSourceMap, } from '../../utils/symbolication.js'; import type { CodeFrame, @@ -101,7 +103,7 @@ export class Symbolicator { sourceMapLength: rawSourceMap.length, }); const sourceMapConsumer = await new SourceMapConsumer( - normalizeInvalidWebpackSourceUrls(rawSourceMap) + prepareSourceMap(rawSourceMap) ); logger.debug({ @@ -201,7 +203,7 @@ export class Symbolicator { return { lineNumber: lookup.line ?? frame.lineNumber, column: lookup.column ?? frame.column, - file: lookup.source, + file: decodeSourceName(lookup.source), methodName: lookup.name || frame.methodName, collapse: false, }; @@ -233,7 +235,10 @@ export class Symbolicator { try { const consumer = sourceMapConsumers.get(inputFrames[index]?.file); - const embeddedSource = consumer?.sourceContentFor(frame.file, true); + const embeddedSource = consumer?.sourceContentFor( + escapeSourceName(frame.file), + true + ); const source = embeddedSource ?? (await this.delegate.getSource(frame.file)).toString(); diff --git a/packages/dev-server/src/plugins/symbolicate/__tests__/Symbolicator.test.ts b/packages/dev-server/src/plugins/symbolicate/__tests__/Symbolicator.test.ts index db258be73..e463df5dd 100644 --- a/packages/dev-server/src/plugins/symbolicate/__tests__/Symbolicator.test.ts +++ b/packages/dev-server/src/plugins/symbolicate/__tests__/Symbolicator.test.ts @@ -1,3 +1,4 @@ +import { stripVTControlCharacters } from 'node:util'; import type { FastifyBaseLogger } from 'fastify'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { logSymbolicatedStackFrame } from '../logSymbolicatedStackFrame.js'; @@ -50,6 +51,20 @@ function createSourceMapWithoutContent(source: string) { }); } +// Maps generated line N to the start of `sources[N - 1]`. +function createSourceMapWithSources( + sources: string[], + sourcesContent?: string[] +) { + return JSON.stringify({ + version: 3, + sources, + sourcesContent, + names: [], + mappings: ['AAAA', ...sources.slice(1).map(() => 'ACAA')].join(';'), + }); +} + function createDeferred() { let resolve!: (value: T | PromiseLike) => void; const promise = new Promise((resolvePromise) => { @@ -188,6 +203,87 @@ describe('Symbolicator', () => { }); }); + describe('source names that URLs encode', () => { + const bundleUrl = 'http://localhost:8081/index.bundle?platform=ios'; + const sources = [ + '[projectRoot^2]/node_modules/react-native/Libraries/Core/ExceptionsManager.js', + '[projectRoot]/src/screens/Home Screen.tsx', + '[projectRoot]/src/Écran.tsx', + '[projectRoot]/src/100%25 done.tsx', + '/Users/me/My Projects/app/src/App.tsx', + ]; + const frameFor = (source: string) => ({ + file: bundleUrl, + lineNumber: sources.indexOf(source) + 1, + column: 0, + methodName: 'frame', + }); + + it.each(sources)('returns %s as emitted by the bundler', async (source) => { + const symbolicator = new Symbolicator( + createDelegate(async () => + createSourceMapWithSources( + sources, + sources.map((name) => `// content of ${name}`) + ) + ) + ); + + const result = await symbolicator.process(logger, [frameFor(source)]); + + expect(result.stack[0]?.file).toBe(source); + expect(result.codeFrame?.fileName).toBe(source); + expect( + stripVTControlCharacters(result.codeFrame?.content ?? '') + ).toContain(`content of ${source}`); + }); + + it.each(sources)( + 'reads %s from the compiler when the map has no content', + async (source) => { + const getSource = vi.fn(async () => 'const fromDisk = true;'); + const symbolicator = new Symbolicator( + createDelegate( + async () => createSourceMapWithSources(sources), + getSource + ) + ); + + const result = await symbolicator.process(logger, [frameFor(source)]); + + expect(getSource).toHaveBeenCalledWith(source); + expect( + stripVTControlCharacters(result.codeFrame?.content ?? '') + ).toContain('const fromDisk = true;'); + } + ); + + it('keeps remote source URLs encoded', async () => { + // `fetchSourceMapFromBundle` rewrites remote source names to URLs. + const remoteSource = + 'http://localhost:8082/__repack_source__/[projectRoot%5E2]/packages/remote/src/App.tsx'; + const symbolicator = new Symbolicator( + createDelegate(async () => + createSourceMapWithSources([remoteSource], ['const remote = 1;']) + ) + ); + + const result = await symbolicator.process(logger, [ + { + file: 'http://localhost:8082/remote.chunk.bundle', + lineNumber: 1, + column: 0, + methodName: 'App', + }, + ]); + + expect(result.stack[0]?.file).toBe(remoteSource); + expect( + stripVTControlCharacters(result.codeFrame?.content ?? '') + ).toContain('const remote = 1;'); + }); + }); + it('loads a source map once for repeated frames in one request', async () => { const bundleUrl = 'http://localhost:8082/repeated.chunk.bundle'; const getSourceMap = vi.fn(async () => @@ -338,6 +434,41 @@ describe('logSymbolicatedStackFrame', () => { }); }); + it('strips the placeholder from a frame outside of the project root', async () => { + const info = vi.fn(); + const runtimeLogger = { ...logger, info } as unknown as FastifyBaseLogger; + const stack = [ + { + file: 'http://localhost:8081/index.bundle?platform=ios', + lineNumber: 1, + column: 0, + methodName: 'Shared', + }, + { + file: 'http://localhost:8081/index.bundle?platform=ios', + lineNumber: 200, + column: 30, + methodName: 'renderWithHooks', + }, + ]; + const symbolicator = new Symbolicator( + createDelegate(async () => + createSourceMapWithSources( + ['[projectRoot^2]/packages/shared/src/Shared.tsx'], + ['export const Shared = () => null;'] + ) + ) + ); + + const results = await symbolicator.process(runtimeLogger, stack); + logSymbolicatedStackFrame(runtimeLogger, stack, results); + + expect(info).toHaveBeenCalledWith({ + msg: 'Symbolicated stack frame: packages/shared/src/Shared.tsx:1:0', + methodName: 'Shared', + }); + }); + it('does not log component-stack-only symbolication', () => { const info = vi.fn(); const runtimeLogger = { info } as unknown as FastifyBaseLogger; diff --git a/packages/dev-server/src/utils/symbolication.ts b/packages/dev-server/src/utils/symbolication.ts index 0fce9fc2b..ec01d9e4f 100644 --- a/packages/dev-server/src/utils/symbolication.ts +++ b/packages/dev-server/src/utils/symbolication.ts @@ -5,15 +5,28 @@ interface StackFrameLike { file: string | null; } -export function normalizeInvalidWebpackSourceUrls( - rawSourceMap: string | Buffer -): string | RawSourceMap | RawIndexMap { - const sourceMapText = rawSourceMap.toString(); - if (!sourceMapText.includes('webpack://')) { - return sourceMapText; - } +// `source-map` normalises every source name through `new URL()`, which +// percent-encodes characters such as spaces, non-ASCII letters and the caret +// in `[projectRoot^N]`. Escaping `%` before the consumer reads the map makes +// `decodeSourceName` an exact inverse, so a file name that contains `%` keeps +// it instead of being decoded a second time. +export function escapeSourceName(source: string) { + return source.replaceAll('%', '%25'); +} - const sourceMap = JSON.parse(sourceMapText) as { +export function decodeSourceName(source: string) { + return decodeURIComponent(source); +} + +/** + * Prepare a raw source map for `SourceMapConsumer`: replace webpack source + * URLs that would make it reject the map and escape every source name, so + * names returned by the consumer can be restored with `decodeSourceName`. + */ +export function prepareSourceMap( + rawSourceMap: string | Buffer +): RawSourceMap | RawIndexMap { + const sourceMap = JSON.parse(rawSourceMap.toString()) as { sources?: unknown[]; sections?: Array<{ map?: unknown }>; }; @@ -39,12 +52,12 @@ export function normalizeInvalidWebpackSourceUrls( 'webpack://$1/' ); if (!normalizedSource.startsWith('webpack://')) { - return normalizedSource; + return escapeSourceName(normalizedSource); } try { new URL(normalizedSource); - return normalizedSource; + return escapeSourceName(normalizedSource); } catch { // Some generated Module Federation runtime modules use their source // text as a webpack URL. A single invalid URL makes source-map reject diff --git a/packages/repack/src/commands/common/__tests__/parseUrl.test.ts b/packages/repack/src/commands/common/__tests__/parseUrl.test.ts index 6ad96c9ea..a995cea28 100644 --- a/packages/repack/src/commands/common/__tests__/parseUrl.test.ts +++ b/packages/repack/src/commands/common/__tests__/parseUrl.test.ts @@ -80,6 +80,34 @@ describe('parseUrl', () => { }); }); + it('should return the decoded path', () => { + // Symbolicated stack frames are passed as file names, not URLs. + expectParsed('[projectRoot^2]/node_modules/pkg/index.js', { + resourcePath: '[projectRoot^2]/node_modules/pkg/index.js', + platform: undefined, + }); + expectParsed('[projectRoot]/src/Home Screen.tsx', { + resourcePath: '[projectRoot]/src/Home Screen.tsx', + platform: undefined, + }); + expectParsed('[projectRoot]/src/Écran.ios.tsx', { + resourcePath: '[projectRoot]/src/Écran.ios.tsx', + platform: 'ios', + }); + // Request URLs arrive percent-encoded. + expectParsed('/assets/src/caret%5Edir/image.png?platform=ios', { + resourcePath: 'assets/src/caret^dir/image.png', + platform: 'ios', + }); + }); + + it('should keep a path that is not a valid escape sequence', () => { + expectParsed('assets/100%.png?platform=ios', { + resourcePath: 'assets/100%.png', + platform: 'ios', + }); + }); + it('should work with different platform lists', () => { expectParsed( '/react-native/src/index.js', diff --git a/packages/repack/src/commands/common/__tests__/resolveProjectPath.test.ts b/packages/repack/src/commands/common/__tests__/resolveProjectPath.test.ts index dbb2fddca..675f14c92 100644 --- a/packages/repack/src/commands/common/__tests__/resolveProjectPath.test.ts +++ b/packages/repack/src/commands/common/__tests__/resolveProjectPath.test.ts @@ -1,4 +1,5 @@ import path from 'node:path'; +import { parseUrl } from '../parseUrl.js'; import { resolveProjectPath } from '../resolveProjectPath.js'; describe('resolveProjectPath', () => { @@ -47,4 +48,21 @@ describe('resolveProjectPath', () => { '/a/b/c/d/e/f' ); }); + + it('should resolve paths returned by parseUrl', () => { + // The dev server passes symbolicated file names through parseUrl before + // resolving them, so encoded characters must not survive that step. + const resolveParsed = (input: string) => + resolveProjectPath( + parseUrl(input, ['ios', 'android']).resourcePath, + path.resolve('/workspace/apps/app') + ); + + expect(resolveParsed('[projectRoot^2]/node_modules/pkg/index.js')).toBe( + path.resolve('/workspace/node_modules/pkg/index.js') + ); + expect(resolveParsed('[projectRoot]/src/Home Screen.tsx')).toBe( + path.resolve('/workspace/apps/app/src/Home Screen.tsx') + ); + }); }); diff --git a/packages/repack/src/commands/common/parseUrl.ts b/packages/repack/src/commands/common/parseUrl.ts index 06b840d89..f64c2bac0 100644 --- a/packages/repack/src/commands/common/parseUrl.ts +++ b/packages/repack/src/commands/common/parseUrl.ts @@ -1,5 +1,17 @@ +// `new URL()` percent-encodes characters such as spaces, non-ASCII letters +// and the caret in `[projectRoot^N]`, but callers look the path up as a file. +function decodePathname(pathname: string) { + try { + return decodeURIComponent(pathname); + } catch { + // a malformed escape sequence, e.g. a literal `%` in a file name + return pathname; + } +} + export function parseUrl(url: string, platforms: string[], base = 'file:///') { - const { pathname, searchParams } = new URL(url, base); + const { pathname: encodedPathname, searchParams } = new URL(url, base); + const pathname = decodePathname(encodedPathname); let path = pathname; let platform = searchParams.get('platform'); diff --git a/packages/repack/src/loaders/babelLoader/babelLoader.ts b/packages/repack/src/loaders/babelLoader/babelLoader.ts index e9ba968f6..c72dbd1eb 100644 --- a/packages/repack/src/loaders/babelLoader/babelLoader.ts +++ b/packages/repack/src/loaders/babelLoader/babelLoader.ts @@ -138,7 +138,6 @@ export default async function babelLoader( filename: this.resourcePath, sourceMaps: withSourceMaps, sourceFileName: this.resourcePath, - sourceRoot: this.context, inputSourceMap: withSourceMaps ? inputSourceMap : undefined, ...babelOverrides, }, diff --git a/packages/repack/src/loaders/babelSwcLoader/babelSwcLoader.ts b/packages/repack/src/loaders/babelSwcLoader/babelSwcLoader.ts index 8caab087c..e570473ef 100644 --- a/packages/repack/src/loaders/babelSwcLoader/babelSwcLoader.ts +++ b/packages/repack/src/loaders/babelSwcLoader/babelSwcLoader.ts @@ -123,7 +123,6 @@ export default async function babelSwcLoader( filename: this.resourcePath, sourceMaps: withSourceMaps, sourceFileName: this.resourcePath, - sourceRoot: this.context, inputSourceMap: withSourceMaps ? inputSourceMap : undefined, ...options.babelOverrides, }; @@ -176,7 +175,6 @@ export default async function babelSwcLoader( ? JSON.stringify(babelResult?.map) : undefined, sourceFileName: this.resourcePath, - sourceRoot: this.context!, ...options.swcOverrides, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 348abf584..fb5925128 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -763,6 +763,9 @@ importers: '@types/node': specifier: 'catalog:' version: 20.19.43 + hermes-parser: + specifier: 0.36.0 + version: 0.36.0 memfs: specifier: ^4.11.1 version: 4.64.0 diff --git a/tests/integration/package.json b/tests/integration/package.json index 76b27843a..dc4013916 100644 --- a/tests/integration/package.json +++ b/tests/integration/package.json @@ -18,6 +18,7 @@ "@module-federation/enhanced-v21": "npm:@module-federation/enhanced@0.21.0", "@rspack/core": "catalog:", "@types/node": "catalog:", + "hermes-parser": "0.36.0", "memfs": "^4.11.1", "typescript": "catalog:", "vitest": "catalog:", diff --git a/tests/integration/src/loaders/source-map-sources.test.ts b/tests/integration/src/loaders/source-map-sources.test.ts new file mode 100644 index 000000000..bfc8d411a --- /dev/null +++ b/tests/integration/src/loaders/source-map-sources.test.ts @@ -0,0 +1,124 @@ +import fs from 'node:fs'; +import { createRequire } from 'node:module'; +import os from 'node:os'; +import path from 'node:path'; +import { afterAll, beforeAll, expect, it } from 'vitest'; +import { compile, createCompiler } from '../helpers.js'; + +const require = createRequire(import.meta.url); +const repackRoot = path.dirname( + require.resolve('@callstack/repack/package.json') +); + +const LOADERS = ['babel-loader', 'babel-swc-loader'] as const; + +// A monorepo with an app two levels below the workspace root. Files inside +// the app cover the standalone case; the workspace package and the hoisted +// dependency cover files resolved outside of the project root. +const FILES: Record = { + 'apps/app/src/index.js': [ + "import { homeScreen } from './screens/Home Screen';", + "import { localDep } from 'local-dep';", + "import { hoistedDep } from 'hoisted-dep';", + "import { format } from '../../../packages/shared/src/format';", + 'export default [homeScreen, localDep, hoistedDep, format];', + ].join('\n'), + 'apps/app/src/screens/Home Screen.js': + "export const homeScreen = () => 'home';", + 'apps/app/node_modules/local-dep/package.json': + '{ "name": "local-dep", "main": "index.js" }', + 'apps/app/node_modules/local-dep/index.js': + "export const localDep = () => 'local';", + 'node_modules/hoisted-dep/package.json': + '{ "name": "hoisted-dep", "main": "index.js" }', + 'node_modules/hoisted-dep/index.js': + "export const hoistedDep = () => 'hoisted';", + 'packages/shared/src/format.js': + 'export const format = (value) => value.trim();', +}; + +let workspaceRoot: string; +let projectRoot: string; + +beforeAll(() => { + // Resolve symlinks (e.g. /var -> /private/var on macOS) so the bundler + // sees the same project root that the files resolve to. + workspaceRoot = fs.realpathSync( + fs.mkdtempSync(path.join(os.tmpdir(), 'repack-source-names-')) + ); + projectRoot = path.join(workspaceRoot, 'apps/app'); + + for (const [file, content] of Object.entries(FILES)) { + const filePath = path.join(workspaceRoot, file); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content); + } + + // Webpack only uses swc when the project can resolve `@swc/core`. + fs.mkdirSync(path.join(workspaceRoot, 'node_modules/@swc')); + fs.symlinkSync( + path.dirname( + require.resolve('@swc/core/package.json', { paths: [repackRoot] }) + ), + path.join(workspaceRoot, 'node_modules/@swc/core'), + 'junction' + ); +}); + +afterAll(() => { + fs.rmSync(workspaceRoot, { recursive: true, force: true }); +}); + +async function getSourceMapSources(loader: (typeof LOADERS)[number]) { + const compiler = await createCompiler({ + context: projectRoot, + mode: 'development', + devtool: 'source-map', + entry: './src/index.js', + output: { + path: '/out', + devtoolModuleFilenameTemplate: '[absolute-resource-path]', + }, + module: { + rules: [ + { + test: /\.js$/, + use: { + loader: require.resolve(`@callstack/repack/${loader}`), + options: { + hermesParserPath: require.resolve('hermes-parser'), + ...(loader === 'babel-swc-loader' && { + hideParallelModeWarning: true, + }), + }, + }, + }, + ], + }, + }); + + const { volume } = await compile(compiler); + const sourceMap = JSON.parse( + volume.readFileSync('/out/main.js.map', 'utf-8') as string + ); + return sourceMap.sources as string[]; +} + +// Webpack, and Rspack when Babel writes the loader map, used to prepend the +// loader's `sourceRoot` to the absolute source path, naming files +// `/` in every source map. +it.each(LOADERS)( + 'names each file once in the source map with %s', + async (loader) => { + const sources = await getSourceMapSources(loader); + + expect( + sources.filter((source) => source.startsWith(workspaceRoot)).sort() + ).toEqual( + Object.keys(FILES) + .filter((file) => file.endsWith('.js')) + .map((file) => path.join(workspaceRoot, file)) + .sort() + ); + } +);