Skip to content

transpiler: emit v3 source map from transformSync / transform#30540

Open
robobun wants to merge 12 commits into
mainfrom
farm/2164512e/transpiler-sourcemap-output
Open

transpiler: emit v3 source map from transformSync / transform#30540
robobun wants to merge 12 commits into
mainfrom
farm/2164512e/transpiler-sourcemap-output

Conversation

@robobun

@robobun robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Problem

Bun.Transpiler already accepted a sourcemap constructor option (parsed at src/runtime/api/JSTranspiler.zig:277), but the option was consumed nowhere else — transformSync/transform returned a plain string with no way for downstream tools to recover original line numbers after TS stripping.

Minimal repro from #30538, before:

const t = new Bun.Transpiler({ loader: 'ts', sourcemap: 'inline' });
const out = t.transformSync(
  `interface Foo { x: number }
const x: number = 5;

interface Bar { y: string }

const y: number = 10;`
);
// out === "const x = 5;\nconst y = 10;\n"   ← no map, anywhere

This makes it impossible for frameworks that build per-file compile pipelines around Bun.Transpiler (SFC compilers, TS-stripping loaders, custom transformer plugins) to chain sourcemaps — stack traces land on generated files instead of the .ts the user wrote.

Fix

Thread the printer's SourceMapHandler into transformSync and the async TransformTask.run() path. When sourcemap is set, the resulting v3 map is exposed to JavaScript via one of four shapes, matching how Bun.build's sourcemap option already behaves:

sourcemap value return type code map
unset / false / "none" string (unchanged) transpiled code
true / "inline" string code + //# sourceMappingURL=data:application/json;base64,…
"external" { code, map } transpiled code (no footer) v3 JSON map
"linked" { code, map } code + //# sourceMappingURL=<name>.map v3 JSON map

Decoding the map from the issue's repro now recovers:

  • generated line 0 (const x = 5;) → original line 1 (const x: number = 5;)
  • generated line 1 (const y = 10;) → original line 5 (const y: number = 10;)

Backwards-compatible — callers that don't set sourcemap (the overwhelming default) keep getting a plain string.

Testing

Added Bun.Transpiler sourcemap option describe block to test/bundler/transpiler/transpiler.test.js (10 tests):

  • Plain-string return is preserved for unset / false / "none".
  • "inline" and true append the base64 data URL.
  • "external" returns { code, map } with no footer on code.
  • "linked" returns { code, map } with a //# sourceMappingURL=<name>.map comment on code.
  • Decoded VLQ mappings round-trip generated lines back to the original lines from the issue's repro.
  • transform() (async) mirrors all three shapes.

Gate: USE_SYSTEM_BUN=1 bun test … -t sourcemap → 7 fail / 3 pass (pass = backwards-compat shape). bun bd test … → 10/10 pass.

Closes #30538

@robobun

robobun commented May 12, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Add v3 source-map emission to Bun.Transpiler (modes: none, inline, external, linked); thread mode through task state; emit maps in async and sync paths; narrow TypeScript return types; add runtime tests and tsd fixture.

Changes

Transpiler sourcemap support

Layer / File(s) Summary
Type contract for source map support
packages/bun-types/bun.d.ts
Introduce TranspilerSourceMapOption; make TranspilerOptions<SM> generic and sourcemap?: SM; add TranspilerTransformResult and conditional TranspilerTransformReturn<SM>; make Transpiler<const SM> generic so transform/transformSync return types narrow by sourcemap mode.
Task state and initialization
src/runtime/api/JSTranspiler.zig
TransformTask adds output_map: bun.String and source_map: api.SourceMapMode; TransformTask.create copies configured source_map into the task.
Async source map capture and return
src/runtime/api/JSTranspiler.zig
Async run() captures source-map chunks when enabled, synthesizes empty v3 maps for empty parses when requested, populates output_code/output_map per `.none
SourceMapCapture helper
src/runtime/api/JSTranspiler.zig
Add SourceMapCapture to accumulate v3 JSON maps and emit an empty map when needed.
Sync source map support with helpers
src/runtime/api/JSTranspiler.zig
transformSync uses SourceMapCapture and buildTransformResult; add sourceMapURLFor, appendInlineSourceMap, and createCodeMapObject to format results for each mode.
Comprehensive source map test suite
test/bundler/transpiler/transpiler.test.js
Add tests for unset/false/"none" (plain string), "inline"/true (string with inline base64 v3 map), "external" (returns { code, map }), and "linked" (returns { code, map } with .map footer); include VLQ decoder and mappings assertions; cover async/sync and empty/type-only parity and per-call loader behavior.
Type-level tests / integration fixture
test/integration/bun-types/fixture/transpiler.ts
Add tsd fixture asserting transformSync/transform return string for none/inline/true and Bun.TranspilerTransformResult ({ code, map }) for external/linked; include regression guards for explicit option narrowing.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: implementing v3 source map emission from the transformSync and transform methods of Bun.Transpiler.
Description check ✅ Passed The description comprehensively covers the problem (sourcemap option was not consumed), the fix (threading SourceMapHandler through both paths with four return shapes), and testing approach with concrete examples.
Linked Issues check ✅ Passed The code changes fully address issue #30538 by implementing v3 sourcemap emission from transformSync/transform with four return shapes (none/inline/external/linked) and comprehensive type definitions, matching the feature request.
Out of Scope Changes check ✅ Passed All changes directly support the core objective: TypeScript type definitions (generics, conditional return types), v3 sourcemap implementation in both sync/async paths, and comprehensive test coverage with type-level assertions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bun-types/bun.d.ts`:
- Line 2385: The default generic parameter for TranspilerOptions narrows
sourcemap to "none" and breaks explicit-typed option objects; change the generic
default on the TranspilerOptions declaration so SM defaults to the full
TranspilerSourceMapOption union (i.e., SM = TranspilerSourceMapOption) instead
of the literal "none" so that constructs like const opts: Bun.TranspilerOptions
= { sourcemap: "external" } type-check correctly while still allowing callers to
explicitly narrow SM (e.g., TranspilerOptions<"external">).

In `@test/integration/bun-types/fixture/transpiler.ts`:
- Around line 51-56: Add an async assertion for the "linked" sourcemap case:
after creating the Bun.Transpiler instance (const t = new Bun.Transpiler({
loader: "ts", sourcemap: "linked" })), call t.transform("const x: number = 1;")
and assert its return type is Promise<Bun.TranspilerTransformResult> (similar to
the existing checks for "external"); keep the existing transformSync assertion
unchanged and add the transform() promise type check to cover the async return
shape and guard against generic regressions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6eaa3d17-622b-43f9-9ec3-7d74b6e5840b

📥 Commits

Reviewing files that changed from the base of the PR and between 49cfffc and 711bb07.

📒 Files selected for processing (2)
  • packages/bun-types/bun.d.ts
  • test/integration/bun-types/fixture/transpiler.ts

