bundler: chain external input sourcemaps and thread chains through the dev server - #32473
Open
robobun wants to merge 36 commits into
Open
bundler: chain external input sourcemaps and thread chains through the dev server#32473robobun wants to merge 36 commits into
robobun wants to merge 36 commits into
Conversation
Closes #30536. When Bun.build reads a source file that carries a trailing `//# sourceMappingURL=data:application/json;...` comment (typically produced by an upstream compile step like .vue/.svelte/.mdx/ts-plugin → .js), the bundler already detected the URL at the lexer but dropped it on the floor. The output sourcemap's deepest `sources[]` entry was the intermediate .js file and `sourcesContent[]` carried the intermediate's bytes verbatim — including the literal `sourceMappingURL=` comment — so stack traces surfaced one hop short of the authored source. - `src/sourcemap/InputSourceMap.zig` (new): owns the parsed inner map + per-source contents, cleans up after itself. - `src/bundler/ParseTask.zig`: after parsing the JS, scan the source for a trailing `//# sourceMappingURL=data:...`. Inline data URLs are parsed (base64 and raw); external `.map` references are left for a follow-up. Gated on `source_map != .none` and `loader.canHaveSourceMap()`. - `src/bundler/Graph.zig`, `src/bundler/bundle_v2.zig`: new `InputFile.input_source_map` field; ownership moves from the parse result onto the file on consumption, freed at bundler teardown. - `src/js_printer/js_printer.zig`, `src/sourcemap/Chunk.zig`: threads the parsed map into `Chunk.Builder`. During `addSourceMapping`, translate each (line, column) through the inner map — on a hit, emit `source_index = 1 + inner.source_index` and the inner original (line, column); on a miss, fall back to slot 0 (the intermediate) so unmapped tokens still land in a real file. - `src/bundler/LinkerContext.zig`: each outer source in the output map's `sources[]` expands to `[intermediate, inner_0 .. inner_N-1]`. `sourcesContent[]` matches slot-for-slot: the intermediate's contents first, then each inner source's contents (drawn from the inner map). Chunk stitching uses `base + chunk.end_state.source_index` as the absolute end state so per-chunk mappings that vary source_index across their length compose correctly. - Repro from the issue (entry.ts → inner.js-with-inline-map → inner.ts): output `sources` now contains `../inner.ts`; `sourcesContent[authored_slot]` is the clean authored bytes with no sourceMappingURL comment. - Multi-inner-source maps (e.g. .vue compilers that split template/script) surface all inner sources. - Malformed or unrecognized inline maps fall back gracefully — build succeeds with the old behavior. - External `.map` references unchanged (out of scope here). - `sourcemap = none` builds skip the scan entirely. Five new cases in `test/bundler/bun-build-api.test.ts` covering: the base64 chain, the raw data-URL chain, multi-inner-source surfacing, malformed payload fallback, and external `.map` reference unchanged.
The ParseTask scan runs on `source.contents` regardless of whether the contents came from disk or an `onLoad` plugin return, so the plugin case from #6173 is covered by the same pipeline. Add an explicit regression test so it stays covered if the scanner ever moves. A custom-extension plugin that emits transformed JS with its own `//# sourceMappingURL=data:...` comment should have the pre-transform authored source show up in the final map's `sources[]` / `sourcesContent[]`. Uses a distinct inner-source filename so the slot assertion can tell the plugin intermediate apart from the chained authored source.
claude[bot] and coderabbit spotted three real issues in the initial
landing. One refactor fixes the first two; the third is just more
assertions.
1. InputSourceMap.parse: return `?*InputSourceMap` made the `errdefer`
blocks dead code — Zig only fires `errdefer` on error returns,
and the function had none. Every mid-parse `return null` (most
realistically `Mapping.parse` rejecting a malformed VLQ, which
passes all the JSON structure checks above it) leaked
`source_paths_slice`, `sources_content_slice`, and every
`allocator.dupe`d string on `bun.default_allocator` — never
reclaimed for the life of the process, nasty in long-running
dev-server / watch-mode callers.
Split into a public `parse` + internal `parseInternal` that
returns `ParseError!*InputSourceMap`. The `errdefer`s now fire
on every malformed-payload bail. OOM propagates through the
error union and the outer `parse` wraps it in
`bun.outOfMemory()` (fatal), while validation failures collapse
back to `null` — the original contract callers depend on.
Ownership-transfer sites (`psm.external_source_names =`,
building the result `InputSourceMap`) neuter the now-redundant
`errdefer`s by zeroing `paths_written` / `contents_written`
and re-pointing the slices at `&.{}`, so a future `try` added
between transfer and return can't double-free.
2. bundle_v2 onParseTaskComplete: the new `input_source_map` slot
was written without freeing any prior value, leaking on dev-server
/ watch-mode reparses where the same source index gets a fresh
map. Also, the early-failure path where `runResolutionForParseTask`
downgrades `.success` to `.err` was dropping the freshly parsed
map without deinit.
Both paths now explicitly `deinit()` before overwrite. The
success transfer also nulls the result slot so the old layout
can't be mistaken for still-owning after the move.
3. Test guards: four call sites pulled `m![1]` without first
asserting `m` matched. Added `expect(m).not.toBeNull()` so a
future output regression fails the test cleanly instead of
surfacing as an unhelpful TypeError on the `m![1]` coercion.
Rejected coderabbit's "restrict to file-backed inputs" suggestion:
plugin `onLoad` returns are explicitly in scope (closes #6173) and
covered by the test committed in 192b355. A blanket
`source.path.isFile()` gate would break that case.
Rejected claude[bot]'s suggestion to add a hostile-VLQ regression
test: it exposed a pre-existing panic inside `Mapping.parse`
(`addScalar` → `fromZeroBased` assert on negative column delta),
which is out of scope for this PR. The structural fix above removes
the leak on every failure path anyway; the test was documentation-
only and didn't need to sit atop a landmine.
coderabbit flag on the inline map scanner: the old
`lastIndexOf(source, "\n//# sourceMappingURL=")` would match the
needle anywhere in the file, including inside a string / template
literal that happens to contain the marker text. Per the Source Map
spec the comment MUST sit on the last line, so rewrite
`findSourceMappingURL` to trim trailing whitespace first, then only
prefix-match the final line.
Added two test-coverage improvements in the same commit:
- New test "sourceMappingURL marker in body is ignored": crafts an
intermediate that embeds a FULLY VALID inline map inside a template
literal, then follows it with a plain `export` on the last line.
Old scanner would chain through the embedded `hijack.ts` source;
new scanner ignores it.
- Strengthened "malformed inline map" assertion: decodes the output
map and verifies `sources` lists the intermediate (not some
fabricated path from the malformed payload) instead of just checking
that a sourceMappingURL comment was emitted.
Skipping coderabbit's third suggestion ("require `version` field")
because Bun's existing `sourcemap.parseJSON` treats `version` as
optional too (sourcemap.zig:82-86). Tightening it is worth doing but
needs to happen in both places together; out of scope for this PR.
Three more real bugs in the chaining landing that claude[bot] walked through in detail. Fixing all three: 1. `InputSourceMap.zig`: passing `std.math.maxInt(i32)` as `sources_count` to `Mapping.parse` disabled its `source_index >= sources_count` bounds check. A malformed inline map whose VLQ segment referenced an index past the end of its own `sources[]` would parse successfully. Downstream, the Builder emits `1 + inner.source_index` unclamped and LinkerContext reserves exactly `1 + external_source_names.len` slots per file, so the out-of-range index silently aliases the NEXT input file's slot range in the output — stack traces from file A get misattributed to file B. Pass the real `source_count` so such maps hit the `.fail` path and we fall back cleanly. Regression test: `inline map with out-of-range inner source_index is rejected` constructs a map with VLQ "AAAA;ACAA" (second mapping source_index = 1) against `sources: ["authored.ts"]` (len 1) and verifies `authored.ts` does NOT appear in the output `sources[]` after the fix. 2. `ParseTask.zig`: the dev-server HMR path in `runFromThreadPool` flips a parse-succeeded-with-errors `Success` into an `.err` by value-assigning a fresh `.err` payload and dropping the original `ast`. The prior commit already handled the resolve-error downgrade in `bundle_v2.runResolutionForParseTask`, but this second drop site leaks `ast.input_source_map` on every HMR rebuild of a file that both carries an inline map and logs parse errors. Deinit before the overwrite. 3. `LinkerContext.zig`: the `input_source_map` was plumbed into the print options unconditionally, but Bake's DevServer has its own sourcemap stitcher (`SourceMapStore.joinVLQ` + `PackedMap`) that hard-codes one `sources[]` slot per file and discards `chunk.end_state.source_index`. With `input_source_map` set, chunks now emit non-zero per-chunk `source_index` deltas that the DevServer stitcher would re-rebase across neighboring files' slots, corrupting served browser stack traces for any prebuilt `.js` carrying an inline `data:` sourcemap. Gate the field on `c.dev_server == null` until the DevServer stitcher is taught the slot-expansion layout (follow-up, separate PR). `test/bake/dev/sourcemap.test.ts` still passes; the existing Bun.build (non-dev) suite still passes too. Gate-check: `bun bd test bun-build-api` → 45/45 + 1 todo, `bake sourcemap.test.ts` → 2/2.
debian-13-x64-asan-test-bun failed with exit 2 on 706b461 (single shard out of 20 parallel). All other ASAN build lanes passed. Local bun bd (ASAN on by default) is clean on the full bundler suite and integration tests. Can't scrape which test failed from Buildkite anonymously; rolling the dice one time.
Gate hit this as `release with fix: BUILD FAILED` on my PR, but it's
a pre-existing race that any release build from a clean state can
trigger. Reproducing it is just:
rm -f build/release/bun-zig.*.o
build/release/bun scripts/build.ts --profile=release
[1/4] link bun-profile
[2/4] bun-profile --revision
/bin/sh: 1: /workspace/bun/build/release/bun: Permission denied
FAILED: bun-profile.smoke-test-passed
[4/4] strip bun
The smoke-test rule wraps its command in `${cfg.jsRuntime} stream.ts
check --console ...`. In configure.ts, jsRuntime is hard-wired to
`process.execPath` of whatever bun drove the build — and because
`build/release/` is ahead of the system bin dir on the standard dev
PATH, that's `build/release/bun`: the same file `strip bun` is
about to write. Ninja schedules the smoke test and strip
concurrently (no declared dep between them), the strip open(O_WRONLY)
races the jsRuntime execve, and execve fails with EACCES.
The race was masked by a warm cache: when the zig .o files are
already present, strip and smoke-test both finish so fast the window
closes. A clean zig recompile (PR commits touching anything the zig
build reads, or any CI box starting cold) opens the window wide
enough to hit reliably.
Fix: make the stripped binary an order-only input of the smoke test.
Ninja then serializes `strip bun` before `bun-profile --revision`
so jsRuntime execve lands on a quiescent file. Confirmed:
[1/5] link bun-profile
[3/5] strip bun
[3/5] bun-profile --revision
[build] done
Order-only (ninja's `||` edge) is the right tool here — the smoke
test doesn't consume `bun` as input, it just needs it to exist
and not be mid-write. Not an implicit dep, which would force a
rebuild of the stamp whenever strip runs.
Threaded through both the full-build (`emitBun`) and link-only
(`emitLinkOnly`) emit paths. Non-strip configs (debug, asan) pass
`undefined` and keep their current behavior — no stripped binary,
no race.
Fixes `test/internal/ban-words.test.ts` failing uniformly across every CI platform (debian 13, 25.04, 3.23 x64+aarch64+baseline+asan; macos 14+26 aarch64+x64; windows 2019+11 x64+aarch64+baseline). The test greps `src/` for each phrase in `test/internal/ban-limits.json` and asserts the count is <= the configured limit; `std.mem.indexOfAny(u8` is set to 0 (reason: "Use bun.strings.indexOfAny") and this PR added exactly one call, pushing the count from 0 to 1. `bun.strings.indexAnyComptime(target, comptime chars) ?usize` is a drop-in with the same `orelse` shape and comptime needle — no behavior change. Other callers in `src/` (resolve_path.zig, package_json.zig, bunx_command.zig, patch.zig) use the same pattern.
Root CLAUDE.md bans new behavior in .zig files — they're kept as a porting reference only. The 8 tests in this block exercise the chained input-sourcemap feature implemented in the .zig tree of this PR, which isn't wired into the live Rust bundler. Wrapping them in describe.todo keeps them as the intended-behavior spec for the eventual Rust port (per the file-by-file plan in the PR description) without red-lining every CI run. Flip back to describe() once InputSourceMap.rs + the Graph.rs / ParseTask.rs / LinkerContext.rs / Chunk.rs changes land.
Port of the Zig design in an earlier commit on this branch to Rust, since the bundler is now implemented in Rust (#30412). Same behavior, same 8 tests, different language. Files touched (Rust): - src/sourcemap/InputSourceMap.rs (new): owns *ParsedSourceMap + per-source contents; `parse` (validates v3 JSON, returns None on malformed), `parse_from_source` (last-line-anchored `//# sourceMappingURL=data:...` scan + base64/raw payload decode). - src/bundler/Graph.rs: `InputFile.input_source_map` column on the MultiArrayList + SoA accessor via `multi_array_columns!`. - src/bundler/ParseTask.rs: after getAST, scan `source.contents` for an inline map; gated on `source_map != None` + `can_have_source_map` + non-empty contents. Field added to `Success`. - src/bundler/ServerComponentParseTask.rs: wrapper Success constructor gets `input_source_map: None` (generated, not authored). - src/bundler/bundle_v2.rs: move from Success into the Graph SoA slot on parse completion; drain slots in `deinit_without_freeing_arena`. - src/bundler/LinkerContext.rs: `generate_source_map_for_chunk` expands outer `sources[]` to [intermediate, inner_0, …, inner_N-1] and mirrors `sourcesContent[]`; stitch the absolute source_index using `base + chunk.end_state.source_index`. Option threaded through to the printer, gated on `dev_server.is_none()` (Bake's SourceMapStore is a separate stitcher with a different layout). - src/sourcemap/Chunk.rs: `Builder.input_source_map`; in `add_source_mapping`, look up the intermediate (line, col) via `find_mapping`. On hit emit `mapped_source_index = 1 + inner.source_index` + inner's (line, col); on miss fall back to slot 0. - src/js_printer/lib.rs: `Options.input_source_map` forwarded into the builder in `get_source_map_builder`. Test suite in test/bundler/bun-build-api.test.ts flipped back from `describe.todo` to `describe` — all 8 pass: - base64 data URL: authored source surfaces - raw data URL: authored source surfaces - multi-inner-source map (e.g. .vue split): all surface - malformed payload: build succeeds, falls back - out-of-range inner source_index: rejected, no slot aliasing - in-body sourceMappingURL marker: ignored, no hijack - external .map reference: unchanged behavior - plugin onLoad inline map: regression guard for #6173 Closes #30536, also fixes #6173.
…ngURL= Matches Zig's `bun.strings.trim(_, " \r\t")` at InputSourceMap.zig:219. A leading space after `=` (e.g. `//# sourceMappingURL= data:...`) is spec-invalid but some toolchains emit it, and `parse_data_url`'s prefix check would fail on the leading space without this. Flagged by claude[bot] as a port-fidelity divergence.
Main's #30875 replaced the auto-derived Default impl for InputFile with an explicit one, which I missed in the last rebase — CI caught the missing-field on every build-rust lane. Add the None init. Fixes the build-rust failure in build #56341.
Only ParsedSourceMap is referenced; the mapping::parse call is fully-qualified through the lowercase module path. Flagged by claude[bot].
…ows UB topts is a shared borrow through a raw pointer into *transpiler. get_ast reborrows the same location mutably (also through the raw pointer), which pops topts's SharedReadOnly tag under Stacked Borrows — any subsequent topts.source_map read becomes Miri-detectable UB. Copy source_map out alongside module_type, before the `let _ = topts;` tombstone the prior author left for exactly this invariant. Flagged by claude[bot].
Main's e_string().string() takes &self now; the `mut` bindings at InputSourceMap.rs:116/126 became unused, which -D unused-mut turns into a hard error on the release build-rust lanes. mappings_e_string keeps its mut (slice() still takes &mut self).
When input_source_map chaining is active, prev_state.original_line holds the remapped *authored* line — wrong coordinate space for the intermediate file's line-offset table, so the O(1) find_line_with_hint fast path missed on every token and fell to binary search. Track the un-remapped intermediate line in a dedicated prev_intermediate_line field and hint from that. No behavior change (binary search was always the sound fallback); restores the per-token fast path on the chained feature path. Chunk.zig uses plain findLine (no hint), so it's unaffected. Flagged by claude[bot].
…xternal-input-sourcemaps
…dev server Extends the inline input-sourcemap chaining from #30539 to also: 1. Load external '//# sourceMappingURL=foo.map' references from disk (resolved relative to the input file). data: URLs were already handled; now linked sidecar .map files are too. http(s):// and protocol-relative URLs are skipped. Missing or malformed sidecars fall back silently to mapping against the intermediate. 2. Thread chained input sourcemaps through the Bun.serve dev server. SourceMapStore previously hard-coded one sources[] slot per input file; PackedMap now carries the inner-source list and render_json/join_vlq emit and stitch them in slot order. The printer's input_source_map path is no longer gated on dev_server == None. ErrorReportRequest uses the new Entry::lookup_source to resolve a flat source_index back to the right path/contents. Fixes #26713: Bun.serve HTML routes that reference a pre-built .js with a --sourcemap=linked sidecar now surface the authored source in browser DevTools instead of the intermediate .js.
Collaborator
Author
|
Updated 1:59 AM PT - Jun 27th, 2026
❌ @autofix-ci[bot], your commit c67b78d has 2 failures in
🧪 To try this PR locally: bunx bun-pr 32473That installs a local version of the PR into your bun-32473 --bun |
This was referenced Jun 17, 2026
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end sourcemap chaining for input files that embed a trailing ChangesSourcemap chaining through inline sourceMappingURL
Build script: smoke-test strip ordering
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
Contributor
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
Collaborator
Author
|
Looked at both:
Leaving both out of the closes list. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 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/bundler/bundle_v2.zig`:
- Around line 2230-2236: Remove all new sourcemap lifecycle behavior from the
Zig reference file bundle_v2.zig. Specifically, delete the deinit loop for
input_source_map that iterates through and deinitializes sourcemaps (the for
loop iterating through this.graph.input_files.items(.input_source_map) and
calling ism.deinit()), and remove all related sourcemap lifecycle code at the
other affected locations (2879-2882 and 3680-3692). Since .zig files are porting
references only and not shipped, ensure this sourcemap management behavior
exists exclusively in the Rust implementation (.rs files) instead and verify the
functional fix is complete in the Rust code before submitting.
In `@src/bundler/LinkerContext.rs`:
- Around line 1095-1104: Remove the stale dev_server.is_none() conditional gate
around the input_source_maps assignment in the code block from lines 1095-1104.
Instead of conditionally setting input_source_maps to Some or None based on
whether dev_server exists, always set it to
Some(self.parse_graph().input_files.items_input_source_map()). Apply this same
fix to the other similar conditional gates mentioned at lines 1135-1143,
1177-1178, and 2292-2297 to ensure input source maps are consistently provided
to all code paths, eliminating the source-index collision issue.
In `@src/js_printer/lib.rs`:
- Around line 1316-1324: Update the comment for the `input_source_map` field to
reflect the new source-slot model. Remove or generalize the specific reference
to inline `data:...` URLs since the field now represents chained input maps more
broadly. Update the DevServer HMR path note to clarify that the stitcher now
handles multi-source maps instead of the old one-slot approach. Keep the comment
focused on durable non-obvious content that explains the field's purpose in the
new architecture.
- Around line 7795-7810: The unsafe transmute at line 7795 erases the lifetime
of input_source_map to 'static, which is unsafe when get_source_map_builder is
public. Either add a lifetime parameter to the Builder struct and Options struct
to properly thread the InputSourceMap lifetime through them instead of erasing
it with transmute, or change get_source_map_builder to be non-public so it can
only be called from graph-owned code paths where the lifetime is guaranteed to
be valid. Remove the unsafe transmute block and adjust the field and function
signatures accordingly to properly track the borrowed lifetime.
In `@src/runtime/bake/dev_server/source_map_store.rs`:
- Line 790: The entry.source_slot_count() call excludes slot 0 (the HMR runtime
slot), but the source map rendering functions emit the HMR runtime at sources[0]
and the VLQ mappings start file slots at 1. This mismatch causes the parser to
receive a source count that is one less than what the VLQ mappings can actually
reference, leading to valid mappings being rejected. Add 1 to the result of
entry.source_slot_count() in the i32::try_from call to include the HMR runtime
slot in the count passed to the parser.
In `@src/runtime/bake/DevServer.rs`:
- Line 4133: The `inner_sources` field now affects the output of
`SourceMapStore::render_json`, but the hot-update `script_id` cache key
calculation (around line 4697) only uses the file path and `map.vlq()` value.
This means changes to the input source map's `sources` or `sourcesContent` won't
invalidate the cache, potentially reusing stale entries. Include `inner_sources`
in the cache key calculation for the hot-update script hash to ensure every
input that shapes the output is covered by the cache key, similar to how it's
collected in the `collect_inner_sources` call. Apply the same fix to the
location noted at line 4152 as well.
- Around line 3891-3895: In the sourcesContent allocation and quoting logic,
remove the silent fallback to an empty buffer and stop ignoring the result of
quote_for_json. Instead of using unwrap_or_else with init_empty() for
MutableString::init, use proper OOM handling like unwrap_or_oom() to propagate
allocation failures. Additionally, remove the let _ = discard pattern on the
quote_for_json call and properly handle any result returned from that function
to ensure failures are not silently swallowed and empty escaped_content is not
created on failure.
- Around line 3862-3882: The collect_inner_sources function has two issues:
first, allocation failures in the MutableString initialization and
quote_for_json call are silently swallowed with underscore discard operators,
potentially leaving corrupted JSON in escaped_content—replace the underscore
discard with proper error handling using unwrap_or_oom or error propagation to
crash cleanly instead of silently corrupting data; second, the source_map_hash
calculation only includes the map's vlq() bytes but does not include the new
inner_sources data, causing the cache key to remain unchanged when inner_sources
paths or content are rebased, leading to stale caching—add inner_sources to the
hash calculation by hashing each element's path and escaped_content before the
vlq() update.
In `@src/sourcemap/Chunk.zig`:
- Around line 214-220: Remove the new input_source_map field declaration from
the Chunk.zig file (the field with the comment about remapping through its inner
map) and also remove the associated remapping implementation block that spans
lines 367-416. Since .zig files are reference-only implementations that are not
compiled or shipped, the sourcemap chaining behavior should exist only in the
Rust (.rs) implementation. The Zig file should be updated in a separate,
documented sync if maintainers decide the reference needs updating.
In `@src/sourcemap/InputSourceMap.rs`:
- Around line 148-150: The code uses `.expect("OOM")` for out-of-memory error
handling but should use `.unwrap_or_oom()` instead to follow Bun's controlled
OOM handling path. Replace the `.expect("OOM")` call on the
`estr.string(&arena)` result with `.unwrap_or_oom()` to ensure consistent OOM
handling across the codebase. Apply the same fix to the similar call at line
158.
In `@test/bundler/bun-build-api.test.ts`:
- Around line 1430-1434: The sourcemap parsing logic is duplicated across eight
test cases (at lines 1430-1434, 1476-1479, 1512-1515, 1542-1545, 1583-1586,
1629-1632, 1657-1660, and 1711-1714). Create a helper function that accepts the
output path, reads the file, extracts the base64-encoded sourcemap using the
regex pattern, decodes it, and returns the parsed JSON object. Replace all eight
occurrences of this repeated block with calls to the new helper function to
avoid future maintenance issues with sourcemap format changes.
- Around line 1435-1447: The current assertions in this test block verify that
the authored source appears in the sources array and that sourcesContent has
correct content, but they do not verify that the actual VLQ mappings in the
source map point to the authored.ts source. Add an assertion that decodes the
VLQ mappings from the parsed source map and verifies that at least one mapping
segment has its source index pointing to authoredIdx (the index of authored.ts)
rather than pointing to intermediate.js or other sources. This ensures the
regression test actually validates that the mappings target the correct authored
source, not just that the metadata exists.
🪄 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: 6ed619d8-a916-4243-b1bf-e9ac6a1684b1
📒 Files selected for processing (25)
scripts/build/bun.tssrc/bundler/Graph.rssrc/bundler/Graph.zigsrc/bundler/LinkerContext.rssrc/bundler/LinkerContext.zigsrc/bundler/ParseTask.rssrc/bundler/ParseTask.zigsrc/bundler/ServerComponentParseTask.rssrc/bundler/bundle_v2.rssrc/bundler/bundle_v2.zigsrc/js_printer/js_printer.zigsrc/js_printer/lib.rssrc/runtime/bake/DevServer.rssrc/runtime/bake/DevServer/ErrorReportRequest.rssrc/runtime/bake/dev_server/incremental_graph.rssrc/runtime/bake/dev_server/packed_map.rssrc/runtime/bake/dev_server/source_map_store.rssrc/sourcemap/Chunk.rssrc/sourcemap/Chunk.zigsrc/sourcemap/InputSourceMap.rssrc/sourcemap/InputSourceMap.zigsrc/sourcemap/lib.rssrc/sourcemap/sourcemap.zigtest/bundler/bun-build-api.test.tstest/regression/issue/26713.test.ts
…key, parse slot count
- LinkerContext.rs: drop the vestigial dev_server.is_none() gate in
generate_source_map_for_chunk now that SourceMapStore handles
multi-slot files; that function never runs on the dev-server path
anyway.
- js_printer Options::input_source_map doc: drop the outdated
dev-server note and the data:-only wording.
- LinkerContext.zig: update the reference comment to reflect that the
Rust side passes input_source_map unconditionally.
- DevServer collect_inner_sources: route MutableString::init /
quote_for_json through bun_core::handle_oom instead of silently
falling back to an empty buffer on allocation failure.
- DevServer HMR script-id hash: fold inner_sources paths + contents
into the source_map_hash so a sidecar .map change that keeps the
same VLQ still invalidates the cached entry.
- source_map_store::get_parsed_source_map: pass source_slot_count()+1
to mapping::parse so the HMR-runtime slot at index 0 is accounted
for (pre-existing off-by-one, surfaced while touching this code).
- InputSourceMap: route estr.string() through bun_core::handle_oom
instead of .expect("OOM").
…_sources_for - InputSourceMap::parse_internal: read the optional sourceRoot field and prepend it to each sources[] entry (inserting a '/' when neither side has a separator, matching esbuild). Covered by a new test in 26713.test.ts. - LinkerContext::write_sources_for: when the intermediate's path is in a non-file namespace (plugin virtual module), emit inner source names as-is instead of joining against a meaningless dirname. Mirrors the guard already in DevServer::collect_inner_sources.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bundler/LinkerContext.rs (1)
1308-1327:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not filesystem-resolve inner sources for non-file namespace inputs.
outer_path.is_file()is respected for the outer source, but inner source names are always resolved throughdirname/join_abs/relative_alloc. For plugin/virtual inputs carrying an inline map, this rewrites logical source names as filesystem-relative paths, producing incorrectsources[]entries.Proposed fix
if let Some(ism) = input_map { let base_dir = bun_paths::resolve_path::dirname::<bun_paths::resolve_path::platform::Auto>( outer_path.text, ); for name in ism.map.external_source_names.iter() { let name: &[u8] = name.as_ref(); - // Use `join_abs` to produce an absolute inner path (when the - // inner map emitted a relative source name) that can then be - // re-relativized against `chunk_abs_dir`. `join_abs` returns - // a borrow into a thread-local buffer; we copy out immediately - // via `relative_alloc`. - let abs_path: &[u8] = if bun_paths::resolve_path::Platform::AUTO.is_absolute(name) { - name + let rel_path_storage; + let rel_path: &[u8] = if !outer_path.is_file() { + name } else { - bun_paths::resolve_path::join_abs::<bun_paths::resolve_path::platform::Auto>( - base_dir, name, - ) + let abs_path: &[u8] = + if bun_paths::resolve_path::Platform::AUTO.is_absolute(name) { + name + } else { + bun_paths::resolve_path::join_abs::< + bun_paths::resolve_path::platform::Auto, + >(base_dir, name) + }; + rel_path_storage = bun_paths::resolve_path::relative_alloc(chunk_abs_dir, abs_path)?; + &rel_path_storage }; - let rel_path = bun_paths::resolve_path::relative_alloc(chunk_abs_dir, abs_path)?; - let mut quote_buf = MutableString::init(rel_path.len() + ", ".len() + 2)?; + let mut quote_buf = MutableString::init(rel_path.len() + ", ".len() + 2)?; quote_buf.append_assume_capacity(b", "); - js_printer::quote_for_json(&rel_path, &mut quote_buf, false)?; + js_printer::quote_for_json(rel_path, &mut quote_buf, false)?; joiner.push_owned(quote_buf.to_default_owned()); } }🤖 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/bundler/LinkerContext.rs` around lines 1308 - 1327, The inner source name resolution code unconditionally performs filesystem path operations (dirname, join_abs, relative_alloc) even for non-file namespace inputs, which rewrites logical source names to filesystem-relative paths. Fix this by adding a check for outer_path.is_file() before the block that processes ism.map.external_source_names.iter(). Only apply the filesystem resolution operations (dirname, join_abs, relative_alloc) when the outer_path is actually a file; for non-file inputs like plugin or virtual namespaces, preserve the source names as logical identifiers without filesystem resolution.
🤖 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/bake/DevServer.rs`:
- Around line 4705-4707: The source-map hash in the loop iterating over
map.inner_sources is vulnerable to collisions because path and escaped_content
are hashed sequentially without delimiters, allowing different inputs to produce
identical bytes. Include the count of inner sources in the hash before the loop,
and add length-prefixes to each variable-width field (path and escaped_content)
when calling source_map_hash.update to ensure distinct inputs produce distinct
hashes and cover all inputs that shape the output.
---
Outside diff comments:
In `@src/bundler/LinkerContext.rs`:
- Around line 1308-1327: The inner source name resolution code unconditionally
performs filesystem path operations (dirname, join_abs, relative_alloc) even for
non-file namespace inputs, which rewrites logical source names to
filesystem-relative paths. Fix this by adding a check for outer_path.is_file()
before the block that processes ism.map.external_source_names.iter(). Only apply
the filesystem resolution operations (dirname, join_abs, relative_alloc) when
the outer_path is actually a file; for non-file inputs like plugin or virtual
namespaces, preserve the source names as logical identifiers without filesystem
resolution.
🪄 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: affc8224-e77b-4295-acbd-9e6673bd0850
📒 Files selected for processing (6)
src/bundler/LinkerContext.rssrc/bundler/LinkerContext.zigsrc/js_printer/lib.rssrc/runtime/bake/DevServer.rssrc/runtime/bake/dev_server/source_map_store.rssrc/sourcemap/InputSourceMap.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bundler/LinkerContext.rs (1)
1169-1180:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPad inner
sourcesContentto match the emitted source slots.
sources[]emits one slot perexternal_source_names, but this loop emits onlysources_content.len()entries. Valid input maps with omitted or partialsourcesContentleave the final arrays out of sync; pad missing inner entries withnull.Suggested fix
- if let Some(ism) = input_source_maps[index as usize].as_deref() { - for content in ism.sources_content.iter() { + if let Some(ism) = input_source_maps[index as usize].as_deref() { + for i in 0..ism.map.external_source_names.len() { + let content = ism + .sources_content + .get(i) + .map(|content| content.as_ref()) + .unwrap_or(b""); j.push_static(b",\n "); if !content.is_empty() { let mut quote_buf = MutableString::init(content.len() + 2)?; js_printer::quote_for_json(content, &mut quote_buf, false)?; j.push_owned(quote_buf.to_default_owned());As per coding guidelines, “One source of truth; update every consumer atomically.”
🤖 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/bundler/LinkerContext.rs` around lines 1169 - 1180, The loop iterating over ism.sources_content in the LinkerContext.rs file only emits entries that exist in sources_content, but it should emit one entry for each external source that was added to the sources array. Instead of iterating only over ism.sources_content.iter(), iterate over the same number of slots as external_source_names (or its length), and for each iteration check if that index exists in sources_content; if it does, emit the quoted content, and if it doesn't, emit null. This ensures the sourcesContent array length matches the sources array length.Source: Coding guidelines
🤖 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/bundler/LinkerContext.rs`:
- Around line 1322-1338: The code in the is_file branch is passing all
non-platform-absolute names through join_abs and relative_alloc, which
normalizes URL-like source names such as https://, webpack://, or //host/
patterns as filesystem paths instead of preserving them verbatim. Add a check
before the if bun_paths::resolve_path::Platform::AUTO.is_absolute(name)
condition to detect and short-circuit URL-like names, assigning them directly to
rel_path without path normalization. Reuse an existing in-tree predicate used
for sourcemap URL handling instead of hand-writing a new URL detection check.
In `@test/regression/issue/26713.test.ts`:
- Around line 142-143: The code dereferences mapFile with a non-null assertion
without first checking if mapFile actually exists, which will throw a TypeError
if the find() operation returns undefined instead of providing a clear test
failure. Add an explicit assertion or conditional check immediately after the
find() call to verify that mapFile exists before attempting to access
mapFile!.path on the next line, following the same pattern used elsewhere in
this test file.
---
Outside diff comments:
In `@src/bundler/LinkerContext.rs`:
- Around line 1169-1180: The loop iterating over ism.sources_content in the
LinkerContext.rs file only emits entries that exist in sources_content, but it
should emit one entry for each external source that was added to the sources
array. Instead of iterating only over ism.sources_content.iter(), iterate over
the same number of slots as external_source_names (or its length), and for each
iteration check if that index exists in sources_content; if it does, emit the
quoted content, and if it doesn't, emit null. This ensures the sourcesContent
array length matches the sources array length.
🪄 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: ea1bccc1-04e3-4241-9383-4f19bb4d274b
📒 Files selected for processing (3)
src/bundler/LinkerContext.rssrc/sourcemap/InputSourceMap.rstest/regression/issue/26713.test.ts
Inner-source names like webpack:///foo.ts or //host/path are virtual identifiers, not filesystem paths; joining them against the intermediate's dirname mangles them. Added bun_sourcemap::is_url_like_source_name (shared with the sourceMappingURL scheme check in parse_from_source_with_fs) and guard both write_sources_for and collect_inner_sources with it. Covered by a new test case. Also add expect(mapFile).toBeDefined() before non-null dereferences in the sourceRoot and missing-sidecar tests.
…cemap URLs The sourceMappingURL value and inner sources[] names are arbitrary bytes from file content; use join_abs_string_buf_checked so an overlong entry falls back to None / the raw name instead of panicking on the fixed PathBuffer.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bundler/LinkerContext.rs (1)
1323-1343:⚠️ Potential issue | 🟠 MajorUse the checked join helper for sourcemap-provided source names.
The URL-like passthrough correctly skips processing of virtual names, but the file-backed branch feeds arbitrary
sources[]bytes from the sourcemap into uncheckedjoin_abs. Per the coding guidelines, "Treat all size/index/length arithmetic on external data as adversarial." Route this throughjoin_abs_string_buf_checkedwith a pooled buffer and emit the rawnamewhen the checked join fails — this guards against overlong relative source entries that would otherwise overflow the implicit thread-local buffer.🤖 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/bundler/LinkerContext.rs` around lines 1323 - 1343, The code feeds untrusted sourcemap-provided bytes directly to the unchecked join_abs function when handling file-backed source names. Replace the call to join_abs with join_abs_string_buf_checked, passing a pooled buffer for safe handling of potentially overlong relative source entries. When the checked join operation fails, fall back to emitting the raw name instead of the result.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/bundler/LinkerContext.rs`:
- Around line 1323-1343: The code feeds untrusted sourcemap-provided bytes
directly to the unchecked join_abs function when handling file-backed source
names. Replace the call to join_abs with join_abs_string_buf_checked, passing a
pooled buffer for safe handling of potentially overlong relative source entries.
When the checked join operation fails, fall back to emitting the raw name
instead of the result.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1fa67a8d-c5a2-43ac-866c-bfad961f7abb
📒 Files selected for processing (5)
src/bundler/LinkerContext.rssrc/runtime/bake/DevServer.rssrc/sourcemap/InputSourceMap.rssrc/sourcemap/lib.rstest/regression/issue/26713.test.ts
Collaborator
Author
|
Diff is ready for review at 354dfdd. All bot review threads are resolved. CI note: every build on this branch (#63226, #63235, #63243, #63247, #63252) has had the Windows test lanes canceled at "Failed to create agent" before any tests run; the non-Windows lanes pass or are still running. This is agent-provisioning infra, not the diff. Two known follow-ups flagged by reviewers that I've intentionally left out to let CI complete:
Happy to address either in this PR if preferred, or as follow-ups. |
…y check bun build --sourcemap=linked emits platform separators in sources[] (src\main.ts on Windows); normalize before comparing so the fixture setup works on Windows aarch64.
Collaborator
Author
|
CI across five completed builds on this branch: #63252 (267 pass), #63263 (284), #63482 (285), #64923 (284), #65324 on head c67b78d (283). Each run's 1-2 failures are a different unrelated item: Docker Hub Ready for review. |
Conflicts were all modify/delete on .zig reference files that main removed entirely; resolved by accepting the deletion (functional changes are all in .rs). Also removed the new InputSourceMap.zig for consistency, and adapted to e::Number.value becoming a getter.
…_map fields faf54b5 updated js_printer::Options::input_source_map to say (inline data: URL or a sidecar .map file resolved on disk) but missed the three parallel fields carrying the same data: Graph::InputFile, ParseTask::Success, and Chunk::NewBuilder. Align them.
One conflict in LinkerContext::generate_source_map_for_chunk: main replaced the inline relative_alloc + platform_to_posix_in_place pair with a source_map_relative_path helper, while this branch had already replaced the whole loop with the multi-slot write_sources_for version. Kept the multi-slot loop and switched write_sources_for to the new helper so it also picks up the Windows posix-separator normalization.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #26713. Also closes #30536 and #6173 (this branch extends #30539 with external-map loading and dev-server support; if this lands, #30539 can be closed).
What does this PR do?
Repro
Before:
sources: ["bun://Bun/Bun HMR Runtime", ".../index.html", ".../main.js"]. Browser DevTools showsmain.js, notsrc/main.ts.After:
sources: [..., ".../main.js", ".../src/main.ts"],sourcesContentcarries the original TypeScript, and every mapping resolves through tosrc/main.ts.Same bug reproduces with
development: false(prod HTML bundler path) and plainBun.build({ entrypoints: ["./main.js"], sourcemap: "external" }).Cause
The bundler (and the dev server's re-bundler) emits a fresh sourcemap mapping the output to its immediate input (
main.js). The//# sourceMappingURL=main.js.mapcomment onmain.jsis detected by the lexer but never consumed, so the chain back tosrc/main.tsis dropped.#30539 added the chaining infrastructure for inline
data:URLs on theBun.build/LinkerContextpath, but left external.mapreferences out of scope and explicitly gated out the dev server becauseSourceMapStorehard-coded onesources[]slot per input file.Fix
External
.maploading (src/sourcemap/InputSourceMap.rs,src/bundler/ParseTask.rs): when the input lives in thefilenamespace and its trailingsourceMappingURLis not adata:URL, resolve it relative to the input's directory and read the sidecar viabun_sys::File::read_from.http(s):/// protocol-relative URLs are skipped. Read or parse failure falls back silently.Dev-server multi-source stitching (
src/runtime/bake/dev_server/{packed_map,source_map_store,incremental_graph}.rs,DevServer.rs):PackedMapgainsinner_sources: Box<[InnerSource]>(path + JSON-quoted content per inner source) andEndState.source_index(chunk-local index of the last mapping).finalize_bundlereadsgraph.input_files.input_source_mapper file, resolves inner source names to absolute paths against the intermediate's directory, JSON-quotes their contents, and passes them throughreceive_chunk.render_jsonemits each file's path followed by its inner-source paths insources[]and likewise forsourcesContent[].join_vlqtracks a running base index; each file's chunk starts at its base andprev_end_state.source_index = base + chunk.end_state.source_index, so chunk-local source indices compose correctly across files.Entry::lookup_sourcemaps a flatsource_indexback to(path, escaped_content);ErrorReportRequestuses it instead ofpaths[idx-1]/files[idx-1]so browser error reports remap through the chain too.The
self.dev_server.is_none()gate inLinkerContext::print_code_for_file_in_chunk_jsis removed now that the dev server stitcher handles multi-slot files.Merge note (f04b85e)
Main removed all
.zigreference files from the repo; this branch (via its base #30539) carried edits to seven of them plus a newInputSourceMap.zig. All conflicts were modify/delete on those.zigfiles; resolved by accepting main's deletion and removingInputSourceMap.zigtoo, since the functional changes are all in the.rscounterparts. Also adapted one call site toe::Number.valuebecoming a getter on main.Merge note (824273d)
Main replaced the linker's inline
relative_alloc+ POSIX-separator normalization with asource_map_relative_pathhelper inside the same loop that this PR had already rewritten into the multi-slotwrite_sources_forform. Kept this PR's loop and switchedwrite_sources_forto the new helper so the chained inner-source paths also get the Windows separator normalization.How did you verify your code works?
bun bd test test/regression/issue/26713.test.ts→ 5 pass (3 fail on current main; the 2 fallback cases pass on both):development: true) with linked sidecar → authored source + content surfacedevelopment: false) → authored source surfacesBun.buildwith external.map→ authored source + content surfacehttp://URL → build succeeds, falls back to intermediatebun bd test test/bake/dev/sourcemap.test.ts→ 2 pass (existing dev-server sourcemap coverage).bun bd test test/bundler/bun-build-api.test.ts→ 54 pass (8 inline-chain tests from #30539 + the rest).bun bd test test/js/bun/http/bun-serve-html.test.ts→ 16 pass.