Repository navigation
Conversation
`(1).#x` printed as `1.#x`, which does not parse: the lexer reads `1.` as the number. The arm that prints `a.b` already writes `1 .b` for this case. The arm that prints `a.#b` did not have the check. Both arms now call `print_member_dot`, which holds the check.
|
Status. The fix is in this PR (#47722). I reproduced the fault on a release build of main (3f3a9ff) with this file: class A { #x = 1; static t() { try { return (1).#x; } catch (e) { return e.name; } } }
console.log(A.t());
A second fault came from the runtime transpiler cache. I reproduced it with release builds and one cache folder, 10 runs for each pair:
CI. No failure in build #136821 (commit d99ed92) is from this PR. The build is complete.
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Member-dot printing and regression coverage src/js_printer/lib.rs, test/bundler/transpiler/transpiler.test.js, test/bundler/bundler_minify.test.ts |
The dot-property and private-index paths use print_member_dot to handle spacing after numeric literals. Tests cover printer output, minification, substitutions, and runtime TypeError results. |
Transpiler cache versioning src/jsc/RuntimeTranspilerCache.rs, test/cli/run/transpiler-cache.test.ts |
The cache version changes from 51 to 52. A regression test checks execution of cached output with the current and previous version fields. |
Suggested reviewers: jarred-sumner, dylan-conway
Merge Risk: 
d99ed
-
The change addresses numeric private-member printing and invalidates older cached output. No concrete merge-blocking issue is established; merge after normal checks pass.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main printer change: inserting a space between a number and the dot in private-name access. Description check The description is detailed and on-topic. It explains the problem, fix, verification steps, tests, cache-version change, tradeoffs, and known limitations. It does not use the exact required headings, … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is detailed and on-topic. It explains the problem, fix, verification steps, tests, cache-version change, tradeoffs, and known limitations. It does not use the exact required headings, but it provides the required information through equivalent sections.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
LGTM — a small, self-contained printer fix: the existing prev_num_end space check from the a.b arm is lifted into print_member_dot and reused in the a.#b arm, and the tests cover the literal, inlined-const, define, minified, and bundled const-enum paths plus the negative cases.
Beyond the inline nit, I also checked the other self.print(b".") sites in src/js_printer/lib.rs (namespace alias and CommonJS export paths) — they follow a printed symbol, never a number, so they are not affected; and the (-1).#x case is safe because negative numbers at postfix level are already wrapped in parentheses by print_number.
Extended reasoning...
The change touches only src/js_printer/lib.rs (extracting an existing four-line check into an inline helper and calling it from the EPrivateIdentifier branch of EIndex) plus two test files; no security-sensitive surface is involved. The refactor is behavior-preserving for the EDot arm and the EIndex arm's optional-chain path is untouched. The new tests assert exact printed output and a combined {stdout, stderr, exitCode} object from a spawned subprocess, and would fail on the merge base since the old output was a syntax error. No CODEOWNERS entry covers the changed files and the only reported finding is a serial-vs-concurrent test nit.
There was a problem hiding this comment.
LGTM — the follow-up commit addresses the earlier nit (the spawn test is now it.concurrent and named after bun -e), and the fix itself is a small, behavior-preserving extraction.
What was reviewed:
print_member_dotis byte-for-byte the oldEDotnon-optional branch; theEIndex/EPrivateIdentifierarm still skips it when?.was printed, so1?.#xis unchanged.prev_num_endis only recorded for non-negative and unwrapped-negative numbers inprint_number, so(-1).#x,1n.#x, and1[0].#xget no space; the remainingprint(b".")sites in the printer follow identifiers/namespace refs, not numbers, so no sibling site is missed.- Tests cover literal,
1 .,1.., call, assignment, minifyWhitespace,inline,define, inlinedconst enumunder full minify, plus the negative cases, and the spawn test drains all three pipes withPromise.all.
Extended reasoning...
The change touches one function pair in src/js_printer/lib.rs (12 lines net), extracting the existing number-then-dot guard into an always-inlined helper and calling it from the private-name member arm, plus three new tests in test/bundler. It touches no security-sensitive surface. The refactor is verified behavior-preserving by reading both arms and the two prev_num_end assignment sites, no CODEOWNERS entry covers the changed files, the only prior feedback (my own nit) was addressed by the second commit, and the bug hunt ran to a dry streak with no findings.
The printer now writes `1 .#x` for a private name read off a number. An entry that version 51 wrote for such a file holds `1.#x`. A build with the printer fix still loaded that entry and failed with the same SyntaxError. The tests also cover a folded constant and a folded `?.` in front of a private name.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — the EIndex private-name arm now reuses the exact prev_num_end check the EDot arm already had, and the follow-up commits add the cache version bump plus a test for it.
What was reviewed:
print_member_dot()is byte-for-byte the former inline EDot code; the remainingprint(b".")sites in the printer (namespace alias, CommonJS export key) always follow a symbol, never a number, so they are correctly left alone.- Optional-chain paths (
1?.x,1?.#x) stay unspaced, which is valid;(-1).#xgoes through the parenthesized negative path that does not setprev_num_end. - The transpiler-cache test's byte offsets (30/38/46) and wyhash seed match
Metadata::encodeandSEEDinRuntimeTranspilerCache.rs; the 5 KiB fixture clearsMINIMUM_CACHE_SIZEand the cache dir holds one file in both debug and release builds. - The serial-spawn nit from the previous review is addressed (
it.concurrent, title now saysbun -e).
Extended reasoning...
The diff touches src/js_printer/lib.rs (a 10-line helper extraction used at the EDot and EIndex-private-name arms), bumps EXPECTED_VERSION 51 to 52 in src/jsc/RuntimeTranspilerCache.rs, and adds tests in three files covering literal, inlined-constant, define, folded, optional-chain, and negative cases plus a cache-rejection test. No security-sensitive surface is involved; the only cross-cutting concern is the on-disk cache format, and the version constant is bumped as REVIEW.md requires. The helper is behavior-identical to the code it replaced, the sibling .-printing sites were audited and provably never follow a number, and no CODEOWNERS entry covers the changed files, which decided approve over defer.
Problem
bun runstops withSyntaxError: Unexpected private name #x. Expected a ';' following a return statement.on a valid file that reads(1).#x. Node loads it.bun buildoutput has the same fault.1.#x, and1.is the number. Thea.#barm prints the.with no check (src/js_printer/lib.rs:3760). Thea.barm has it (:3690).Fix
print_member_dot. After a number literal it prints a space first:1 .#x.--definevalue.1.#x.test/bundler/transpiler/transpiler.test.js,test/bundler/bundler_minify.test.ts,test/cli/run/transpiler-cache.test.ts. The 4 new tests fail on the merge base. Other suites: Notes.Background
(1).#xis a member access on a number.prev_num_endis the output position after the last number literal.bun runuses an entry only if its version isEXPECTED_VERSION.Downsides
1.5 .#x), as.xdoes, though1.5.#xparses..#nameruns 3 more instructions, and.namenone. Releasebun(linux-x64):sizetext +512 bytes.Notes
Source. #47712 names this fault: number 6 in its list of faults of main. No issue exists for it.
Repro. Node prints
TypeErrorfor both files.Release builds of the merge base (3f3a9ff) and of this PR:
a.jsbun a.jsSyntaxError: Unexpected private name #x.TypeErrora.jsbun build --no-bundle --minify-syntax, then runerror: Expected ";" but found "#x"TypeErrora.jsbun build, then runerror: Expected ";" but found "#x"TypeErrora.jsbun build --minify, then runerror: Expected ";" but found "#t"TypeErrorb.jsbun b.jsSyntaxError: Unexpected private name #x.TypeErrorb.jsbun build --no-bundle --minify-syntax, then runSyntaxError: Unexpected private name #x.TypeErrorb.jsbun build, then runTypeErrorTypeErrorb.jsbun build --minify, then runerror: Expected ";" but found "#t"TypeErrorThe control
(1).xprintsundefinedin every mode on both builds.Other forms.
1 .#xor1..#xfailed in the same way, because the printer prints what it parsed.(1 + 1).#x: where the sum is folded (bun run,--minify-syntax), the merge base printed2.#x.(1)?.#x: where the?.after a number is dropped (bun run,--minify-syntax), the merge base printed1.#x. Where the printer keeps the?., the output is1?.#xon both builds.const enummember from another file is inlined by the printer.E.One.#xprinted1.#tin a minified bundle. Without--minifyit prints1 /* One */.#x, which parses.--define CFG.n=10,CFG.n.#xprinted10.#x.1n.#x,(-1).#x,1[0].#x.--target=bun. Every cell isTypeErroron this PR. 16 of the 22 shapes fail in some cell on the merge base.The transpiler cache.
bun runstores the printed output of a file of 4 KiB or more. The merge base stores the entry for a file that reads(1).#x, though the file then fails to load. Release builds, one cache folder, 10 runs for each pair:SyntaxError: Unexpected private name #x.in 10 of 10TypeErrorin 10 of 10TypeErrorin 10 of 10The new test in
transpiler-cache.test.tsgives a fresh entry the output1.#x. It first shows that the entry is used under the current version. Then it sets the version of the entry to 51 and expectsTypeError. On a build with the printer fix and version 51, that last step fails with theSyntaxError.esbuild. On
main, theEPrivateIdentifiercase ofEIndexininternal/js_printer/js_printer.goprints the.with no check. esbuild 0.25.1 printsreturn 1.#x;fora.js.Measurements. Release builds (
--profile=release, LTO, linux-x64) in one build directory: the merge base, then this PR.size bun: text 140,288,644 to 140,289,156 (+512 bytes).dataandbssare equal. The file is 140,478,752 bytes in both builds. The version change adds no byte.nm -S:print_exprhas five instantiations. They grow by 34, 36, 39, 34 and 434 bytes. The last one is the printer for Bun with source maps. It has 32 more instructions (3,472 to 3,504). The other bytes are from the register allocator:selfmoved fromr13tor12, and each operand withr12as its base is one byte longer.objdump -d: thea.barm has the same four instructions for the check in both builds (two loads,cmp,jne). Thea.#barm now has them too. One of the two loads was there before, so the arm runs 3 more instructions.#[inline], the release build keptprint_member_dotout of line: 129 bytes in each instantiation, and a call for each printed.name. So the helper is#[inline(always)].perfandvalgrindare not in the build container. The instruction counts are from the disassembly, not from a run.Open PRs that touch the same code. #42448 changes the flags that the
EIndexarm passes to its target. #41156 changes whenprint_numberrecordsprev_num_end. Both keepprev_num_end, and the helper reads it. Other open PRs also add aVersion 52line toRuntimeTranspilerCache.rs. The one that merges later takes the next number.Suites, debug build.
test/cli/run/transpiler-cache.test.ts(35 pass, 0 fail),test/bundler/transpiler/transpiler.test.js(260 pass, 0 fail),test/bundler/bundler_minify.test.ts(68 pass, 0 fail),test/regression/issue/30887.test.tsand28159.test.ts(6 pass, 0 fail).test/bundler/bundler_edgecase.test.ts(197 pass, 0 fail),test/bundler/esbuild/default.test.ts,dce.test.ts,ts.test.ts(301 pass, 0 fail),test/bundler/transpiler/es-decorators.test.ts,decorators.test.ts,runtime-transpiler.test.ts,property.test.ts(470 pass, 0 fail).no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/run/transpiler-cache.test.ts