Comment thread packages/bun-types/bun.d.ts Outdated
Comment thread test/integration/bun-types/fixture/transpiler.ts
Comment thread src/runtime/api/JSTranspiler.zig Outdated
Comment thread packages/bun-types/bun.d.ts Outdated
Comment thread src/runtime/api/JSTranspiler.zig
robobun added a commit that referenced this pull request May 12, 2026
…line-terminate inline footer

Three fixes from coderabbit review on #30540:

1. Empty / type-only inputs diverged between sync and async. The async
   path early-returned with output_code = empty when parse_result.empty
   or printed == 0, never populating output_map, and finish() branched
   on output_map.isEmpty() — so `await t.transform("interface Foo {}")`
   with sourcemap: "external" resolved to `""` instead of
   `{ code: "", map }`, while transformSync correctly returned the
   object shape.

   Fix: branch finish() on source_map (not output_map.isEmpty()), drop
   the printed == 0 early-return, and when parse_result.empty + a map
   was requested, synthesize an empty v3 map via SourceMapCapture.writeEmpty
   so the return shape is preserved.

2. The inline data URL footer lacked a trailing newline, while the
   .linked branch next to it and Bun.build's inline emitter (generate
   ChunksInParallel.zig) both append one. Without the newline, naive
   concatenation of two transformer outputs silently swallows the
   next file into the `//#` line comment.

   Fix: always append `\n` after the base64 payload.

3. Incidental: dropped a stale `buffer_writer.buffer.list.items =
   buffer_writer.written` assignment that trashed the buffer's backing
   allocation when the printer was skipped (crash surfaced under ASAN
   once the early-returns were removed). The assignment was a no-op
   in the printer-ran path and dangerous otherwise.

Regression tests cover empty and type-only inputs across external/
linked/inline, both sync and async; plus an `endsWith('\n')` assertion
for inline output.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/api/JSTranspiler.zig`:
- Around line 1264-1275: The function sourceMapURLFor currently uses the entire
virtual source_path (which may be absolute or contain directories) leading to
root-relative sourceMappingURL; change it to derive the basename (strip leading
directories and path separators) before appending ".map" so linked mode emits a
sibling filename. Concretely, inside sourceMapURLFor compute an effective base
by finding the last path separator ('/' and maybe '\\') in source_path and using
the substring after it (falling back to "input" when result is empty), then
append the ".map" suffix into buf and return that slice; keep the existing
bun.assert and memcpy usage but use the basename length when copying.

In `@test/bundler/transpiler/transpiler.test.js`:
- Around line 3865-3871: Convert the two for...of parameterized blocks into
describe.each() tables per repo convention: replace the loop that creates
Bun.Transpiler({ loader: "ts", sourcemap: value }) and asserts
transformSync(input) returns a plain string (no sourceMappingURL) with a
describe.each([...]) block and individual it cases for each sourcemap value;
likewise update the empty-input sync matrix (the tests that exercise
transformSync on empty input) to include sync coverage for "linked" and "inline"
in addition to the existing "external" case so the sync path’s empty-input
handling is fully tested; target the Bun.Transpiler constructor and
transformSync calls when making these changes.
- Around line 3807-3819: The decodeVLQ function can enter an infinite loop when
charset.indexOf returns -1 for a malformed segment; update decodeVLQ to validate
the digit after computing charset.indexOf(seg[offset++]) and immediately throw a
clear Error (including the invalid character and offset) if digit === -1, so the
code fails fast on invalid VLQ digits instead of hanging; reference the
decodeVLQ function and the local variables digit and offset when making this
change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 824bdd83-fb3a-4441-a1c4-df4fc0c1678a

📥 Commits

Reviewing files that changed from the base of the PR and between 2b00f17 and b477f20.

📒 Files selected for processing (2)
  • src/runtime/api/JSTranspiler.zig
  • test/bundler/transpiler/transpiler.test.js

Comment thread src/runtime/api/JSTranspiler.zig
Comment thread test/bundler/transpiler/transpiler.test.js
Comment thread test/bundler/transpiler/transpiler.test.js Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Additional findings (outside current diff — PR may have been updated during review):

  • 🟡 src/runtime/api/JSTranspiler.zig:1170-1172 — nit: getParseResult() derives the virtual source name from this.config.default_loader.stdinName(), ignoring the per-call loader override, while the async path (TransformTask.run()) uses the override — so new Bun.Transpiler({ sourcemap: "external" }).transformSync(code, "ts") emits sources:["/input.jsx"] / sourceMappingURL=input.jsx.map but await .transform(code, "ts") emits /input.ts / input.ts.map. The default_loader-vs-override naming is pre-existing, but this PR is the first to surface source.path.text to JS via the new sourcemap output, making the sync/async divergence user-observable. One-line fix: const name = (loader orelse this.config.default_loader).stdinName(); in getParseResult.

    Extended reasoning...

    What the bug is

    The synthetic source filename that ends up in the v3 map's sources array and in the //# sourceMappingURL=<name>.map footer differs between transformSync() and await transform() when a per-call loader override is passed.

    • Sync pathtransformSync calls getParseResult(this, allocator, code, loader, …), which builds the virtual source via const name = this.config.default_loader.stdinName(); (src/runtime/api/JSTranspiler.zig:~870). The per-call loader argument is threaded into ParseOptions.loader, but it is not used to pick the source name. transformSync then passes parse_result.source.path.text straight into buildTransformResult(…, parse_result.source.path.text).
    • Async pathTransformTask.create stores loader orelse this.config.default_loader into task.loader, and TransformTask.run() does const name = this.loader.stdinName(); — i.e. the per-call override.

    buildTransformResult / the async switch then use that path in two places this PR introduces:

    1. sourceMapURLFor(parse_result.source.path.text, …) → the //# sourceMappingURL=<name>.map footer in linked mode.
    2. SourceMapCapture.onChunkchunk.printSourceMapContents(source, …), which writes source.path.text into the v3 map's "sources" array.

    Step-by-step proof

    const t = new Bun.Transpiler({ sourcemap: "external" }); // default_loader = jsx

    Sync: t.transformSync(code, "ts")

    1. transformSync resolves loader = .ts from the second argument.
    2. getParseResult ignores loader for the name and sets name = this.config.default_loader.stdinName()"input.jsx".
    3. parse_result.source.path.text == "input.jsx" is passed to buildTransformResult.
    4. SourceMapCapture.onChunk writes "sources":["/input.jsx"]; in linked mode the footer is //# sourceMappingURL=input.jsx.map.

    Async: await t.transform(code, "ts")

    1. transform resolves loader = .ts and calls TransformTask.create(this, code, globalThis, .ts).
    2. TransformTask.run sets name = this.loader.stdinName()"input.ts".
    3. The map's "sources" is ["/input.ts"]; in linked mode the footer is //# sourceMappingURL=input.ts.map.

    Same transpiler, same input, same per-call loader → different observable output depending on sync vs async.

    Why nothing prevents it

    The new tests always set loader: "ts" in the constructor and never pass a per-call loader to transformSync/transform, so default_loader and the override always agree and the divergence isn't exercised.

    Why this is a nit, not a blocker

    The underlying getParseResult behavior (using default_loader for the stdin name regardless of the per-call override) is pre-existing — this PR just makes source.path.text user-visible for the first time via the new sourcemap output. And the impact is cosmetic: the name is a synthetic placeholder (input.<ext>) that doesn't correspond to a real file; the VLQ mappings and sourcesContent are correct; and most callers set loader once in the constructor rather than per call. Still, sync and async should agree on output for identical inputs, and it's a one-line fix.

    Fix

    In getParseResult:

    const name = (loader orelse this.config.default_loader).stdinName();

    so the sync path matches TransformTask.run()'s use of the per-call override.

Comment thread src/runtime/api/JSTranspiler.zig

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (3)
src/runtime/api/JSTranspiler.zig (1)

1273-1280: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Strip directories before building the linked map filename.

source_path here is the virtual parser path (/input.ts in these tests), so appending .map directly yields /input.ts.map. That makes the footer root-relative instead of a sibling filename and breaks the advertised "linked" contract.

Proposed fix
 fn sourceMapURLFor(source_path: []const u8, buf: *bun.PathBuffer) []const u8 {
-    const base = if (source_path.len == 0) "input" else source_path;
+    const source = if (source_path.len == 0) "input" else source_path;
+    var last_sep: ?usize = null;
+    for (source, 0..) |ch, i| {
+        if (ch == '/' or ch == '\\') last_sep = i;
+    }
+    const candidate = if (last_sep) |i| source[i + 1 ..] else source;
+    const base = if (candidate.len == 0) "input" else candidate;
     const suffix = ".map";
     const total = base.len + suffix.len;
     bun.assert(total <= buf.len);
     `@memcpy`(buf[0..base.len], base);
     `@memcpy`(buf[base.len..][0..suffix.len], suffix);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/api/JSTranspiler.zig` around lines 1273 - 1280, sourceMapURLFor
