Skip to content

Fix #9: emit source maps for transformed output - #14

Merged
sam-parsons merged 2 commits into
mainfrom
feature/9-emit-source-maps
Apr 26, 2026
Merged

sam-parsons merged 2 commits into
mainfrom
feature/9-emit-source-maps

Conversation

@sam-parsons

Copy link
Copy Markdown
Owner

Summary

  • Emit valid v3 source maps whenever transform changes module source, using magic-string removals keyed by original indices.
  • Compose maps with @jridgewell/remapping when the functions pass also normalizes line endings (split(/\r?\n/).join("\n")).
  • Fix whole-line removal ranges for the last line so multi-line call stripping matches existing join("\n") output (no stray newline after the sole kept line).
  • Document map behavior in the README and extend tests to assert non-empty mappings plus one originalPositionFor trace.

Issues

How to test

  • npm run check
  • npm run build

Notes

  • Runtime deps added: magic-string, @jridgewell/remapping (map composition only when LF normalization runs).
  • Dev dep added: @jridgewell/trace-mapping (tests only).
  • An internal guard throws if the map pipeline ever diverges from the string pipeline; that indicates a bug in range bookkeeping rather than user input.

Learning notes

Source map composition across sequential transforms

What it is
When two transforms run in sequence (here: delete ranges, then normalize newlines), each can emit its own map. A composed map traces generated positions through both steps back to the original file.

Where it appears in this PR
src/index.ts — after magic-string removals, optional LF normalization uses a second MagicString on the intermediate string; remapping([normalizeMap, removalMap], () => null) folds those maps so consumers only see one map rooted at the Rolldown module id.

Why it was the right call here
Returning only the first-stage map would mis-attribute columns whenever \r\n collapses to \n. Composing keeps the contract “map matches emitted code” without teaching the removal pass about newline normalization edge cases.

If you want to go deeper
@jridgewell/remapping README — “Multiple transformations of a file” (array order: most recent map first).

Collect removal ranges in original coordinates, apply them with magic-string, and compose maps when the functions pass normalizes line endings. Add tests that assert map presence and trace generated positions back to the source.
Comment thread test/strip.test.ts Outdated
});
expect(transformed?.code).toBe(["let x = 0;", "let z = 0;", "console.log('kept');"].join("\n"));
const raw = parseSourceMapJson(transformed!.map as { toString(): string });
expect((raw.mappings as string).length).toBeGreaterThan(0);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a deep assertion that can be made here and other places where toBeGreaterThan is used

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in latest push: added expectSourceMapHasTraceableSegments using eachMapping so we assert decoded segments exist, not only non-empty VLQ.

Comment thread src/index.ts Outdated
ms.remove(start, end);
}

const mid = ms.toString();

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we find a more descriptive name for this?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to codeAfterRemovals (and the same name in buildNormalizeMagicString) so it reads as the module text after all removal passes, before optional LF normalization.

Comment thread src/index.ts Outdated
const allRemovalRanges = mergeRanges([...dbgRanges, ...fnRangesOrig, ...labelRangesOrig]);

const ms = new MagicString(code);
const sortedRemovals = [...allRemovalRanges].sort((a, b) => b[0] - a[0]);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it is not obvious from the code, in a single line comment explain how you are sorting the array

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a one-line comment: ranges are sorted by descending start offset so we apply deletes from the end of the file toward the start (common batch-delete convention; all indices stay in original-file space).

Comment thread src/index.ts Outdated
function computeLineStarts(code: string, lines: string[]): number[] {
const starts: number[] = new Array(lines.length);
starts[0] = 0;
let pos = 0;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more descriptive name

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed the loop cursor to offsetInOriginalFile — it is the index walking code while aligning split(/\\r?\\n/) line boundaries to absolute positions for removal ranges.

Comment thread src/index.ts
chainedCalls?: ChainedCallsPolicy;
}

const STRIP_INTERMEDIATE = "\0rolldown-plugin-strip/intermediate";

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this doing and why is it necessary?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documented above the const: it is a virtual sources/file bridge name so remapping can chain the removal-stage map (output = synthetic intermediate) with the LF-normalization map (input = that same intermediate, output = real module id).

Rename removal-stage string and scanning offset for clarity, document virtual map filename and removal sort order, and assert source maps contain traceable segments via eachMapping.
@sam-parsons

Copy link
Copy Markdown
Owner Author

Pushed 148e40d addressing the inline review threads:

  • Tests: expectSourceMapHasTraceableSegments uses @jridgewell/trace-mapping eachMapping so we assert the map decodes to at least one traceable segment (not only non-empty mappings text).
  • index.ts: renamed mid → codeAfterRemovals, pos → offsetInOriginalFile, sortedRemovals → sortedRemovalRangesDescending + sort-order comment, and a one-line explanation of STRIP_INTERMEDIATE for remapping.

Replies are threaded under each original comment.

@sam-parsons
sam-parsons merged commit 35fe480 into main Apr 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue G: Emit source maps for transformed output

1 participant