Conversation
The runtime transpiler (plain `bun file.mjs`) was printing every
non-ASCII codepoint as an escape sequence: string literals as \xHH /
\uHHHH, identifiers as \u{HEX}, regex literals as \uHHHH, and
template raw text as \uHHHH. JavaScriptCore then stored that printed
text as the module's source, so Function.prototype.toString(),
RegExp#source, and tagged-template .raw all reflected the escaped form
instead of the author's source text.
The root cause was print_ast coupling ASCII_ONLY to IS_BUN_PLATFORM
(both driven by the same const generic) combined with the printer
output being handed to JSC via String::clone_latin1. This change:
* decouples the two flags in print_ast so the runtime path runs with
ASCII_ONLY = false, IS_BUN_PLATFORM = true
* gates regex-literal escaping on ASCII_ONLY rather than
IS_BUN_PLATFORM
* combines UTF-16 surrogate pairs in write_pre_quoted_string_inner
so astral codepoints round-trip as UTF-8 instead of paired
\uHHHH escapes (lone surrogates are still escaped)
* switches the four consumers of the printed buffer (jsc_hooks,
RuntimeTranspilerStore, AsyncModule, RuntimeTranspilerCache) from
clone_latin1 to clone_utf8, and bumps the cache version
bun build --target=bun keeps its ASCII-only output (that path goes
through print_with_writer_and_platform, untouched here), so the
existing #14976 'outputs only ascii' assertion still holds.
WalkthroughUpdates printer escaping and runtime source handling so non-ASCII output is preserved as UTF-8, renames the printer const-generic to ChangesNon-ASCII output preservation
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 3:51 PM PT - Jul 9th, 2026
❌ @robobun, your commit 67571f1 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 33868That installs a local version of the PR into your bun-33868 --bun |
|
Found 5 issues this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
On the duplicate check:
Verified that #8745, #16763, #18115, #25169 and #15492 all reproduce on the current release and pass under this branch; added |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The --watch/--hot path hands the printer buffer to JSC via ref_counted_resolved_source, which wraps the bytes as a Latin-1 external string. Now that the printer emits UTF-8, non-ASCII output was mojibaked on that path (f() returned 'café'). Check is_all_ascii and fall back to clone_utf8 when the output contains multi-byte UTF-8. Also tighten the comment on the cache vtable put() per review, and add a --hot variant plus an evaluated-value assertion to the test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/runtime/jsc_hooks.rs (1)
2648-2648: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBytecode-cache path still wraps source bytes as Latin-1.
src/runtime/jsc_hooks.rs:2648still usesclone_latin1(&source.contents), so cached modules with non-ASCII source text will mojibake inFunction.prototype.toString()and stack traces. The same call site also remains insrc/jsc/RuntimeTranspilerStore.rs:1037.Proposed fix
- source_code: bun_core::String::clone_latin1(&source.contents), + source_code: bun_core::String::clone_utf8(&source.contents),🤖 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/jsc_hooks.rs` at line 2648, The bytecode-cache path is still cloning source bytes as Latin-1, which can corrupt non-ASCII module text in toString and stack traces. Update the `source_code` handling in `jsc_hooks` and the matching call in `RuntimeTranspilerStore` to preserve the original source encoding instead of `clone_latin1(&source.contents)`, using the same source representation as the non-cached path so cached and uncached modules behave identically.
🤖 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 `@test/js/bun/transpiler/transpiler-non-ascii-source-text.test.ts`:
- Around line 41-106: The tests in the runtime transpiler non-ASCII source suite
are independent subprocess/temp-dir cases, so they should run concurrently
instead of sequentially. Update the top-level describe in
transpiler-non-ascii-source-text.test.ts to use describe.concurrent (or convert
the individual test() calls to test.concurrent) around the existing fixtures so
the Bun subprocess runs and tempDir usage can execute in parallel. Keep the
existing assertions in the same test bodies; only change the test registration
style for the describe block and the named tests inside it.
---
Outside diff comments:
In `@src/runtime/jsc_hooks.rs`:
- Line 2648: The bytecode-cache path is still cloning source bytes as Latin-1,
which can corrupt non-ASCII module text in toString and stack traces. Update the
`source_code` handling in `jsc_hooks` and the matching call in
`RuntimeTranspilerStore` to preserve the original source encoding instead of
`clone_latin1(&source.contents)`, using the same source representation as the
non-cached path so cached and uncached modules behave identically.
🪄 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: 91674d52-3110-42e6-a12c-1db9511154b4
📒 Files selected for processing (10)
src/js_printer/lib.rssrc/jsc/AsyncModule.rssrc/jsc/RuntimeTranspilerCache.rssrc/jsc/RuntimeTranspilerStore.rssrc/jsc/VirtualMachine.rssrc/runtime/cli/repl.rssrc/runtime/jsc_hooks.rstest/bundler/transpiler/transpiler.test.jstest/js/bun/transpiler/transpiler-non-ascii-source-text.test.tstest/regression/issue/14976/14976.test.ts
- jsc_hooks.rs / RuntimeTranspilerStore.rs: the already_bundled path (// @Bun pragma) still wrapped raw file bytes as Latin-1; switch to clone_utf8 so hand-written or future non-ASCII pragma files are decoded correctly. Added a test case. - transpiler.rs: update the prose comment that still named the old print_ast ASCII_ONLY const generic. - test file: switch to describe.concurrent per test conventions.
|
Addressed the remaining review feedback in 3bbc8ae:
All 7 test cases fail on 1.3.14 and pass on this branch. |
The test was asserting the old .raw escape behaviour with a TODO saying to fix it once unicode in template tags works. It works now; update the expectation to the literal output.
There was a problem hiding this comment.
All my earlier feedback is addressed and I found nothing new, but this changes the encoding contract between the printer and every JSC source-code consumer on the runtime hot path — worth a human pass.
Extended reasoning...
Overview
This PR decouples ASCII_ONLY from IS_BUN_PLATFORM in print_ast so the runtime transpiler emits raw UTF-8 instead of ASCII-escaped output, then updates every consumer of that buffer (jsc_hooks, RuntimeTranspilerStore, AsyncModule, RuntimeTranspilerCache, ref_counted_resolved_source) to decode as UTF-8 instead of Latin-1. It also teaches write_pre_quoted_string_inner to combine UTF-16 surrogate pairs, gates regex-literal escaping on ASCII_ONLY rather than IS_BUN_PLATFORM, and bumps the on-disk transpiler cache version to 23. Twelve files touched across the printer, JSC glue, VirtualMachine, REPL, and four test files.
Security risks
None identified. This is an internal encoding change; no new user-controlled input reaches a parser or filesystem path, and the surrogate-pair combiner still escapes lone surrogates.
Level of scrutiny
High. The printer → JSC source handoff runs on every bun file.js invocation, and encoding mismatches here corrupt evaluated string values (not just .toString() cosmetics) — exactly what happened on the --hot path during review before 7345f95. The fix now branches on is_all_ascii inside ref_counted_resolved_source and adds a --hot test variant, but the number of sibling consumers (watcher path, async-module path, already-bundled path, cache write path, cache read path) and the fact that one was already missed once argue for a maintainer confirming the set is complete. The surrogate-combining change in the UTF-16 arm of write_pre_quoted_string_inner also affects Bun.Transpiler output (the updated transpiler.test.js expectations reflect a user-visible behaviour change there).
Other factors
All three of my earlier inline threads (the --hot Latin-1 regression, the stale ASCII_ONLY comment in transpiler.rs, and the exact-empty-stderr assertions) are resolved in 7345f95 / 3bbc8ae / a1c2cd2. The new test file covers .mjs/.cjs/.ts/--hot/// @bun and the un-skipped 14976 assertion exercises the shell path. The CodeRabbit describe.concurrent nit was also taken. No new findings from the bug-hunting pass on the current head. This is well-tested and looks correct to me, but the blast radius (every non-ASCII source file, every module-load path) puts it outside what I'll approve without a human look.
|
CI is green on everything related to this diff. The remaining red across build 71160 and build 71168 is unrelated infra/flake:
None of those touch the printer or JSC source-string paths this PR changes. The new test file and every test I updated ( |
|
Closing in favour of #33866, which has been rebased onto current main. This PR turned ASCII escaping off for the whole runtime transpiler path; #33866 keeps escaping for identifiers, string literals and cooked templates (so the zero-copy Latin-1 loading path still applies to almost every module and Bun.Transpiler / bun build --target=bun output is unchanged for existing code) and prints only the two spec-observable pieces of text, regex literals and tagged template raw text, verbatim. It also covers bun build --target=bun, --compile, --bytecode and NODE_COMPILE_CACHE, which this approach left escaped. The --hot test case and the ref-counted source handling from here were folded into #33866. |
1 participant
What
Under plain
bun file.mjs, the runtime transpiler rewrote every non-ASCII codepoint to an escape sequence before handing the source to JavaScriptCore. That escaped text is what JSC stores as the module source, so it leaked into three observable APIs:Node and browsers return the author's source text verbatim (ES2026 §20.2.3.5).
new Function('return "café"').toString()already worked in Bun because dynamic bodies never pass through the file transpiler.Cause
print_astcoupledASCII_ONLYtoIS_BUN_PLATFORM(same const generic filled both slots), and the printed buffer was then handed to JSC viaString::clone_latin1, so the printer had to ASCII-escape everything.Fix
print_astso the runtime path runs withASCII_ONLY = false, IS_BUN_PLATFORM = true.ASCII_ONLYrather thanIS_BUN_PLATFORM.write_pre_quoted_string_innerso astral codepoints round-trip as UTF-8 instead of paired\uHHHHescapes. Lone surrogates are still escaped.jsc_hooks,RuntimeTranspilerStore,AsyncModule,RuntimeTranspilerCache) fromclone_latin1toclone_utf8, and bump the cache version.bun build --target=bunkeeps its ASCII-only output (that path goes throughprint_with_writer_and_platform, untouched here), so the existing #14976 "outputs only ascii" assertion still holds. The previously-skippedString.rawassertion in that file now passes and is un-skipped.Verification
New test at
test/js/bun/transpiler/transpiler-non-ascii-source-text.test.tscovers string literals, identifiers, method names, template literals with substitution, regex literals, tagged-template.rawwith BMP + astral codepoints, andRegExp#source, across.mjs/.cjs/.ts. All 5 cases fail on the current release (USE_SYSTEM_BUN=1) and pass with this change.Two existing
Bun.Transpilerassertions intest/bundler/transpiler/transpiler.test.jswere encoding the old behaviour and have been updated, with two extra lone-surrogate cases added.Related
Supersedes #33866, which exempts only regex/template-raw from escaping; this PR turns off ASCII escaping for the whole runtime path so
Function.prototype.toStringis also correct for string literals and identifiers. Earlier attempts at the same bug class: #31394 (conflicting) and #15047 (pre-Rust, conflicting).Fixes #8745
Fixes #16763
Fixes #18115
Fixes #25169
Fixes #15492
no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/shell/bunshell.test.ts