Conversation
--compile internally forces the bundler's source_map option to External so the standalone module graph receives the map as a separate OutputFile to embed. The post-compile sidecar write checked that post-override value, so any non-none --sourcemap mode (bare/linked/inline) also dropped an <entry>.js.map next to the outfile even though the map is already embedded in the executable. Record whether the user explicitly requested external before the override and gate the sidecar write on that instead. Closes #8341
|
Status: diff is green. 17/17 pass in The CI failure on this PR is Reproduced with: printf 'function f(){throw new Error("x")}\nf()\n' > thr.ts
bun build --compile thr.ts --outfile app --sourcemap
ls thr.js.map # exists before this PR, gone afterAlso covers the |
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Same fix as the CLI path. JSBundler forces source_map to External so the standalone graph receives the map as a separate OutputFile; the completion task then wrote every Sourcemap output to disk regardless of whether the user requested an external file. Record the user's choice on CompileOptions before the override and gate the sidecar write on it.
There was a problem hiding this comment.
LGTM — the JS API path is now fixed alongside the CLI path, addressing my earlier review.
What was reviewed:
compile_sourcemap_sidecar/CompileOptions.sourcemap_sidecarcapture the pre-override choice before both entry points forceExternal; gates inbuild_command.rs:937andjs_bundle_completion_task.rs:447are correctly placed.- Confirmed the standalone-HTML branch is unaffected (that path clears
config.compilesodo_compilationnever runs), and thatsourcemap: true/ string-target / objectcompileforms all reach the override block that sets the flag. - Existing
external+ splitting tests still hitsourcemap_sidecar = true; the removedopt_source_mapsnapshot had no other consumers.
Extended reasoning...
Overview
Fixes #8341: bun build --compile --sourcemap[=inline|linked] and Bun.build({ compile: true, sourcemap: 'inline'|'linked'|true }) were writing a stray <entry>.js.map sidecar even though the map is embedded in the executable. The fix records whether the user's original choice was external before the compile path forces source_map = External (which it must, since the standalone module graph consumes the map as a separate OutputFile), and gates the post-compile disk write on that recorded intent. Applied symmetrically to both the CLI (build_command.rs) and the JS API (JSBundler.rs → js_bundle_completion_task.rs), with parameterized tests covering all four sourcemap spellings on each entry point.
Prior review
My previous review on this PR flagged that the JS API path had the identical bug and wasn't fixed. Commit 64ffca1 addressed it exactly as suggested: added CompileOptions.sourcemap_sidecar: bool (default false), set it in Config::from_js before the override, and added && compile_options.sourcemap_sidecar to the sidecar-write branch in do_compilation. A matching describe.each test block was added asserting both no .map on disk and no kind: "sourcemap" in result.outputs for non-external modes. That resolves the finding.
Security risks
None. This only narrows when a .map file is written next to a compiled executable; no new inputs are parsed, no paths are derived from untrusted data beyond what already existed.
Level of scrutiny
Low-to-medium. The change is a small boolean gate on existing write paths; the tricky part (why the override-to-External exists at all) is untouched. I traced the flag through every construction site of CompileOptions (default, boolean compile: true, string compile: 'bun-…', object compile: {…}, and target: 'bun-…') — all reach the if let Some(compile) = this.compile.as_mut() block that sets sourcemap_sidecar from the pre-override this.source_map, so no path leaves it stale. The standalone-HTML branch skips that block, but it also nulls config.compile in configure_bundler, so do_compilation (the only reader) never runs for it.
Other factors
- The removed
opt_source_mapsnapshot inbuild_command.rshad exactly one consumer (the line being replaced); grep confirms no other references. - Existing tests in the same file (
externalwrites .map, splitting writes multiple .map, subdir outfile) all use'external'and remain green under the new gate. The pre-existingtestSourcemapOptionand "multiple source files" tests use'inline'/trueand only assert the embedded map resolves — they don't index intoresult.outputsin a way the now-dropped sourcemap entry would break. - Test hygiene looks good:
tempDir+using, per-mode temp dirs, subprocess pipes drained concurrently, exit code asserted last, CLI cases usetest.concurrent.
…nks can load it (#41264) ### Problem - In an executable built with `--compile --splitting`, an `import()` or `require()` of the entry point from another chunk fails: `ResolveMessage: Cannot find module '/$bunfs/root/main.js' imported from /$bunfs/root/tool.js`. It happens with `bun build` and `Bun.build`, on 1.4.1-canary.1 and main. - The linker prints the path of the entry point's chunk, `main.js`, into the other chunks. After linking, the CLI and `Bun.build` renamed that output file to the name of the outfile (`build_command.rs:807`, `js_bundle_completion_task.rs:334`). So the executable has no `/$bunfs/root/main.js`. ### Fix - `compute_chunks` names the entry point's chunk after the outfile (new option `compile_entry_point_name`). It is the chunk that `StandaloneModuleGraph::to_bytes` embeds as the entry point: the first server-side `EntryPoint` (`chunk_side`, `chunk_output_kind`). The post-link renames are removed. - Each path printed for the chunk now names the embedded module: `import()`, `import.meta.require`, module records, and the bytecode source URL. - Visible change: the external source map of the entry point is `<outfile>.map`, not `<entry>.js.map`. Its metafile output is `./<outfile>`. - Verified: 5 new tests in `test/bundler/bundler_compile_splitting.test.ts` and 1 stricter test in `bun-build-compile-sourcemap.test.ts`, which fail on 1.4.1-canary.1. Self-reviewed: 4 concerns raised, 4 addressed. The notes list the other suites. ### Background - `--compile` embeds each output file at `/$bunfs/root/<path>`. The entry point is embedded under the outfile's name, so `import.meta.path` is `/$bunfs/root/<outfile>`. - With `--splitting`, a chunk refers to another chunk by its output path. In an executable, that path starts with `/$bunfs/root/`. - Since #40519, a split `require()` of an ES module is a chunk boundary. It prints `import.meta.require("<path>")`. <details><summary>Notes</summary> - Found while testing another change. No issue reports it. - Released behavior (bun 1.4.0): with one entry point, a lazy chunk's `import()` of it fails, and `require()` of it works. `require()` fails on main because of #40519, which is not released. With two entry points, 1.4.0 runs the program, but the main entry point runs twice. - The fix covers specifiers that the bundler resolves. A computed specifier, such as `import(name)`, still resolves at run time against the embedded names, as before. - Source map name: `bun build --compile --sourcemap` writes the external map for every mode (the mode is forced to `external`, `build_command.rs:321`). So a script that reads `<entry>.js.map` next to the executable must read `<outfile>.map` now. The new name matches the module name in stack traces. Two executables built from the same entry name into one directory no longer write the same map file. #36384 (open) changes which modes write the map file. It does not change the name. - For an empty outfile, `.`, `..` and `../`, the CLI writes the executable as `index`. The entry point is now embedded as `index` too (`compile_outfile`). Before, `--outfile .` embedded it as `/$bunfs/root/`, and relative specifiers from the entry point did not resolve. Main has the same problem. - An outfile name that equals the chunk name of another entry point (`--outfile tool.js` with `tool.ts` as the second entry point) is now a "Multiple files share the same output path" error. Before, the build succeeded, and the executable loaded the main entry point in place of `tool.js`. - `chunk_side` and `chunk_output_kind` have the same text and position as in #41235, so the two PRs merge without a conflict. #41235 also finds the main module in `link` with the same rule. - `writeOutputFilesToDisk.rs` uses the two helpers too. Its old `side` code did not check `IS_BROWSER_CHUNK_FROM_SERVER_BUILD`, so a shared chunk with a browser file that is not its first file now gets `Client` there, as in memory. Nothing reads `side` for files written to disk. - `mergeSmallChunks.rs` still pins the chunks of user entry points under `--compile`. Its comment named the rename as the reason. The reason is gone, but the pin stays, because a removal changes the default chunk layout of split executables. #40601 excludes `--compile` for the same reason. - Tests that fail on the debug build with and without this diff: 3 bytecode tests in `bun-build-compile.test.ts` and 2 in `bun-build-api.test.ts` (timeouts), and `compile/HelloWorldWithProcessVersionsBun`. - Suites run on the debug build: `bundler_compile_splitting`, `bundler_compile`, `bun-build-compile`, `bun-build-compile-sourcemap`, `compile-asset-bunfs`, `compile-sourcemap-internal`, `bundler_compile_autoload`, `compile-argv`, `compile-process-execargv`, `bundler_html_server`, `standalone`, `html-import-manifest`, `metafile`, `bundler_splitting`, `bun-build-api`, `bundler_edgecase`. - Checked by hand: `--bytecode --format=esm` (the entry point and the other chunk load from bytecode), `--minify`, `--sourcemap=external`, entry points in different directories, a static and a dynamic import of the entry point in one file, and a server entry point with an HTML import. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bundler_compile_splitting.test.ts, test/bundler/bun-build-compile-sourcemap.test.ts <!-- robobun:evidence:end -->
|
Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-29, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main. |
2 participants
Closes #8341
Repro
--sourcemap,=inline,=linked, and=externalall produced a byte-identical executable and all wrote the sidecar. The sidecar is named after the entry (<entry>.js.map), so it can clobber a project's own map file sitting next to the outfile.Bun.build({ compile: true, sourcemap: 'inline' | 'linked' | true })had the same behavior.Cause
Both the CLI (
build_command.rs) and the JS API (JSBundler.rs) forcesource_map = Externalfor the executable compile path so the standalone module graph receives the map as a separateOutputFileto embed (the runtime does not read//# sourceMappingURL). The post-compile sidecar write then checked the post-override value, so it was true whenever any sourcemap mode was set.Fix
Record whether the user's original choice was
externalbefore the override runs, and gate the sidecar write on that in both paths.--sourcemap/=inline/=linked(andsourcemap: true | 'inline' | 'linked') now embed only;=external/'external'still writes the.mapnext to the executable.Verification
New parameterized tests in
test/bundler/bun-build-compile-sourcemap.test.tscover both entry points: a CLI block over all four flag spellings and a JS API block overtrue/'linked'/'inline'/'external'. Each asserts no.mapin the output dir for non-external modes, exactly one for external, and that the embedded map still resolvesthr.tsin the thrown stack. 6/8 new cases fail on main, 17/17 pass with the fix.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bun-build-compile-sourcemap.test.ts