Repository navigation
Conversation
`x = (a,)` ran, and the printer wrote it as `x = a`. Node, JavaScriptCore and tsc refuse it. `parse_paren_expr` now records the `)` that follows a comma and reports `Unexpected )` there when the list is a comma expression. This is the error that `b || (a,)` already gets. Arrow function parameters and the arguments of `async(...)` keep their trailing comma. esbuild accepts `(a,)`, so the parser departs from esbuild here on purpose. The runtime transpiler cache version is 44, so that an entry written for a file with `(a,)` is not served.
|
Status: fix is in this PR (#47139). The diff is green and ready for review. How it was reproduced // comma.js
var a = 1;
var x = (a,);
console.log("RAN", x);
CI No failure comes from this diff.
|
|
Updated 6:27 AM PT - Oct 28th, 2026
❌ @robobun, your commit 106ca44 has 4 failures in
🧪 To try this PR locally: bunx bun-pr 47139That installs a local version of the PR into your bun-47139 --bun |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Track and reject expression trailing commas src/js_parser/parse/mod.rs, src/jsc/RuntimeTranspilerCache.rs |
The parser records the closing-parenthesis location after a comma and reports Unexpected ) for non-arrow expressions. The runtime transpiler cache format version changes from 43 to 44. |
Validate parser and runtime behavior test/bundler/transpiler/transpiler.test.js |
Tests cover expression-list errors and locations, valid arrow parameters and call arguments, TypeScript cases, error ordering across transformation and scanning APIs, JavaScript parsing comparisons, and Bun runtime behavior. |
Possibly related PRs
- oven-sh/bun#46209: Both PRs change
parse_paren_exprdecisions for TypeScript parenthesized syntax and preserve arrow-function parsing paths.
Suggested reviewers: jarred-sumner
Priority:
Merge Risk: 
106ca
-
Merge Risk: ⚪ Minimal · up to
106caThe change is ready to merge subject to normal checks. It intentionally loses support for a documented TypeScript conditional-arrow case.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely identifies the main parser change: rejecting trailing commas in parenthesized expressions. Description check The description explains the problem, implementation, preserved valid cases, compatibility impact, cache update, testing, and known limitations. It does not use the template headings exactly, but it p… 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 explains the problem, implementation, preserved valid cases, compatibility impact, cache update, testing, and known limitations. It does not use the template headings exactly, but it provides the required information and is complete.
-
- 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.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
With `replMode`, `{a: 1},` is parsed as `({a: 1},)`. The position of an
error is a position in that text, for each error in such an input. The
test for the trailing comma now checks the message only.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The comment at the trailing comma check and the cache version note had two lines each. The second line of each is gone. No code changes.
Problem
var x = (a,);runs and prints asx = a. Node, JavaScriptCore, tsc andbun lintrefuse it:SyntaxError: Unexpected token ')'. A parser review found it, not a user.parse_paren_expr(src/js_parser/parse/mod.rs:455) accepts a comma before), for arrow parameters andasync(...)arguments. Its comma-expression exit does not check for it.Fix
)after a comma. The comma-expression exit reportsUnexpected )there and stops, asb || (a,)already does.(a,) => a,async(a,)and(a,): T => astay valid. The transpiler cache version is 44.test/bundler/transpiler/transpiler.test.js(10 new tests, stock bun fails 4). Also the transpiler, repl and cache suites.Background
(a,). This PR is stricter on purpose, where node, JavaScriptCore and tsc refuse. If the parser must follow esbuild here, close it.=>at the comma: each valid(a, b,) => {}pays a second lex.Downsides
(a,)stops loading: 0 of 60,881 files in this repo, 1 of 50,678 in 537 npm packages that target bun.typescript.js, spread 0.02%) and 16 to 32 stack bytes per nesting level.c ? (a,): T => (d) : e => fis an error now. main prints another program for it, also without the comma.Notes
Where it came from and what it waits for
x = (a,): no PR". The loop is in the tree since 97cb54d (2021-05-02).x = aforx = (a,), andparseParenExpron esbuild's main branch (0.28.2) has the same loop. esbuild refuses the same list after an operator:x = b || (a,)givesUnexpected ")". So does bun, on main.bun lintrefusesvar x = (a,);withParsing error: Unexpected token )at the).bun file.jsruns it.async(...)a call before a:in TypeScript #45743, parser: a:after(b as B)or<B[]>(b)is not an arrow function return type (TypeScript) #46209 and parser: ana ? (b) : c => d : eattempt inside another attempt keeps its arrow function #46311 change this function for valid TypeScript. They matter more than this PR.The check
close_paren_after_commais a local ofparse_paren_expr, next tocomma_after_spread. The loop sets it and leaves when a comma is directly before).log_expr_errorsand after theUnexpected "..."check. The arrow function exit and theasync(...)exit do not.Unexpected )with the range of the), and the parser stops.x = (L)andx = 0 || (L)give the same message, length and count for 26 of 26 lists, 5 columns apart.x = ({ a = 1 },)givesUnexpected "=", thenUnexpected ), asx = b || ({ a = 1 },)does on main.x = (...a,)andx = (a: T,)stop at their own error.Bun.TranspilertransformSync,transform,scanandscanImports,bun file.js,bun buildand the REPL use this parser.bun check- a TypeScript type checker builtin to Bun #44361 recorded the same fact in this function for its tolerant mode. Addbun lintandbun format#44805 removed that mode.REPL
The REPL puts
( )around an input that starts with{. When that does not parse, it gives the input to JavaScriptCore as it is written. AreplModerow in the new tests holds the first step. WithreplMode, the position of an error is a position in the text with the( ), for each error in such an input. That is so on main too.bun repl -p '{ a: 1 },'SyntaxError: Unexpected token ',', exit 1bun repl -p '(1, 2,)'2, exit 0SyntaxError: Unexpected token ')', exit 1TypeScript between
?and:In TypeScript a
:after)can start a return type. For a body in parentheses, the parser takes a: e =>that follows for the return type of that body. Then the:of the conditional is gone. esbuild 0.21.5 does the same: it refusesx = c ? (a) => (b) : e => f..tsinputx = c ? (a) => (b) : e => fExpected ":" but found ";"x = c ? (a): T => (d) : e => fc ? a : (T) => (d) => fx = c ? async (a,): T => (d) : e => fc ? async(a) : (T) => (d) => fx = c ? (a,): T => (d) : e => fc ? a : (T) => (d) => fUnexpected )at the first)x = c ? (a, b,): T => { return f(a b) } : dbExpected ")" but found "b"Unexpected )at the first)x = c ? (a,) : e => f)c ? a : (e) => fUnexpected )at the)c ? HEAD: T => BODY : ELSE(6 heads, 5 return types, 20 bodies, 14 else branches, 3 places), 23,220 of them valid for tsc 6.0.2. main prints 20,700 as tsc reads them, and each of those prints the same with this PR. main prints 432 as another program: 288 have a head with a trailing comma and are an error now, 144 have no such comma and stay. 2,088 are an error on both builds. For 1,152 of them the error is nowUnexpected ).it.failingtests intranspiler.test.jshold the valid forms with the output of tsc.: T =>follows. It changes no verdict, only messages.x = c ? (a,) : e => fwould then reportExpected ":"at the end, where tsc points at the). Heads with typed parameters have the same fault on main and would keep it.Suites
commaOperatorWithoutOperand.ts(intest/cli/check/typescript-go/bundle.zst) expects TS1109 at the)of(ANY, );and four more lines. The transpiler accepted 5 of 5. It refuses 5 of 5 now, at the same column.Bun.Transpiler: 65 -> 0 disagree.Unexpected ). V8, JavaScriptCore and tsc refuse each of them. 4,394 are accepted by both builds and print the same text. 0 are accepted only by the PR.scanagrees withtransformSyncon each row. Agreement with V8: 5,350 -> 9,285 of 9,360. Agreement with tsc: 5,346 -> 8,211 of 8,750. This set has no arrow function body in parentheses before an arrow function. The table above has that family.(…,)and a second mistake. Examples:(a, b,) = 1(Invalid assignment target->Unexpected ), which is what node reports) andx = ({ a = 1 },)(one error -> two).Measurements
Release builds of main 99ee03e and of this diff on it, before the rebase.
valgrindandperfare not on the machine. Instruction counts come fromperf_event_open(instructions:u) and from a gdb single-step count.babel.min.jsis not on the machine, sotypescript.jstakes its place.parse_paren_exprcall, callees included, main vs PR (gdb single-step):(a, b) => {}js 14,182 -> 14,190, ts 18,195 -> 18,195.(a, b,) => {}js 14,262 -> 14,270, ts 18,250 -> 18,251.(a)js 2,755 -> 2,766, ts 2,798 -> 2,806.() => {}js 3,806 -> 3,808, ts 3,824 -> 3,824. Six parameters: js +12, ts -8.async(a, b): js +10, ts +33.P<false, false>19 -> 19 instructions,P<true, false>20 -> 19.)compares on that path: 1 -> 1 (gdb trace overllvm-objdump).transformSync(typescript.js)(9.1 MB, loader js): 18,944,973,753 -> 18,946,448,195 instructions:u (+0.008%), run-to-run spread 0.012% and 0.020% (10 interleaved runs). 300 xsrc/js/node/http2.ts: +0.003%, spread 0.04%. 100 xbun-install.test.ts: +0.019%, spread 0.05% and 0.07%.transformSync:x = (a, b) => {};js +0.104%, ts -0.000%.x = (a + b) * c;js +0.157%, ts +0.108%.parse_paren_exprx4: 3064/3050/6746/6758 -> 3165/3159/5694/5660 bytes. No new function..text-1,024 bytes,.rodata+0 (llvm-nm -S,llvm-size).parse_paren_exprframe: 0x188/0x198/0x418/0x408 -> 0x198/0x1a8/0x428/0x428. Deepest((((a))))on an 8 MiB stack: js 7,445 -> 7,338, ts 4,712 -> 4,627.(a,)that main cached: exit 0 -> exit 1 with the PR binary. With the version field of that entry set to 44 by hand, the PR binary runs the file, so the version change is needed. 12 cached modules: 12 cache files written on the first run after the upgrade, 0 on the second.node_modules, 0 of 60,881 (454 MB). 537 published packages that target bun, 1 of 50,678:src/__tests__/PieServer.test.js:1155of@websocketpie/server-bun1.2.6. It holds( actual, expected, 'message', );, an assertion that lost its function name.bun testreportsUnexpected )there until the name is back. String and template literals intest/that hold source text: 0 of 479, the new tests aside.Not changed
set m(a,) {}is accepted. V8 and tsc accept it. JavaScriptCore refuses it. A test holds the output.Unexpected "..."inx = (...a,)has no line or column. js_parser: reject parenthesized arrow parameters #41624 and parser: a:after(b as B)or<B[]>(b)is not an arrow function return type (TypeScript) #46209 correct that line.new vm.Script, as a test in js_parser: reject parenthesized arrow parameters #41624 does. The PR that lands second can share one helper.Verified
test/bundler/transpiler/transpiler.test.json the debug build: 268 pass, 0 fail. With stock bun 1.4.3-canary, 4 of the 10 new tests fail. One new block holds the forms that must stay valid, and fiveit.failingrows hold the TypeScript forms above.test/cli/run/transpiler-cache.test.ts(24),test/js/bun/repl/repl.test.ts(130),test/bundler/esbuild/ts.test.tsanddce.test.ts.test/bundler/transpiler/andtest/js/bun/transpiler/(37 files) on the debug build: 5,355 pass. The 32 failures are 5 s timeouts of tests that start processes, 31 of them injsx-production.test.ts. That file has the same result without this diff.test/bundler/bundler_edgecase.test.tsfail on the debug build with and without this diff:aot/EntryPointNamedLikeBuiltinIsALocalFile,aot/EntryPointIsExternalPackageandDeepImportDiamondDAG.