diff --git a/src/application/javascript/JavaScriptArtifactAnalysis.sourceMaps.test.ts b/src/application/javascript/JavaScriptArtifactAnalysis.sourceMaps.test.ts index c60358dfb6..e5786c8b62 100644 --- a/src/application/javascript/JavaScriptArtifactAnalysis.sourceMaps.test.ts +++ b/src/application/javascript/JavaScriptArtifactAnalysis.sourceMaps.test.ts @@ -1,11 +1,10 @@ import { expect, it } from "vitest"; -import { TraceMap } from "@jridgewell/trace-mapping"; import { analyzeJavaScriptArtifactFiles } from "./JavaScriptArtifactAnalysis.js"; const HASH = "a".repeat(64); -it("retains batched TraceMap-resolved paths and null source positions", () => { +it("retains resolved source declarations, Unicode spelling and null source positions", () => { const sourceMapText = JSON.stringify({ version: 3, file: "main.js", @@ -35,20 +34,6 @@ it("retains batched TraceMap-resolved paths and null source positions", () => { }); const originals = analysis.source_maps[0]?.sources; const original = originals?.[0]; - const producerPath = new TraceMap( - { - version: 3, - file: "main.js", - sourceRoot: "../src/", - sources: ["cafe\u0301.ts", "", "other.ts"], - names: [], - mappings: "", - }, - "file:///artifact/dist/main.js.map", - ).resolvedSources[0]; - - if (producerPath === undefined) - throw new Error("TraceMap should resolve its single source declaration"); expect(originals?.[1]?.reference).toBeNull(); expect(originals?.[2]?.reference?.resolution).toEqual({ kind: "artifact-relative", @@ -63,10 +48,4 @@ it("retains batched TraceMap-resolved paths and null source positions", () => { path: "src/cafe\u0301.ts", }, }); - expect(original?.reference?.resolution).toEqual({ - kind: "artifact-relative", - path: decodeURIComponent( - new URL(producerPath).pathname.slice("/artifact/".length), - ), - }); }); diff --git a/src/browser/WebSourceMapFetcher.test.ts b/src/browser/WebSourceMapFetcher.test.ts index c8675cf109..3f4a3848db 100644 --- a/src/browser/WebSourceMapFetcher.test.ts +++ b/src/browser/WebSourceMapFetcher.test.ts @@ -114,3 +114,53 @@ it("observes a fetch rejection that arrives after its host aborts synchronously" process.off("unhandledRejection", onUnhandled); } }); + +it.each([ + { + sourceRoot: "https://cdn.example/src/", + expected: "https://cdn.example/src/a.js", + }, + { + sourceRoot: "https://cdn.example/src//", + expected: "https://cdn.example/src//a.js", + }, + { sourceRoot: "webpack://", expected: "webpack://a.js" }, + { sourceRoot: "webpack:///", expected: "webpack:///a.js" }, + { sourceRoot: "", expected: "https://app.example.test/a.js" }, + { sourceRoot: null, expected: "https://app.example.test/maps/a.js" }, +])( + "retains the admitted ECMA-426 source identity for root $sourceRoot", + async ({ sourceRoot, expected }) => { + const text = JSON.stringify({ + version: 3, + sourceRoot, + sources: ["a.js", null], + names: [], + mappings: "AAAA;ACAA", + }); + const fetchInput = analyzeWebBundleInputSchema.parse({ + cdp_endpoint: "http://127.0.0.1:9222", + target_id: "page-1", + fetch_source_maps: true, + allowed_origins: [new URL(request.fetchUrl).origin], + }); + const selectedRequest = { + ...request, + declaredUrl: "https://app.example.test/maps/app.js.map", + fetchUrl: "https://app.example.test/maps/app.js.map", + }; + const result = await fetchWebSourceMaps( + [selectedRequest], + fetchInput, + undefined, + { + fetch: async () => new Response(text, { status: 200 }), + }, + ); + expect(result.items[0]).toMatchObject({ + status: "included", + original_sources: [{ source: expected }, { source: null }], + mappings: [{ source: expected }, { source: null }], + }); + }, +); diff --git a/src/browser/WebSourceMapFetcher.ts b/src/browser/WebSourceMapFetcher.ts index f2f61782cd..70412511d0 100644 --- a/src/browser/WebSourceMapFetcher.ts +++ b/src/browser/WebSourceMapFetcher.ts @@ -595,12 +595,10 @@ const normalizeSourceMap = ( 4, ); const sourcesByRaw = new Map(); - const resolvedByRoot = new Map>(); - preflightResolvedSourceBytes( + const resolvedByRoot = prepareResolvedSources( inspected.leaves, fetchedUrl, context.budget, - resolvedByRoot, sourcesByRaw, check, ); @@ -614,11 +612,12 @@ const normalizeSourceMap = ( const nameJsonBytes = new Map(); const originalSources: ParsedSourceMapItem["original_sources"] = []; let declarationIndex = 0; - for (const { leaf, resolvedSources } of decoded) { + for (const { leaf } of decoded) { + const resolvedSources = resolvedByRoot.get(leaf.map.sourceRoot ?? null); for (const [index, rawSource] of leaf.map.sources.entries()) { if ((index & 255) === 0) check(); const resolved = - rawSource === null ? null : (resolvedSources[index] ?? null); + rawSource === null ? null : (resolvedSources?.get(rawSource) ?? null); const sanitized = resolved === null ? null : cachedSanitized(sourcesByRaw, resolved); const content = leaf.map.sourcesContent?.[index] ?? null; @@ -647,7 +646,8 @@ const normalizeSourceMap = ( } const mappings: ParsedSourceMapItem["mappings"] = []; declarationIndex = 0; - for (const { leaf, rows, resolvedSources } of decoded) { + for (const { leaf, rows } of decoded) { + const resolvedSources = resolvedByRoot.get(leaf.map.sourceRoot ?? null); const declarationBase = declarationIndex; declarationIndex += leaf.map.sources.length; for (const [line, row] of rows.entries()) { @@ -657,7 +657,9 @@ const normalizeSourceMap = ( const rawSource = leaf.map.sources[segment[1]]; if (rawSource === undefined) continue; const resolved = - rawSource === null ? null : (resolvedSources[segment[1]] ?? null); + rawSource === null + ? null + : (resolvedSources?.get(rawSource) ?? null); const source = resolved === null ? null : cachedSanitized(sourcesByRaw, resolved); const name = @@ -908,14 +910,15 @@ const sanitizeSource = (value: string): string => { } }; -const preflightResolvedSourceBytes = ( +/** Resolve and admit source identities once, before allocating public mapping rows. */ +const prepareResolvedSources = ( leaves: readonly SourceMapLeaf[], mapUrl: string, budget: { readonly expandedBytes: number; resolvedSourceBytes: number }, - resolvedByRoot: Map>, sanitizedByResolved: Map, check: () => void, -): void => { +): ReadonlyMap> => { + const resolvedByRoot = new Map>(); let estimate = budget.expandedBytes; let resolvedBytes = budget.resolvedSourceBytes; for (const { map } of leaves) { @@ -950,6 +953,7 @@ const preflightResolvedSourceBytes = ( } } budget.resolvedSourceBytes = resolvedBytes; + return resolvedByRoot; }; const consumeExpandedBytes = ( diff --git a/src/domain/javascript/javascriptSourceMapPaths.ts b/src/domain/javascript/javascriptSourceMapPaths.ts index 137150a2f4..083d63ead8 100644 --- a/src/domain/javascript/javascriptSourceMapPaths.ts +++ b/src/domain/javascript/javascriptSourceMapPaths.ts @@ -1,6 +1,5 @@ import { posix } from "node:path"; import { domainToASCII, fileURLToPath, pathToFileURL } from "node:url"; -import { TraceMap } from "@jridgewell/trace-mapping"; import { z } from "zod"; @@ -32,24 +31,16 @@ export const resolveJavaScriptSourceMapReferences = ( sourceRoot: string | null, mapPath: string, ): (JavaScriptSourceMapReference | null)[] => { - if (sourceNames.length === 0) return []; - const resolvedSources = resolveSourceMapUrls( - sourceNames.map((sourceName) => sourceName ?? ""), - sourceRoot, - "", - ); - return sourceNames.map((sourceName, index) => { + return sourceNames.map((sourceName) => { if (sourceName === null) return null; - const reference = resolvedSources[index]; - if (reference === undefined) - throw new TypeError( - `TraceMap did not resolve source declaration ${String(index)}`, - ); return { source_name: sourceName, source_root: sourceRoot, map_path: mapPath, - resolution: resolveJavaScriptSourceMapPath(reference, mapPath), + resolution: resolveJavaScriptSourceMapPath( + sourceMapReference(sourceName, sourceRoot), + mapPath, + ), }; }); }; @@ -74,25 +65,6 @@ const sourceMapReference = ( return `${sourceRoot}${sourceRoot.endsWith("/") ? "" : "/"}${source}`; }; -const resolveSourceMapUrls = ( - sources: string[], - sourceRoot: string | null | undefined, - mapUrl: string, -): readonly string[] => { - // ECMA-426 prefixes each source when sourceRoot is non-null, adding `/` - // only when the root does not already end with one. Do this before handing - // the references to TraceMap so its URL resolver cannot duplicate a slash. - return new TraceMap( - { - version: 3, - names: [], - sources: sources.map((source) => sourceMapReference(source, sourceRoot)), - mappings: "", - }, - mapUrl, - ).resolvedSources; -}; - /** Resolve a source-map URL reference without changing its recorded identity. */ const resolveJavaScriptSourceMapPath = ( source: string, diff --git a/src/domain/javascript/sourceToBundleComparison.test.ts b/src/domain/javascript/sourceToBundleComparison.test.ts index a16ea4c0c2..483380b533 100644 --- a/src/domain/javascript/sourceToBundleComparison.test.ts +++ b/src/domain/javascript/sourceToBundleComparison.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it } from "vitest"; -import { TraceMap } from "@jridgewell/trace-mapping"; import { createJavaScriptApplicationGraph, @@ -9,6 +8,7 @@ import type { ApplicationNode } from "./javascriptApplicationGraphSchemas.js"; import { createHistoricalSourceGraph } from "../referenceSourceGraph.js"; import { compareUnicodeCodePoints } from "../unicodeCodePointOrder.js"; import { compareSourceToBundle } from "./sourceToBundleComparison.js"; +import { resolveJavaScriptSourceMapUrl } from "./javascriptSourceMapPaths.js"; import { SOURCE_TO_BUNDLE_SIGNAL_WEIGHTS, sourceToBundleComparisonResultSchema, @@ -433,7 +433,7 @@ describe("source to bundle path syntax", () => { }); }); -describe("source-map resolver equivalence", () => { +describe("source-map declaration resolution", () => { it.each([ { sourceRoot: "https://cdn.example/src/", @@ -442,37 +442,56 @@ describe("source-map resolver equivalence", () => { }, { sourceRoot: "https://cdn.example/src//", - expectedSource: "https://cdn.example/src/a.js", - expectedResolution: { kind: "suffix", path: "cdn.example/src/a.js" }, + expectedSource: "https://cdn.example/src//a.js", + expectedResolution: { kind: "suffix", path: "cdn.example/src//a.js" }, }, { sourceRoot: "webpack://", + expectedSource: "webpack://a.js", + expectedResolution: { + kind: "unresolved", + reason: + "Source reference uses a non-hierarchical URL without a file pathname.", + }, + }, + { + sourceRoot: "webpack:///", expectedSource: "webpack:///a.js", expectedResolution: { kind: "suffix", path: "a.js" }, }, + { + sourceRoot: "../src/", + expectedSource: "file:///artifact/src/a.js", + expectedResolution: { kind: "artifact-relative", path: "src/a.js" }, + }, + { + sourceRoot: "", + expectedSource: "file:///a.js", + expectedResolution: { kind: "suffix", path: "a.js" }, + }, + { + sourceRoot: null, + expectedSource: "file:///artifact/dist/a.js", + expectedResolution: { kind: "artifact-relative", path: "dist/a.js" }, + }, ])( - "matches TraceMap for sourceRoot $sourceRoot", + "applies ECMA-426 sourceRoot $sourceRoot consistently to URLs and graph facts", ({ sourceRoot, expectedSource, expectedResolution }) => { - const producerSource = new TraceMap( - { - version: 3, - file: "main.js", + expect( + resolveJavaScriptSourceMapUrl( + "a.js", sourceRoot, - sources: ["a.js"], - names: [], - mappings: "", - }, - "file:///artifact/dist/main.js.map", - ).resolvedSources[0]; - const reference = sourceMapReference( - "a.js", - sourceRoot, - "dist/main.js.map", - ); - - expect(producerSource).toBe(expectedSource); - expect(reference.source_root).toBe(sourceRoot); - expect(reference.resolution).toEqual(expectedResolution); + "file:///artifact/dist/main.js.map", + ), + ).toBe(expectedSource); + expect( + sourceMapReference("a.js", sourceRoot, "dist/main.js.map"), + ).toEqual({ + source_name: "a.js", + source_root: sourceRoot, + map_path: "dist/main.js.map", + resolution: expectedResolution, + }); }, ); }); diff --git a/src/javascript/sourceMaps/DecodedSourceMap.ts b/src/javascript/sourceMaps/DecodedSourceMap.ts index cf431d6329..520241284a 100644 --- a/src/javascript/sourceMaps/DecodedSourceMap.ts +++ b/src/javascript/sourceMaps/DecodedSourceMap.ts @@ -28,7 +28,6 @@ export const decodeValidatedSourceMapLeaves = ( readonly leaf: SourceMapLeaf; readonly map: TraceMap; readonly rows: ReturnType; - readonly resolvedSources: readonly string[]; }[] => leaves.map((leaf) => { check?.(); @@ -59,5 +58,5 @@ export const decodeValidatedSourceMapLeaves = ( ); } } - return { leaf, map, rows, resolvedSources: map.resolvedSources }; + return { leaf, map, rows }; }); diff --git a/tests/acceptance/applications/sourceMapPrefixCli.test.ts b/tests/acceptance/applications/sourceMapPrefixCli.test.ts index f1d8fa4b6e..de87f16265 100644 --- a/tests/acceptance/applications/sourceMapPrefixCli.test.ts +++ b/tests/acceptance/applications/sourceMapPrefixCli.test.ts @@ -417,3 +417,69 @@ cliTest.for(["CLI", "stdio MCP"] as const)( expect(await readFile(join(root, "indexed.js.map"), "utf8")).toBe(mapText); }, ); + +cliTest.for(["CLI", "stdio MCP"] as const)( + "preserves source-root declarations and URL path identity through %s", + async (surface, { cli }) => { + const root = await createTestTempDirectory("rea-source-root-identity-"); + const cases = [ + { + sourceRoot: "https://cdn.example/src//", + resolution: { kind: "suffix", path: "cdn.example/src//a.js" }, + }, + { + sourceRoot: "webpack://", + resolution: { + kind: "unresolved", + reason: + "Source reference uses a non-hierarchical URL without a file pathname.", + }, + }, + { + sourceRoot: "webpack:///", + resolution: { kind: "suffix", path: "a.js" }, + }, + ] as const; + const maps = cases.map(({ sourceRoot }, index) => ({ + path: `${String(index)}.js.map`, + text: JSON.stringify({ + version: 3, + sourceRoot, + sources: ["a.js"], + names: [], + mappings: "AAAA", + }), + })); + await writeFixtureFiles( + root, + Object.fromEntries(maps.map(({ path, text }) => [path, text])), + ); + const { graph } = await analyzeJavaScriptDirectory(surface, cli, root); + for (const [index, { sourceRoot, resolution }] of cases.entries()) { + const map = maps[index]!; + expect(graph.nodes).toContainEqual( + expect.objectContaining({ + kind: "source-module", + identity: expect.objectContaining({ + strategy: "source-map-original", + source_map_sha256: createHash("sha256") + .update(map.text) + .digest("hex"), + original_source: "a.js", + }), + observations: expect.arrayContaining([ + expect.objectContaining({ + source_map_reference: { + source_name: "a.js", + source_root: sourceRoot, + map_path: map.path, + resolution, + }, + }), + ]), + }), + ); + expect(await readFile(join(root, map.path), "utf8")).toBe(map.text); + } + }, +);