currently appends ".map" to the full virtual parser path (source_path) which can
produce a root-relative path (e.g. "/input.ts.map"); change it to use only the
basename portion before appending the suffix so the resulting map filename is a
sibling name. In sourceMapURLFor, compute base by taking the substring after the
last path separator in source_path (if none or result is empty, fall back to
"input"); then append ".map" into buf as you already do and return
buf[0..total]; reference function sourceMapURLFor, variables source_path, base,
suffix, and buf when making the change. Ensure you handle both '/' and '\\' as
separators if needed.
test/bundler/transpiler/transpiler.test.js (2)

3807-3819: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fail fast on malformed VLQ digits.

If charset.indexOf(...) returns -1, (digit & 32) !== 0 stays truthy and this loop never terminates. A broken mappings string would hang the suite instead of failing clearly.

Proposed fix
     function decodeVLQ(seg, offset) {
       let value = 0;
       let shift = 0;
       let cont = true;
       while (cont) {
-        const digit = charset.indexOf(seg[offset++]);
+        const char = seg[offset++];
+        const digit = charset.indexOf(char);
+        if (digit === -1) {
+          throw new Error(`Invalid VLQ digit "${char}" at offset ${offset - 1} in "${seg}"`);
+        }
         cont = (digit & 32) !== 0;
         value += (digit & 31) << shift;
         shift += 5;
       }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/bundler/transpiler/transpiler.test.js` around lines 3807 - 3819, In
decodeVLQ, guard against invalid VLQ characters by checking the result of
charset.indexOf(...) (the local symbol digit) and throwing a clear error when
digit === -1 so the loop cannot spin forever; update the while loop in function
decodeVLQ to validate digit before using it (and include context in the thrown
error, e.g., the offending character and offset) so malformed mappings fail fast
instead of hanging the test suite.

3865-3871: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Use describe.each() for these matrices, and cover the sync empty-input branches too.

These are parameterized cases, so they should follow the repo’s describe.each() convention. While touching the empty-input table, please add sync assertions for "linked" and "inline" as well—the sync path has separate result-shaping branches for those modes, and they’re only exercised async right now.

As per coding guidelines, Use describe.each() for parameterized tests.

Also applies to: 4013-4052

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/bundler/transpiler/transpiler.test.js` around lines 3865 - 3871, Replace
the manual for-loop parameterization with the repo convention of describe.each()
to drive the matrix (e.g., describe.each([[false], ["none"]]) for the sourcemap
modes) and move the existing assertions on Bun.Transpiler/transformSync into
that table-driven block; additionally, extend the empty-input test table to
include sync assertions for the "linked" and "inline" sourcemap modes (call
transformSync(input) and assert the shaped results mirror the async
expectations) so the separate sync result-shaping branches in transformSync are
exercised; apply the same describe.each() refactor pattern to the other affected
block range around lines 4013-4052.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@src/runtime/api/JSTranspiler.zig`:
- Around line 1273-1280: sourceMapURLFor currently appends ".map" to the full
virtual parser path (source_path) which can produce a root-relative path (e.g.
"/input.ts.map"); change it to use only the basename portion before appending
the suffix so the resulting map filename is a sibling name. In sourceMapURLFor,
compute base by taking the substring after the last path separator in
source_path (if none or result is empty, fall back to "input"); then append
".map" into buf as you already do and return buf[0..total]; reference function
sourceMapURLFor, variables source_path, base, suffix, and buf when making the
change. Ensure you handle both '/' and '\\' as separators if needed.

In `@test/bundler/transpiler/transpiler.test.js`:
- Around line 3807-3819: In decodeVLQ, guard against invalid VLQ characters by
checking the result of charset.indexOf(...) (the local symbol digit) and
throwing a clear error when digit === -1 so the loop cannot spin forever; update
the while loop in function decodeVLQ to validate digit before using it (and
include context in the thrown error, e.g., the offending character and offset)
so malformed mappings fail fast instead of hanging the test suite.
- Around line 3865-3871: Replace the manual for-loop parameterization with the
repo convention of describe.each() to drive the matrix (e.g.,
describe.each([[false], ["none"]]) for the sourcemap modes) and move the
existing assertions on Bun.Transpiler/transformSync into that table-driven
block; additionally, extend the empty-input test table to include sync
assertions for the "linked" and "inline" sourcemap modes (call
transformSync(input) and assert the shaped results mirror the async
expectations) so the separate sync result-shaping branches in transformSync are
exercised; apply the same describe.each() refactor pattern to the other affected
block range around lines 4013-4052.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3027959a-df68-4e04-984d-7158314dff6e

📥 Commits

Reviewing files that changed from the base of the PR and between b477f20 and 4b50431.

📒 Files selected for processing (2)
  • src/runtime/api/JSTranspiler.zig
  • test/bundler/transpiler/transpiler.test.js

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All my earlier inline findings look addressed — thanks. I'm deferring because this introduces new public API surface (generic Transpiler<SM> + conditional return types in CODEOWNER-protected bun.d.ts) alongside ~300 lines of transpiler-runtime Zig, and the build-zig CI jobs are still red on 8b57a7a.

Extended reasoning...

Overview

This PR wires the previously-parsed-but-ignored sourcemap option through Bun.Transpiler.transform / transformSync, emitting v3 source maps in four modes (none, inline, external, linked). It touches:

  • src/runtime/api/JSTranspiler.zig (~300 net lines): new SourceMapCapture helper, threads SourceMapHandler through both sync and async print paths, adds buildTransformResult / appendInlineSourceMap / sourceMapURLFor / createCodeMapObject, reworks TransformTask.run() and finish() to populate and return { code, map }, and changes getParseResult to honor the per-call loader for the virtual source name.
  • packages/bun-types/bun.d.ts: makes Transpiler and TranspilerOptions generic over a const SM extends TranspilerSourceMapOption so return types narrow to string vs TranspilerTransformResult.
  • test/bundler/transpiler/transpiler.test.js: ~330 lines of new tests including a hand-rolled VLQ decoder, empty-input parity matrix, per-call-loader parity, and target: "bun" internal-format regression coverage.
  • test/integration/bun-types/fixture/transpiler.ts: new tsd fixture pinning the conditional return types.

Security risks

None identified. Inputs are user-supplied source strings already flowing through the existing parser/printer; the new code only formats printer output and source-map JSON. No filesystem, network, auth, or eval surface is introduced.

Level of scrutiny

High. This is a user-facing feature on a public API (Bun.Transpiler), not a mechanical fix. Specific reasons for human review:

  • packages/bun-types/bun.d.ts is CODEOWNER-protected. The new generic-class design (Transpiler<const SM>, TranspilerTransformReturn<SM>) is an API-shape decision that the types owner should sign off on — particularly the choice that TranspilerOptions (without a type argument) widens the return to the union, and that the class default SM = "none" keeps existing callers at string.
  • The Zig changes rewire the hot path of transformSync and the async TransformTask, including buffer-writer reuse semantics (buffer_writer.written vs buffer_writer.buffer.list.items after appending the footer) and a behavior change in getParseResult (per-call loader now drives the virtual source name). These deserve a maintainer's eye for memory-ownership and pooling correctness.
  • CI is currently failing: build-zig / build-cpp shards are red on 8b57a7a across all platforms. The PR can't be evaluated as merge-ready until those compile.

Other factors

All four of my earlier inline findings (async/sync empty-output shape divergence, missing trailing newline on inline footer, type-level break for default callers, and target: "bun" emitting the packed internal-sourcemap binary into mappings) have been addressed in 2b00f17 / b477f20 / 8b57a7a, with regression tests added for each. CodeRabbit's comments are likewise resolved. No new bugs were found by the bug-hunting system on this revision. Test coverage for the new behavior is thorough. The remaining gate is API sign-off plus green CI.

robobun added a commit that referenced this pull request May 12, 2026
Flaky test failures on x64-asan / windows-x64 unrelated to the
Transpiler sourcemap diff:

- fetch-http2-client: WebKit AtomString wasRemoved assertion
  (h2 client under ASAN has recent deflaking history — #29954, #29809)
- test-worker-nested-uncaught: panic
  "EventLoop.enqueueTaskConcurrent: VM has terminated" — worker VM
  teardown race
- test-http-should-emit-close-when-connection-is-aborted: same failure
  landed on PR #30540's neighbour build 53635
- AsyncLocalStorage-tracking / bun-install-registry: async_hooks /
  install flakes

None of these touch Bun.Transpiler / js_printer / sourcemap. All
Linux/Darwin/FreeBSD build-zig and build-cpp lanes passed.
robobun added a commit that referenced this pull request May 23, 2026
…line-terminate inline footer

Three fixes from coderabbit review on #30540:

1. Empty / type-only inputs diverged between sync and async. The async
   path early-returned with output_code = empty when parse_result.empty
   or printed == 0, never populating output_map, and finish() branched
   on output_map.isEmpty() — so `await t.transform("interface Foo {}")`
   with sourcemap: "external" resolved to `""` instead of
   `{ code: "", map }`, while transformSync correctly returned the
   object shape.

   Fix: branch finish() on source_map (not output_map.isEmpty()), drop
   the printed == 0 early-return, and when parse_result.empty + a map
   was requested, synthesize an empty v3 map via SourceMapCapture.writeEmpty
   so the return shape is preserved.

2. The inline data URL footer lacked a trailing newline, while the
   .linked branch next to it and Bun.build's inline emitter (generate
   ChunksInParallel.zig) both append one. Without the newline, naive
   concatenation of two transformer outputs silently swallows the
   next file into the `//#` line comment.

   Fix: always append `\n` after the base64 payload.

3. Incidental: dropped a stale `buffer_writer.buffer.list.items =
   buffer_writer.written` assignment that trashed the buffer's backing
   allocation when the printer was skipped (crash surfaced under ASAN
   once the early-returns were removed). The assignment was a no-op
   in the printer-ran path and dangerous otherwise.

Regression tests cover empty and type-only inputs across external/
linked/inline, both sync and async; plus an `endsWith('\n')` assertion
for inline output.
@robobun robobun force-pushed the farm/2164512e/transpiler-sourcemap-output branch from affc2ef to 63fc842 Compare May 23, 2026 02:37
robobun added a commit that referenced this pull request May 23, 2026
Flaky test failures on x64-asan / windows-x64 unrelated to the
Transpiler sourcemap diff:

- fetch-http2-client: WebKit AtomString wasRemoved assertion
  (h2 client under ASAN has recent deflaking history — #29954, #29809)
- test-worker-nested-uncaught: panic
  "EventLoop.enqueueTaskConcurrent: VM has terminated" — worker VM
  teardown race
- test-http-should-emit-close-when-connection-is-aborted: same failure
  landed on PR #30540's neighbour build 53635
- AsyncLocalStorage-tracking / bun-install-registry: async_hooks /
  install flakes

None of these touch Bun.Transpiler / js_printer / sourcemap. All
Linux/Darwin/FreeBSD build-zig and build-cpp lanes passed.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All prior review feedback has been addressed and the bug-hunter found nothing new, but this is a ~600-line feature touching the transpiler print path in both the Zig and Rust runtimes plus a public API-shape change (generic Transpiler<SM>), so it's worth a human pass — note also that Build #57112 is currently red.

Extended reasoning...

Overview

This PR wires the previously-parsed-but-ignored sourcemap option through Bun.Transpiler.transformSync / transform, emitting a v3 source map in four shapes (none/inline/external/linked) to match Bun.build. It touches:

  • src/runtime/api/JSTranspiler.zig and its Rust port JSTranspiler.rs — new SourceMapCapture helper, threading SourceMapHandler into both the sync print path and the async TransformTask, plus result-shaping helpers (buildTransformResult, appendInlineSourceMap, createCodeMapObject, sourceMapURLFor).
  • packages/bun-types/bun.d.ts — makes Transpiler and TranspilerOptions generic over the sourcemap literal so the return type narrows to string vs {code, map}.
  • ~335 lines of runtime tests + a tsd type fixture.

Security risks

None identified. Inputs are user-supplied source strings already flowing through the existing parser/printer; the new code only post-processes the printer's output (base64-encodes the JSON map, builds a {code, map} object). No filesystem, network, or privilege boundaries are crossed.

Level of scrutiny

High. This is a new user-facing API surface with a documented return-shape contract, implemented in parallel across two runtimes (Zig + the in-progress Rust port) that must stay behaviorally identical. It involves bun.String refcounting across an async task boundary, MutableString ownership inside an arena, and a public-type change (conditional generic return) that affects every downstream @types/bun consumer. None of that is mechanical.

Other factors

  • All four of my earlier inline findings (empty-output async shape divergence, type-level back-compat, missing trailing newline, target: "bun" internal-format VLQ) and CodeRabbit's points have been addressed and resolved in follow-up commits, with regression tests added for each.
  • The current bug-hunt pass found nothing new.
  • The robobun status comment reports CI failures on the latest commit (Build #57112), which on its own blocks approval.
  • The API-shape decision (generic class vs. overloads vs. union return) is a design call a maintainer should sign off on.

Comment thread packages/bun-types/bun.d.ts

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All prior review feedback looks addressed and I didn't find anything new this pass — deferring to a human for sign-off since this adds a public API surface (conditional {code, map} return shape + generic Transpiler<SM>) and threads new state through both the Zig and Rust transform paths.

Extended reasoning...

Overview

This PR wires the previously-parsed-but-ignored sourcemap option through Bun.Transpiler.transformSync / transform, emitting a v3 source map in four shapes (none/inline/external/linked) that mirror Bun.build. It touches src/runtime/api/JSTranspiler.zig and its Rust port JSTranspiler.rs (new SourceMapCapture helper, TransformTask state, result-building helpers), makes Transpiler generic over its sourcemap option in bun.d.ts so return types narrow, and adds ~330 lines of runtime tests plus a tsd fixture.

Security risks

None identified. No auth, crypto, network, or filesystem-write surface; inputs are user-supplied source strings already flowing through the existing parser/printer. The new code paths assemble JSON/base64 from printer output and hand it back to JS.

Level of scrutiny

This warrants human review rather than auto-approval: it's a ~600-line net feature change to core runtime in two languages, introduces a new public API contract (return type changes shape based on a constructor option, expressed via a class-level generic), and carries memory-lifecycle additions (output_map deref, SourceMapCapture deinit, arena-backed JSON buffer). Several real bugs were found and fixed across earlier review rounds (async/sync empty-output divergence, target: "bun" internal-format mappings, type-level back-compat, JSDoc orphaning), which is exactly the kind of PR where a maintainer should confirm the final shape.

Other factors

All prior inline comments (mine and CodeRabbit's) are resolved, with regression tests added for each. Test coverage is thorough — return-shape matrix across sync/async, empty/type-only inputs, per-call loader override parity, target: "bun" VLQ re-encoding, and tsd assertions for the conditional generic. The default path (sourcemap unset) is unchanged at runtime and at the type level. Nothing blocking from my side; deferring purely on scope and API-design grounds.

robobun added a commit that referenced this pull request May 23, 2026
…line-terminate inline footer

Three fixes from coderabbit review on #30540:

1. Empty / type-only inputs diverged between sync and async. The async
   path early-returned with output_code = empty when parse_result.empty
   or printed == 0, never populating output_map, and finish() branched
   on output_map.isEmpty() — so `await t.transform("interface Foo {}")`
   with sourcemap: "external" resolved to `""` instead of
   `{ code: "", map }`, while transformSync correctly returned the
   object shape.

   Fix: branch finish() on source_map (not output_map.isEmpty()), drop
   the printed == 0 early-return, and when parse_result.empty + a map
   was requested, synthesize an empty v3 map via SourceMapCapture.writeEmpty
   so the return shape is preserved.

2. The inline data URL footer lacked a trailing newline, while the
   .linked branch next to it and Bun.build's inline emitter (generate
   ChunksInParallel.zig) both append one. Without the newline, naive
   concatenation of two transformer outputs silently swallows the
   next file into the `//#` line comment.

   Fix: always append `\n` after the base64 payload.

3. Incidental: dropped a stale `buffer_writer.buffer.list.items =
   buffer_writer.written` assignment that trashed the buffer's backing
   allocation when the printer was skipped (crash surfaced under ASAN
   once the early-returns were removed). The assignment was a no-op
   in the printer-ran path and dangerous otherwise.

Regression tests cover empty and type-only inputs across external/
linked/inline, both sync and async; plus an `endsWith('\n')` assertion
for inline output.
robobun added a commit that referenced this pull request May 23, 2026
Flaky test failures on x64-asan / windows-x64 unrelated to the
Transpiler sourcemap diff:

- fetch-http2-client: WebKit AtomString wasRemoved assertion
  (h2 client under ASAN has recent deflaking history — #29954, #29809)
- test-worker-nested-uncaught: panic
  "EventLoop.enqueueTaskConcurrent: VM has terminated" — worker VM
  teardown race
- test-http-should-emit-close-when-connection-is-aborted: same failure
  landed on PR #30540's neighbour build 53635
- AsyncLocalStorage-tracking / bun-install-registry: async_hooks /
  install flakes

None of these touch Bun.Transpiler / js_printer / sourcemap. All
Linux/Darwin/FreeBSD build-zig and build-cpp lanes passed.
@robobun robobun force-pushed the farm/2164512e/transpiler-sourcemap-output branch from 48c9176 to 7c1be99 Compare May 23, 2026 06:52
robobun added a commit that referenced this pull request May 25, 2026
…line-terminate inline footer

Three fixes from coderabbit review on #30540:

1. Empty / type-only inputs diverged between sync and async. The async
   path early-returned with output_code = empty when parse_result.empty
   or printed == 0, never populating output_map, and finish() branched
   on output_map.isEmpty() — so `await t.transform("interface Foo {}")`
   with sourcemap: "external" resolved to `""` instead of
   `{ code: "", map }`, while transformSync correctly returned the
   object shape.

   Fix: branch finish() on source_map (not output_map.isEmpty()), drop
   the printed == 0 early-return, and when parse_result.empty + a map
   was requested, synthesize an empty v3 map via SourceMapCapture.writeEmpty
   so the return shape is preserved.

2. The inline data URL footer lacked a trailing newline, while the
   .linked branch next to it and Bun.build's inline emitter (generate
   ChunksInParallel.zig) both append one. Without the newline, naive
   concatenation of two transformer outputs silently swallows the
   next file into the `//#` line comment.

   Fix: always append `\n` after the base64 payload.

3. Incidental: dropped a stale `buffer_writer.buffer.list.items =
   buffer_writer.written` assignment that trashed the buffer's backing
   allocation when the printer was skipped (crash surfaced under ASAN
   once the early-returns were removed). The assignment was a no-op
   in the printer-ran path and dangerous otherwise.

Regression tests cover empty and type-only inputs across external/
linked/inline, both sync and async; plus an `endsWith('\n')` assertion
for inline output.
robobun added a commit that referenced this pull request May 25, 2026
Flaky test failures on x64-asan / windows-x64 unrelated to the
Transpiler sourcemap diff:

- fetch-http2-client: WebKit AtomString wasRemoved assertion
  (h2 client under ASAN has recent deflaking history — #29954, #29809)
- test-worker-nested-uncaught: panic
  "EventLoop.enqueueTaskConcurrent: VM has terminated" — worker VM
  teardown race
- test-http-should-emit-close-when-connection-is-aborted: same failure
  landed on PR #30540's neighbour build 53635
- AsyncLocalStorage-tracking / bun-install-registry: async_hooks /
  install flakes

None of these touch Bun.Transpiler / js_printer / sourcemap. All
Linux/Darwin/FreeBSD build-zig and build-cpp lanes passed.
@robobun robobun force-pushed the farm/2164512e/transpiler-sourcemap-output branch from 7c1be99 to da23686 Compare May 25, 2026 00:10
robobun added a commit that referenced this pull request May 25, 2026
…line-terminate inline footer

Three fixes from coderabbit review on #30540:

1. Empty / type-only inputs diverged between sync and async. The async
   path early-returned with output_code = empty when parse_result.empty
   or printed == 0, never populating output_map, and finish() branched
   on output_map.isEmpty() — so `await t.transform("interface Foo {}")`
   with sourcemap: "external" resolved to `""` instead of
   `{ code: "", map }`, while transformSync correctly returned the
   object shape.

   Fix: branch finish() on source_map (not output_map.isEmpty()), drop
   the printed == 0 early-return, and when parse_result.empty + a map
   was requested, synthesize an empty v3 map via SourceMapCapture.writeEmpty
   so the return shape is preserved.

2. The inline data URL footer lacked a trailing newline, while the
   .linked branch next to it and Bun.build's inline emitter (generate
   ChunksInParallel.zig) both append one. Without the newline, naive
   concatenation of two transformer outputs silently swallows the
   next file into the `//#` line comment.

   Fix: always append `\n` after the base64 payload.

3. Incidental: dropped a stale `buffer_writer.buffer.list.items =
   buffer_writer.written` assignment that trashed the buffer's backing
   allocation when the printer was skipped (crash surfaced under ASAN
   once the early-returns were removed). The assignment was a no-op
   in the printer-ran path and dangerous otherwise.

Regression tests cover empty and type-only inputs across external/
linked/inline, both sync and async; plus an `endsWith('\n')` assertion
for inline output.
robobun added a commit that referenced this pull request May 25, 2026
Flaky test failures on x64-asan / windows-x64 unrelated to the
Transpiler sourcemap diff:

- fetch-http2-client: WebKit AtomString wasRemoved assertion
  (h2 client under ASAN has recent deflaking history — #29954, #29809)
- test-worker-nested-uncaught: panic
  "EventLoop.enqueueTaskConcurrent: VM has terminated" — worker VM
  teardown race
- test-http-should-emit-close-when-connection-is-aborted: same failure
  landed on PR #30540's neighbour build 53635
- AsyncLocalStorage-tracking / bun-install-registry: async_hooks /
  install flakes

None of these touch Bun.Transpiler / js_printer / sourcemap. All
Linux/Darwin/FreeBSD build-zig and build-cpp lanes passed.
@robobun robobun force-pushed the farm/2164512e/transpiler-sourcemap-output branch from da23686 to d6164a1 Compare May 25, 2026 04:10

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All earlier review feedback has been addressed and I have no new findings, but this is a ~500-line public-API addition (new return shapes for transformSync/transform, generic Transpiler<SM>, dual Zig + Rust runtime paths) that warrants a human sign-off — and transpiler.test.js is currently red on the Windows shards.

Extended reasoning...

Overview

This PR wires the previously-parsed-but-ignored sourcemap option through Bun.Transpiler's transformSync / transform, emitting a v3 source map in four shapes (none/inline/external/linked). It touches:

  • packages/bun-types/bun.d.ts — makes Transpiler and TranspilerOptions generic over the sourcemap option, adds TranspilerTransformResult / TranspilerTransformReturn<SM> so return types narrow to string vs {code, map}.
  • src/runtime/api/JSTranspiler.zig and src/runtime/api/JSTranspiler.rs — parallel implementations: new SourceMapCapture helper, buildTransformResult / appendInlineSourceMap / sourceMapURLFor / createCodeMapObject, threading through both the sync path and the async TransformTask, handling the target: "bun" internal-format chunk re-encode, and synthesising empty maps for type-only/empty inputs.
  • ~330 lines of new tests in transpiler.test.js (VLQ decoder, shape parity, empty-input matrix, target: "bun" regression) plus a tsd fixture.

Security risks

None identified. Inputs are user-provided source strings already flowing through the existing parser/printer; the new code only formats the printer's existing source-map chunk into JSON/base64 and builds a small JS result object. No filesystem, network, auth, or privilege boundaries are crossed.

Level of scrutiny

High. This is a user-visible API change to a documented class:

  • It changes the return type of transformSync/transform (conditionally, via a generic), which is a public-contract decision a maintainer should ratify.
  • It has two parallel runtime implementations (Zig + Rust port) that must stay behaviourally identical — the kind of thing that benefits from a second pair of eyes on memory lifetimes (bun.String deref in TransformTask.deinit, arena-backed SourceMapCapture, the buffer_writer pooling interaction with the new footer appends).
  • Source-map correctness has subtle edge cases (internal-vs-VLQ chunk format, empty parses, per-call loader name) — I flagged several of these in earlier rounds and they were fixed, but that history itself argues for human review of the final shape.

Other factors

  • All four of my prior inline findings (async empty-output shape divergence, inline trailing-newline, target: "bun" binary-chunk bug, JSDoc orphaning) and CodeRabbit's findings have been addressed and resolved; the current bug-hunting pass found nothing.
  • Test coverage is thorough for the new behaviour, including regression cases for each fixed review item.
  • CI is not green: robobun reports test/bundler/transpiler/transpiler.test.js failing with exit code 1 on all three Windows shards (x64, x64-baseline, aarch64) at the latest commit. That should be resolved or understood before merge.
  • This is well outside the "simple / mechanical / config-tweak" bar for auto-approval.

robobun added a commit that referenced this pull request May 27, 2026
…line-terminate inline footer

Three fixes from coderabbit review on #30540:

1. Empty / type-only inputs diverged between sync and async. The async
   path early-returned with output_code = empty when parse_result.empty
   or printed == 0, never populating output_map, and finish() branched
   on output_map.isEmpty() — so `await t.transform("interface Foo {}")`
   with sourcemap: "external" resolved to `""` instead of
   `{ code: "", map }`, while transformSync correctly returned the
   object shape.

   Fix: branch finish() on source_map (not output_map.isEmpty()), drop
   the printed == 0 early-return, and when parse_result.empty + a map
   was requested, synthesize an empty v3 map via SourceMapCapture.writeEmpty
   so the return shape is preserved.

2. The inline data URL footer lacked a trailing newline, while the
   .linked branch next to it and Bun.build's inline emitter (generate
   ChunksInParallel.zig) both append one. Without the newline, naive
   concatenation of two transformer outputs silently swallows the
   next file into the `//#` line comment.

   Fix: always append `\n` after the base64 payload.

3. Incidental: dropped a stale `buffer_writer.buffer.list.items =
   buffer_writer.written` assignment that trashed the buffer's backing
   allocation when the printer was skipped (crash surfaced under ASAN
   once the early-returns were removed). The assignment was a no-op
   in the printer-ran path and dangerous otherwise.

Regression tests cover empty and type-only inputs across external/
linked/inline, both sync and async; plus an `endsWith('\n')` assertion
for inline output.
robobun added a commit that referenced this pull request May 27, 2026
Flaky test failures on x64-asan / windows-x64 unrelated to the
Transpiler sourcemap diff:

- fetch-http2-client: WebKit AtomString wasRemoved assertion
  (h2 client under ASAN has recent deflaking history — #29954, #29809)
- test-worker-nested-uncaught: panic
  "EventLoop.enqueueTaskConcurrent: VM has terminated" — worker VM
  teardown race
- test-http-should-emit-close-when-connection-is-aborted: same failure
  landed on PR #30540's neighbour build 53635
- AsyncLocalStorage-tracking / bun-install-registry: async_hooks /
  install flakes

None of these touch Bun.Transpiler / js_printer / sourcemap. All
Linux/Darwin/FreeBSD build-zig and build-cpp lanes passed.
@robobun robobun force-pushed the farm/2164512e/transpiler-sourcemap-output branch from d6164a1 to 48639e5 Compare May 27, 2026 15:20
robobun and others added 12 commits June 5, 2026 20:40
Bun.Transpiler already parses the sourcemap option but never threaded
it through to the printer, so transformSync returned a plain string
with no way for downstream tools to recover original line numbers
after TS stripping.

Behaviour now matches Bun.build's sourcemap option:
  - unset / false / "none"  → returns string (unchanged)
  - "inline" / true          → string + inline //# sourceMappingURL=
                                  data:application/json;base64,<map>
  - "external"               → { code, map } (map is v3 JSON text)
  - "linked"                 → { code, map } and code carries a
                                  //# sourceMappingURL=<name>.map comment

The map correctly remaps generated lines back to their original input
lines even after the transpiler drops interface/type declarations and
collapses adjacent blank lines.

Closes #30538
…string

CI's bun-plugin-svelte type check failed with:
  src/index.ts(104,13): error TS2322: Type 'string |
  TranspilerTransformResult' is not assignable to type 'string'.

Widening transform/transformSync to 'string | TranspilerTransformResult'
broke existing callers that assign the result to a string variable.

Fix by making Transpiler generic over the sourcemap mode (inferred as
a literal thanks to 'const SM'). When sourcemap is unset / false /
'none' / 'inline' / true, the return stays string — bun-plugin-svelte
keeps compiling. When sourcemap is 'external' / 'linked', the return
narrows to TranspilerTransformResult.

Regression coverage in
test/integration/bun-types/fixture/transpiler.ts.
When TranspilerOptions was declared with `SM = "none"` as the
generic default, writing `const opts: Bun.TranspilerOptions =
{ sourcemap: "external" }` failed type-checking because the default
narrowed `sourcemap` to just "none". Inline-inferred constructor
calls still worked (SM gets inferred from the argument), but
intermediate variables didn't.

Widen the default to the full TranspilerSourceMapOption union. Callers
who want the narrow return type can still opt in with
`TranspilerOptions<"external">` / `new Bun.Transpiler<"external">(…)`.

Added fixture cases for both the regression (explicit TranspilerOptions)
and the narrowing-via-type-argument path, plus the async transform()
assertion for the "linked" mode.
…line-terminate inline footer

Three fixes from coderabbit review on #30540:

1. Empty / type-only inputs diverged between sync and async. The async
   path early-returned with output_code = empty when parse_result.empty
   or printed == 0, never populating output_map, and finish() branched
   on output_map.isEmpty() — so `await t.transform("interface Foo {}")`
   with sourcemap: "external" resolved to `""` instead of
   `{ code: "", map }`, while transformSync correctly returned the
   object shape.

   Fix: branch finish() on source_map (not output_map.isEmpty()), drop
   the printed == 0 early-return, and when parse_result.empty + a map
   was requested, synthesize an empty v3 map via SourceMapCapture.writeEmpty
   so the return shape is preserved.

2. The inline data URL footer lacked a trailing newline, while the
   .linked branch next to it and Bun.build's inline emitter (generate
   ChunksInParallel.zig) both append one. Without the newline, naive
   concatenation of two transformer outputs silently swallows the
   next file into the `//#` line comment.

   Fix: always append `\n` after the base64 payload.

3. Incidental: dropped a stale `buffer_writer.buffer.list.items =
   buffer_writer.written` assignment that trashed the buffer's backing
   allocation when the printer was skipped (crash surfaced under ASAN
   once the early-returns were removed). The assignment was a no-op
   in the printer-ran path and dangerous otherwise.

Regression tests cover empty and type-only inputs across external/
linked/inline, both sync and async; plus an `endsWith('\n')` assertion
for inline output.
getParseResult() was deriving the virtual source name from
this.config.default_loader.stdinName() regardless of the per-call
loader argument, while TransformTask.run() used the override. Pre-PR
this was invisible — source.path.text was internal. With this PR
piping the name into map.sources and the //# sourceMappingURL=<name>.map
footer, the divergence became user-observable:

  const t = new Bun.Transpiler({ sourcemap: "external" });
  // default_loader defaults to "jsx"
  t.transformSync(code, "ts");       // map.sources: ["/input.jsx"]
  await t.transform(code, "ts");     // map.sources: ["/input.ts"]

One-line fix: use (loader orelse config.default_loader).stdinName() so
the sync path matches the async path. Regression test covers both the
external map sources field and the linked footer.
…orm API

When `target: "bun"` is set, `printWithSourceMap(.esm_ascii)` routes
through `getSourceMapBuilder(.lazy, is_bun_platform=true, …)` which
packs mappings as the Bun runtime's InternalSourceMap binary format
instead of standard VLQ. `SourceMapCapture.onChunk` was
unconditionally calling `chunk.printSourceMapContents`, so the raw
binary went into the v3 map's `"mappings"` field and JSON.parse
rejected the result.

```js
new Bun.Transpiler({ loader: "ts", target: "bun", sourcemap: "external" })
  .transformSync("const x = 1;");
// JSON.parse(map) → SyntaxError: Invalid escape character v
```

Fix: thread `transpiler.options.target.isBun()` into SourceMapCapture
and call `printSourceMapContentsFromInternal` for that case, which
re-encodes the internal blob to standard VLQ before emitting JSON.
The helper already existed; the transform path just wasn't using it.

Regression coverage: the target:"bun" matrix exercises all four JS
loaders for both `external` and `inline` modes, and the test's own
`decodeMappings` VLQ decoder now throws fast on invalid digits so a
future regression surfaces as a test failure instead of a CI hang.

Also converts the two parameterized for...of loops in the test block
to `describe.each` / `it.each` per repo convention, and extends the
empty-input matrix to cover sync paths for "linked" / "inline" in
addition to the existing "external" sync case.
Flaky test failures on x64-asan / windows-x64 unrelated to the
Transpiler sourcemap diff:

- fetch-http2-client: WebKit AtomString wasRemoved assertion
  (h2 client under ASAN has recent deflaking history — #29954, #29809)
- test-worker-nested-uncaught: panic
  "EventLoop.enqueueTaskConcurrent: VM has terminated" — worker VM
  teardown race
- test-http-should-emit-close-when-connection-is-aborted: same failure
  landed on PR #30540's neighbour build 53635
- AsyncLocalStorage-tracking / bun-install-registry: async_hooks /
  install flakes

None of these touch Bun.Transpiler / js_printer / sourcemap. All
Linux/Darwin/FreeBSD build-zig and build-cpp lanes passed.
main rewrote src/runtime/api/JSTranspiler.zig in Rust
(src/runtime/api/JSTranspiler.rs, registered via #[bun_jsc::JsClass]),
and the rebase brought in the Rust port which didn't yet include the
sourcemap work from the earlier Zig commits on this branch.

Port every piece of the Zig implementation to the live Rust file:

- TransformTask: add `output_map: BunString` + `source_map:
  api::SourceMapMode` fields, initialized from the owning
  Config.transform.source_map in `create`.

- run(): drop the `parse_result.empty` early-return when a map was
  requested; synthesize an empty v3 map via
  `SourceMapCapture::write_empty` so `.external`/`.linked` keep the
  `{ code, map }` contract for type-only / empty inputs. Select
  `print_with_source_map` vs `print` on the `want_source_map` flag.
  Branch on `source_map` to build the final code/map pair.

- finish(): branch on `source_map` (not `output_map.is_empty()`) so
  `.external`/`.linked` always resolve with the `{ code, map }`
  object even when the code is empty.

- transform_sync(): factor out `build_transform_result` for the
  post-print dispatch; thread a `SourceMapCapture` through
  `print_with_source_map` when enabled.

- SourceMapCapture: impls `OnSourceMapChunk`, carrying an
  `is_internal_format` flag initialized from `target.is_bun()`.
  Branches `on_source_map_chunk` between `print_source_map_contents`
  (standard VLQ) and `print_source_map_contents_from_internal` (packed
  binary blob re-encoded to VLQ) so `target: "bun"` produces a
  v3-consumer-parseable `mappings` field instead of raw binary.

- `get_parse_result`: use `loader.unwrap_or(config.default_loader)`
  for `stdin_name()` so the virtual source name matches
  `TransformTask::run`'s use of the per-call override; sync and async
  now agree on `map.sources` and the linked-mode footer when the
  caller passes a per-call loader.

- Helpers: `append_inline_source_map` (trailing-newline-terminated
  `//# sourceMappingURL=data:application/json;base64,...`),
  `source_map_url_for` (basename + `.map`), `create_code_map_object`
  (`{ code, map }` via `JSValue::create_object2`).

All 30 sourcemap tests pass under bun bd; full transpiler.test.js
suite green (156 pass / 0 fail).
clippy's 'unsafe_block_missing_a_safety_comment' rule wants the comment
immediately before each unsafe block, not above a wrapping `if` that
contains two of them.
@robobun robobun force-pushed the farm/2164512e/transpiler-sourcemap-output branch from 48639e5 to 427d5bf Compare June 5, 2026 20:48

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found in this pass and all earlier feedback is resolved, but this is a ~700-line feature touching the Zig and Rust transpiler print paths plus a public API/type-contract change, so it's worth a human sign-off on the API shape and the memory handling in the new Rust port.

Extended reasoning...

Overview

This PR wires the previously-parsed-but-ignored sourcemap constructor option through Bun.Transpiler.transformSync / transform, emitting a v3 source map in four shapes (none/inline/external/linked) that mirror Bun.build. It touches:

  • src/runtime/api/JSTranspiler.zig (~290 net new lines): SourceMapCapture helper, buildTransformResult, appendInlineSourceMap, createCodeMapObject, sourceMapURLFor; threads sourcemap mode through both the sync path and TransformTask.run()/finish().
  • src/runtime/api/JSTranspiler.rs (~300 net new lines): a parallel Rust port of the same logic, including new unsafe blocks around transpiler_mut() for the print_with_source_map call.
  • packages/bun-types/bun.d.ts: makes Transpiler generic over a const SM extends TranspilerSourceMapOption so transform/transformSync return types narrow to string vs { code, map }.
  • ~330 lines of new runtime tests and a tsd type fixture.

Security risks

None apparent. Inputs are user-supplied source strings that already flow through the parser/printer; the new code only post-processes the printer's output (base64-encodes the map, builds a 2-key JS object). No filesystem, network, auth, or privilege boundaries are crossed.

Level of scrutiny

Medium-high. This is runtime code in both Zig and Rust with manual buffer sizing (writableNBytes(prefix.len + encode_len + 1), @memcpy into a stack PathBuffer), arena/allocator interplay, and a pooled buffer_writer whose contents are mutated after written is captured. It also changes the public return shape of transformSync/transform and the published bun-types contract via conditional generics — an API design decision worth a maintainer's eye. The Rust port (b0463d15) is recent and adds parallel unsafe paths that should be sanity-checked by someone familiar with the JsCell/transpiler_mut aliasing rules.

Other factors

The PR has been through several review rounds; earlier real bugs (async/sync shape divergence on empty output, target: "bun" emitting the packed internal map instead of VLQ, the orphaned JSDoc, the TranspilerOptions generic default) were all fixed and have regression tests. The current bug-hunting pass found nothing. Test coverage is solid (shape matrix × sync/async × empty/type-only × target: "bun", plus VLQ round-trip). Given the scope and the public API surface change, deferring rather than auto-approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bun.Transpiler should optionally emit a sourcemap from transformSync / transform

1 participant