Conversation
A #!/usr/bin/env bun hashbang on the entry file sets that file's per-file target to Bun, but every other file in the chunk is printed according to the build target. When building with --target=node or --target=browser, dependencies are printed with raw UTF-8 identifiers while the chunk still carries the // @Bun pragma. Bun's loader trusts the pragma and reads the bytes as Latin-1, so the bundle it just produced fails with SyntaxError: Invalid character. Gate the pragma on c.options.target.is_bun() so it matches the encoding the printer actually produced for the whole chunk.
|
Updated 12:44 PM PT - Jul 9th, 2026
❌ @robobun, your commit efb4765 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 33859That installs a local version of the PR into your bun-33859 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
WalkthroughThis change modifies how the bundler determines whether to emit Bun-specific wrapper output for a chunk, switching from per-file source target inspection to the global build target. Corresponding bundler tests verify hashbang and pragma output across node, browser, and bun targets. ChangesBun hashbang/wrapper target logic
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Heads up: #33862 fixes the underlying target-propagation bug (514d37b dropped the The two changes touch different files and do not conflict. This PR's approach (gate the pragma on the build target) is still a reasonable belt-and-suspenders in case some other per-file target divergence is introduced later. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The build-target check alone regresses browser JS chunks emitted by a --target=bun HTML or server-components build: c.options.target is Bun but every part in the chunk was printed for Target::Browser with raw UTF-8. Keep the original per-entry target check and also exclude IS_BROWSER_CHUNK_FROM_SERVER_BUILD so the pragma is only emitted when the whole chunk was printed ASCII-only.
The same is_bun local gates the @bun-cjs function-expression wrapper. Before the predicate fix, a #!/usr/bin/env bun entry built with --format=cjs --target=node emitted that wrapper, which only Bun's own loader calls. Under node or any other CJS host the module body was parsed and discarded (exit 0, no output). Asserts the wrapper is absent for target=node/browser and that the node build runs under node. target=bun is the positive control.
robobun
changed the title
There was a problem hiding this comment.
LGTM — the three-way gate strictly narrows the original check, so this can only drop the pragma/wrapper where it was wrong, and the mixed-target regression from the first iteration is resolved.
Extended reasoning...
Overview
Changes one boolean in postProcessJSChunk.rs: is_bun now requires c.options.target.is_bun() && entry_target.is_bun() && !IS_BROWSER_CHUNK_FROM_SERVER_BUILD instead of just entry_target.is_bun(). Six new itBundled cases in test/bundler/bundler_bun.test.ts cover hashbang + {node, browser} × {esm, cjs} plus positive controls for --target=bun.
Security risks
None. This gates emission of a comment pragma and a CJS function-expression wrapper in bundler output; no auth, crypto, filesystem, or network surface.
Level of scrutiny
Bundler linker is a hot/critical path, but this change is strictly narrowing: the new expression ANDs two more predicates onto the original one, so is_bun can only flip true→false, never false→true. That means no chunk gains a pragma or wrapper it lacked before — the only observable effect is dropping them where the build target is not Bun (the reported bug) or where the browser-chunk flag is set (belt-and-suspenders; those chunks already had entry_target=Browser). The wrapper open and close both read the same is_bun local, so they remain balanced.
Other factors
- I flagged a regression on the first iteration (which replaced rather than narrowed the check); the author addressed it in 5cd61af and CI's
html-import-manifest.test.tssnapshot confirmed the fix. Thread resolved. - Test coverage is thorough: negative assertions (no
// @bun, no wrapper) for node/browser, positive assertions for--target=bun, a runtime execution check under node for the CJS case, and a non-ASCII-identifier dependency that exercises the actual encoding bug. - No CODEOWNERS on these paths. No bugs from the bug-hunting pass.
- robobun noted #33862 fixes the same issue from the other side (target propagation); the two are complementary and non-conflicting.
There was a problem hiding this comment.
LGTM — the three-way gate is a strict narrowing of the original condition, so no regression surface, and the mixed-target concern from my earlier review is addressed.
Extended reasoning...
Overview
Changes a single is_bun boolean in postProcessJSChunk.rs from entry_target.is_bun() to c.options.target.is_bun() && entry_target.is_bun() && !IS_BROWSER_CHUNK_FROM_SERVER_BUILD. This gates emission of the // @bun pragma and the @bun-cjs function-expression wrapper. Adds 6 itBundled cases in test/bundler/bundler_bun.test.ts covering node/browser/bun targets × esm/cjs formats, including positive controls that assert --target=bun output is unchanged.
Security risks
None. This is bundler output-header logic with no user input parsing, no auth/crypto/permissions, no external I/O.
Level of scrutiny
Moderate — bundler linker output is a critical path, but the change is a strict narrowing: the original per-entry check is retained and ANDed with two additional constraints. The pragma can therefore only be removed from cases where it was previously emitted, never added. The only behavioral delta is the intended fix (hashbang-derived Bun entry target under a non-Bun build target no longer gets the pragma/wrapper). The wrapper open and close both key off the same is_bun local, so they remain balanced.
Other factors
- My earlier review flagged that gating solely on
c.options.targetwould regress HTML/server-component browser chunks; the author confirmed via CI (html-import-manifest.test.tssnapshot break) and fixed it in 5cd61af by re-adding the per-entry check plus the browser-chunk flag. That thread is resolved. - The bug hunting system found no issues on the current revision.
- Test coverage is thorough: negative cases (no pragma for node/browser, verified by running the output), positive controls (pragma still present for
--target=bun), and aruntime: "node"execution check for the--format=cjsnode case that would have caught the original silent-no-op bug. - robobun noted PR #33862 fixes the same user-visible symptom via target propagation; the two changes are in different files, don't conflict, and this gate remains correct as belt-and-suspenders.
|
CI on build 71120 is red on lanes this diff does not touch:
No bundler test is in either list; The diff itself is green and ready for review. |
|
Re-checked against main at 165dc9f now that #25767 is closed. The four negative tests from this PR ( #25767 was closed after verifying the issue's literal repro, which has no hashbang. On main, a |
|
Heads-up from #39715: that PR adds |
|
Verification against current main (731aa92). I applied this PR's six test cases on main without its source change:
With the source change applied on main, all six pass. #25767 was closed as fixed on main. That verification used the snippet from the issue body, which has no hashbang. That snippet does not reproduce the bug on 1.3.4 or on main. The hashbang case still reproduces on main, so I reopened the issue. This PR is still needed. #33862 fixes the same hashbang inconsistency from the other side: it makes the dependencies inherit the |
This branch has not been deployed
1 participant
Fixes #25767
What does this PR do?
A
#!/usr/bin/env bunhashbang on an entry file sets that one file's per-file target toTarget::Bun(viatarget_from_hashbang), regardless of--target. The linker then emitted the// @bunpragma, and under--format=cjsthe// @bun @bun-cjsfunction-expression wrapper, whenever the entry's per-file target was Bun.--format=cjs: silent no-op under nodeThe wrapper is a function expression statement that only Bun's own module loader (via the
@bun-cjspragma) ever invokes. Under node and every other CommonJS host the module body is parsed and discarded with no build-time or run-time error. Removing the hashbang produces flat CJS that runs correctly.--format=esm: mojibake /SyntaxErrorunder bunThe printer's ASCII-only escaping policy is decided per file from
ast.target: the entry (target=Bun) escapes non-ASCII identifiers, but every bundled dependency follows the build target (node/browser) and keeps them as raw UTF-8. The pragma tells Bun's loader the whole file is safe to read as Latin-1, so it mis-decodes the UTF-8 bytes from dependencies and rejects the bundle it just produced.The same mismatch produces mojibake for string literals from dependencies (#25767):
Node runs the same output file without error.
Repro:
Fix
Gate
is_buninpostProcessJSChunk.rson three conditions so the pragma and the@bun-cjswrapper are only emitted when the whole chunk was printed for Bun:c.options.target.is_bun(): the build target determines what every non-entry dependency is printed with, and it is the target the output must run under. Fixes both hashbang cases above.ast_targets[entry].is_bun()(the original check): a--target=bunHTML entry produces a browser-printed JS chunk whose entry target isBrowser; the flag below is not set on that path.!IS_BROWSER_CHUNK_FROM_SERVER_BUILD: a server build importing HTML produces browser chunks, and a code-split chunk may have a Bun-targeted entry while later files are browser-targeted.The
@bun-cjswrapper open and close both key off the sameis_bunlocal, so they stay balanced.--target=bunwith a plain JS entry is unchanged: pragma and wrapper are still emitted, identifiers are still escaped, output still runs.How did you verify your code works?
Added six
itBundledcases intest/bundler/bundler_bun.test.ts:bun/HashbangBunNoPragmaFor_nodeandbun/HashbangBunNoPragmaFor_browser: a#!/usr/bin/env bunentry importing a dependency with a non-ASCII method name, built for node/browser. Assert no// @bunin the output and that the bundle runs in Bun and prints7. Both fail on the released binary (pragma present, bundle throwsSyntaxError).bun/HashbangBunNoCjsWrapperFor_nodeandbun/HashbangBunNoCjsWrapperFor_browser: a#!/usr/bin/env bunentry built with--format=cjsfor node/browser. Assert no@bun-cjspragma and no function-expression wrapper. The node case additionally runs the output under node and assertsALIVE 42on stdout. Both fail on the released binary (wrapper present, node emits nothing).bun/HashbangBunPragmaForBunTargetandbun/HashbangBunCjsWrapperForBunTarget: same inputs with--target=bun. Assert the pragma / wrapper are still present and the bundle runs. Pass before and after (positive controls).test/bundler/html-import-manifest.test.tscovers the browser-chunk regression (an earlier iteration of this PR that gated only on the build target broke its etag snapshots).bun bd test test/bundler/bundler_bun.test.ts test/bundler/bundler_banner.test.ts test/bundler/bundler_cjs.test.ts test/bundler/html-import-manifest.test.ts test/bundler/bundler_html.test.ts test/bundler/bundler_html_server.test.ts: pass, 0 fail.[review] gate passed · iteration 1 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 4 passed · 0 rejected · iteration 1
evidence per changed file