Repository navigation
Conversation
…d from s_enum
`const base = 100; enum Status { Ok = base, Created }` gave
`Status.Created === undefined`. tsc gives 101 (TypeScript 5.0 constant
expressions).
When the visit leaves an enum initializer that is not a literal, `s_enum`
checks whether it is a numeric constant expression that names `const`s
declared with a number. If so, it writes the numbers in and visits the
expression again, so the visitor folds it as it folds the same
initializer with the numbers written in place. Any other initializer is
left as it is.
A name resolves from `const_values` (a const of an enclosing scope whose
statement is visited) or from the statements before the enum in its own
list. The two enum pre-visit loops hand those statements over for each
enum. They are read the first time an initializer names one of their
consts, and each statement list keeps its own table.
The two enum pre-visit loops are now compiled only into the TypeScript
parser.
A folded member can be a negative number. The printer now puts
parentheses around an inlined enum value on the left of `**`, as in
#36133, because `-5 ** x` is a SyntaxError.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 12 seconds. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Preceding-statement context src/js_parser/p.rs, src/js_parser/parse/parse_entry.rs, src/js_parser/visit/mod.rs |
Parser state now holds earlier statements and cached numeric values. TypeScript enum preprocessing passes preceding statements into relevant enum visits. |
Numeric constant resolution and folding src/js_parser/visit/visit_stmt.rs, test/bundler/transpiler/transpiler.test.js |
The visitor resolves eligible earlier numeric const declarations and supported numeric expressions in enum initializers. Tests cover folding, auto-numbering, and scope and eligibility boundaries. |
Exponentiation printing and bundler validation src/js_printer/lib.rs, src/jsc/RuntimeTranspilerCache.rs, test/bundler/esbuild/ts.test.ts |
Printer precedence handling covers negative inlined enum values on the left of exponentiation. The transpiler cache version changes, and bundler tests cover cross-module output and minification. |
Suggested reviewers: jarred-sumner, alii
Priority:
Severity of issue fixed: Medium
Merge Risk: 
08f1a
-
No identified issue currently prevents merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the main change: folding a number const referenced by an enum initializer, triggered from s_enum. It matches the changeset, which adds fold_ts_enum_numbers, substitution of const value… Description check The description follows the template with both required sections, 'What does this PR do?' and 'How did you verify your code works?'. The first section gives the problem, the fix, and the limits. The s… Linked Issues check The PR satisfies [ #19581]. The addedts/EnumConstInitializersMinify/apiandts/EnumConstInitializersMinify/clitests use the issue example. They verify runtime output25and exact minified outpu…Out of Scope Changes check The changes stay within the enum-folding scope of [ #19581]. The numeric-constant handling, cross-module enum tests, and exponentiation parenthesization support correct enum folding and safe generated …Full details: Title check
-
Explanation
The title names the main change: folding a number const referenced by an enum initializer, triggered from s_enum. It matches the changeset, which adds fold_ts_enum_numbers, substitution of const values, and a second visit in s_enum. The title is specific and uses the repository's module-prefix style, but the term 'on demand' is omitted and the lowercase 'const' and 'number' wording is dense.
Full details: Description check
-
Explanation
The description follows the template with both required sections, 'What does this PR do?' and 'How did you verify your code works?'. The first section gives the problem, the fix, and the limits. The second section names the new test files, the 20 failing stock cases, and the performance and output checks. It is detailed and fully covers the template's required information.
Full details: Linked Issues check
-
Explanation
The PR satisfies [
#19581]. The addedts/EnumConstInitializersMinify/apiandts/EnumConstInitializersMinify/clitests use the issue example. They verify runtime output25and exact minified outputconsole.log(25);. The implementation substitutes numericconstvalues in enum initializers and revisits the expression, which enables the requiredconst enumfolding.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within the enum-folding scope of [
#19581]. The numeric-constant handling, cross-module enum tests, and exponentiation parenthesization support correct enum folding and safe generated syntax. The parser-state changes, cache version update, and automated tests support this implementation. No unrelated change is demonstrated.
-
- 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.
|
Updated 2:37 AM PT - Oct 24th, 2026
✅ @robobun, your commit 014bf949ae26319ff81fd084ef426156d85cf977 passed in 🧪 To try this PR locally: bunx bun-pr 46693That installs a local version of the PR into your bun-46693 --bun |
|
Reproduced on main (5c6d4e2) and on 1.4.3: // a.ts
const base = 100;
enum Status { Ok = base, Created, Gone = base * 2, After }
console.log(JSON.stringify([Status.Ok, Status.Created ?? "undefined", Status.Gone, Status.After ?? "undefined"]));
PR: #46693 CI on 014bf94 (build 131749): 179 of 181 jobs pass. The 2 open jobs are the macOS x64 test lane. They expired twice with no agent, not started, and are queued again. Five tests in other areas failed once and passed on a retry ( |
![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.
Beyond the inline findings, I also checked the BackRef<[Stmt]> taken under split_at_mut in visit_stmts — it is re-pointed at the fresh earlier slice on every with_ts_enum_siblings call and only read inside that closure, so the cursor-persisted numbers table never dereferences a stale slice. The two-pass check-then-replace in substitute_ts_enum_numbers is all-or-nothing, so a mixed const/inlined-enum initializer either folds to a number or is left untouched; and the ignore_usage on replaced refs does not remove the sibling const declaration outside minify, matching the base branch.
Extended reasoning...
The change adds TS enum-initializer const folding to the parser (new per-list sibling state on P, both pre-visit loops, a cold fold path in s_enum), a ** parenthesization fix in the printer, and a transpiler cache version bump, with tests in both bundler test files. It touches no security-sensitive surface. Three confirmed findings are posted inline, so a human look is already signalled; this note only records what else was examined and ruled out.
…uter list's consts An enum at index 0 of a statement list kept the siblings of the enum being visited around it. That is meant for a top-level enum, which the tree-shaking path visits as a list of one. A function body inside an enum initializer has its own list, so its first enum folded from the outer list while an enum one statement lower did not. Only an enum at module scope keeps the siblings of its caller now.
|
Pushed 08f1a19 for the review.
|
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.
Beyond the inline nit, I also checked the raw BackRef<[Stmt]> into the sibling slice: it is only dereferenced inside the with_ts_enum_siblings closure while swapped into P, both halves of split_at_mut are disjoint, and stmts cannot be appended to during the pre-visit because nearest_stmt_list is only pointed at the list after that loop — so it does not dangle, and the swap-back restores the caller's state on the success path. The second commit (i > 0 || current_scope != module_scope) covers the nested-enum-at-index-0 case raised earlier; a human look at the deliberate tsc/esbuild divergence (annotated consts folding) is still worthwhile.
Extended reasoning...
The change adds on-demand folding of same-list number consts into TypeScript enum initializers across src/js_parser/p.rs, parse_entry.rs, visit/mod.rs and visit/visit_stmt.rs, plus a ** parenthesization fix in src/js_printer/lib.rs and a RuntimeTranspilerCache version bump to 44, with 23 new tests. It touches no security-sensitive surface. It is not approvable automatically because it introduces new raw back-pointer state on the parser and a deliberate semantic divergence from esbuild and tsc that a maintainer should weigh; the lifetime of the back-pointer was verified by reading the pre-visit loop and the swap helper, and the second commit addressed the prior nested-enum finding.
There was a problem hiding this comment.
I reviewed the two follow-up commits and found no bugs; they resolve the points I raised on the earlier revisions (the nested index-0 enum now gets its own empty sibling table, and the --no-bundle test asserts the full output). Because this adds a new const-resolution path to the TypeScript enum lowering and the description names a competing design (#42004) that a maintainer must choose between, a human look is still worthwhile.
What was reviewed:
- Lifetime of the
BackRef<[Stmt]>inTsEnumSiblings: it is set and swapped out insidewith_ts_enum_siblings,stmtsis only read during the enum pre-visit, andPis left holding the caller's table on both the Ok and?paths. - The
i > 0 || current_scope != module_scopeguard: at module scope an index-0 enum is either the one-element list from the tree-shaking loop (siblings already set) or a true first statement (nothing earlier), so both branches are correct. ts_enum_sibling_numbermirrorsvisit_decls'sreplace_exportshandling (Replace substitutes, Delete skips, namespace members excluded); onlyKind::Constantsymbols with literal or-literalinitializers fold.- The
**printer change raisesleft_leveltoCallonly for EDot/EIndex whents_enumsis set, andprint_numberwraps negatives at that level, so non-enum dot expressions print unchanged.
Extended reasoning...
The change touches the TypeScript parser's enum pre-visit loops in src/js_parser/parse/parse_entry.rs and src/js_parser/visit/mod.rs, adds a cold fold path and sibling-const lookup in src/js_parser/visit/visit_stmt.rs, adjusts ** precedence in src/js_printer/lib.rs, and bumps the runtime transpiler cache version to 44, with 23 new test cases. It touches no security-sensitive surface. The follow-up commits since my last review addressed both of my open comments and the ruled-out concerns above found no defect. I deferred rather than approved because the parser change is non-trivial (a raw back-pointer held in P, two parallel loops that must stay in sync) and the PR itself states a maintainer must choose between this design and the open #42004.
import.meta.main node and the left operand of ** by level
#33447
Fixes #19581
Problem
const base = 100; enum Status { Ok = base, Created }givesStatus.Created === undefined, with no error. tsc gives 101.s_enum(src/js_parser/visit/visit_stmt.rs) keeps a value only for a literal initializer. Enums are visited before the rest of their list (feat(bundler): implement enum inlining / more constant folding #12144), so a siblingconsthas no value yet.Fix
s_enumcalls the coldfold_ts_enum_numbersfor an initializer that is not a literal after the visit. A numeric expression over numberconsts gets the numbers written in and one more visit.const_values(whatbun runinlines) or from the statements before the enum, read on first use.**(from js_printer: parenthesize inlined negative enum value on LHS of ** #36133).test/bundler/transpiler/transpiler.test.js(18 new cases),test/bundler/esbuild/ts.test.ts(5). Stock bun fails 20. Self-reviewed: 17 concerns raised, 16 addressed (Notes).Background
constwith a constant initializer and no type annotation (Enum unification and improvements microsoft/TypeScript#50528). esbuild skips that rule on purpose.const, a branch inhandle_identifier) and fix(transpiler): fold const enum members that reference const variables #27167 (a loop over all statements, refused in review). This starts at the enum initializer. js_parser: fold references to const variables in enum initializers #42004 is still open.Downsides
Pgrows 24 B, the binary 1,792 B.constwith a type annotation folds too. tsc leaves it computed.1 << 10andN.astay as before.Notes
Measurements. Release builds of main 5c6d4e2 and this branch, non-LTO, in one container.
valgrind,perf,bloatyandstraceare not installed there. Instruction counts are user-mode instructions fromperf_event_openaround a fixedBun.Transpilerloop (BUN_JSC_useJIT=0, per transpile = difference of two loop lengths, minimum of 5 runs). The noise column is the spread of those runs.react-dom.development.js(JavaScript, 1.0 MB)src/codegen/bindgen-emit.ts(no enum, 86 KB)src/js/node/http2.ts(one enum, 271 KB)fold_ts_enum_numbersis entered 0 times over the 3,834 tracked.ts/.tsxfiles of this repository under 5 option sets (gdb breakpoint count). Every new allocation is behind it.{},inline,treeShaking, both, andminify.syntax(19,170 outputs).llvm-nm -S): +2,039 B of text symbols. New:substitute_ts_enum_numbers1,882,HashMap<Ref, f64>1,319,with_ts_enum_siblings136,fold_ts_enum_numbers62.P<true,false>::visit_stmts+369,s_enum+48.P<false,false>::visit_stmts-709 and_parse::<false>-1,123, because the two enum pre-visit loops are now compiled only into the TypeScript parser.handle_identifieris 2,188 and 1,506 B in both builds.Against tsc and esbuild. 124 probe programs, each under
bun file.ts,bun build,bun build --no-bundle,Bun.TranspilerandBun.Transpilerwithinline, compared with the output ofts.transpileModule(6.0.2) run in node.constwith a type annotation (TS1061), 1 reads a member before the enum statement (TS2450), which main already inlines for an enum with literal initializers.constvariables is not followed both because esbuild's enum processing is done in an isolated module setting and because doing so would potentially require esbuild to use a type system, which it doesn't have." In Incorrect code for enums generated evanw/esbuild#4387 the type-dependent part is the annotation. A same-file const needs neither:ts.transpileModulefolds it withisolatedModuleson, andbun checkacceptsconst t: 101 = Status.Createdfor the reported enum. On TypeScript enum emit inconsistency microsoft/TypeScript#63275 the TypeScript team says the rule is meant to be one that a transpiler can apply from the syntax of the initializer.Not folded yet (tsc folds these, the output is unchanged from main):
constdeclared with a string, or with an expression (1 << 10,a + 1, an enum member)N.a,M.Z.Q, an exported const read inside its own namespace,declare const, aforhead constThe string case needs care with non-ASCII text: the visitor folds only 8-bit strings, and a second visit of a partly folded template prints wrong text. It is left for a follow-up with run-time value tests.
Where it differs from tsc on purpose. The parser drops a type annotation,
as const,satisfiesand!before the enum is visited, so such a const folds.bun runalready inlines a const like that when it is at the start of an enclosing scope. For a number this changes no program that tsc accepts, except one where a call reads the const before its declaration runs: tsc's output throws a ReferenceError there and the folded enum does not.History.
bun runprinted 101 up to 1.1.17 when nothing came before the const, because the const was inlined before the enum was visited. Below an import it printed 0. From 1.1.18 (#12144) it isundefinedin every layout.Self-review. 17 concerns, 16 addressed. The first draft also folded strings and templates, stored the value before the fold, kept one table for the file keyed by the enum, and used
NonNull::from(&*x). The review found wrong text for a template with a non-ASCII const, a quadratic case, a source-lint failure, and a load-time SyntaxError forE.A ** xwith a negative const. This version folds numbers only, folds before the value is stored, keeps one table per statement list behind one helper, usesBackRef, readsexports.replaceby name, and carries the printer fix. Not done: a value-returning evaluator in place of the second visit. It needs the numeric folds ofvisit_binary.rsin a shared function. The second visit here sees only numbers and operators that the visitor folds on numbers, and it does not setis_revisit_for_substitution(#45507 removes that flag).Related. #42004 fixes the same issue with another design and is still open. Only one of the two should merge, and a maintainer makes that choice. #42118 is stacked on #42004 and uses its evaluator, so it needs a new base if this one is chosen. #36133 has the same printer change with more tests for literal enums. #37263 still covers the run-time reverse key of a string member. Jarred-Sumner on #27167: "Do not add an extra loop over all the statements. Read over esbuild's implementation and understand what they do." The only statement walk here runs when an initializer names a const that
const_valuesdoes not hold.