js_parser: keep legacy-decorated static field initializers inside the class body - #38953
js_parser: keep legacy-decorated static field initializers inside the class body#38953robobun wants to merge 2 commits into
Conversation
… class body
With experimentalDecorators, a decorated static field with an initializer
was lowered to an `A.x = init` statement after the class, so `this` in the
initializer was the enclosing scope's `this` and `super.x` did not parse at
all. Emit `static { this.x = init; }` at the field's position instead,
which is what tsc emits for useDefineForClassFields: false. This also runs
the initializer in source order with the other static members, as tsc does.
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
|
Status: fix and tests pushed, self-review addressed.
|
There was a problem hiding this comment.
LGTM — decorated static field initializers now stay in the class body as static { this.x = ... }, matching tsc's experimentalDecorators output.
What was reviewed:
- The
static_membersrelocation is fully removed and replaced with an in-place static block;this/superbinding,[[Set]]semantics, and initializer-before-decorator ordering are all preserved. - Checked that the removed
record_usage(class_ref)is correct to drop — the static path no longer emits a class-name reference. - Verified the existing
decorators randomtest's static-field assertions (S.j,S[h],S[q]) still hold under the new emission order. - New tests cover
this,super(call/getter/computed/assign/arrow), source order, computed keys, anonexport default class, and an output snapshot.
Extended reasoning...
Overview
This PR fixes the legacy (experimentalDecorators) TypeScript decorator lowering in src/js_parser/p.rs: decorated static fields with initializers were being hoisted out of the class body as ClassName.y = <init>; statements after the class, which broke super (SyntaxError) and this (wrong receiver) in the initializer, and reordered them after undecorated static members. The fix keeps the field in the class body as static { this.y = <init>; }, reusing the existing make_static_block helper (bumped to pub(crate)). The static_members vec and its post-class splice are deleted; instance-field lowering is unchanged.
Security risks
None. This is a transpiler output-shape change with no untrusted-input parsing, no allocation-size arithmetic, and no FFI/syscall surface.
Level of scrutiny
Medium-high — lower_class is on every TS class's path — but the change is ~30 net lines, localized to one branch of one loop, and the semantic argument is airtight: a static block runs with exactly the environment a static field initializer has (this = class, [[HomeObject]] = class, source-order among static elements), and this.y = ... preserves the [[Set]] semantics the lowering intentionally uses. The relative order of "static assignment → __legacyDecorateClassTS" is preserved (blocks run during class definition; decorator calls remain after the class statement), so property decorators that install accessors on the constructor still see the initializer via [[Set]] — I traced this through the decorators random test's S.j / S[h] assertions and they hold. The removed record_usage(class_ref) is correct because the static branch no longer emits an E::Identifier for the class name.
Other factors
The output shape is byte-for-byte what tsc 5.9 (--experimentalDecorators --target es2022 --useDefineForClassFields false) and esbuild produce, so this converges on the reference implementation rather than inventing a new form. Test coverage is thorough for the size of the fix: this (identity, inherited/own reads, computed key, arrow-captured, function-expr non-capture), source order interleaved with plain statics and a real static block, every super form (call, getter with receiver check, computed member, arrow, assignment through super landing on the subclass, computed key), an anonymous export default class, and an inline snapshot of the emitted body plus a check that only __legacyDecorateClassTS calls follow the class. The PR description states all four new tests fail on released Bun and pass with the change, and that the wider decorator/bundler suites and NestJS/TypeGraphQL integration tests were re-run. No bugs were found by the bug-hunting system.
…outside the class body A computed key is evaluated in the scope enclosing the class, so moving it into a static block would change what `this`, `arguments` or `await` in the key refer to. Only fields whose key is a name or a literal move into a static block; the others keep the previous emission.
|
Updated 12:13 PM PT - Aug 15th, 2026
🔄 @robobun, the build for your commit |
Problem
experimentalDecorators: true, a decorated static field whose initializer usessupermakes the whole module fail to load:SyntaxError: super is not valid in this context.thisgets the enclosing scope'sthis(undefinedin ESM), so@dec static z = this.tagthrowsTypeError: undefined is not an object (evaluating 'this.tag'). tsc givesthisthe class andsuper.xthe parent'sxwith the class as receiver.lower_class(src/js_parser/p.rs, thestatic_memberslist) removes the field from the class body and emitsA.y = <initializer>;as a statement after the class. The initializer is printed verbatim, so itsthisandsupernow belong to whatever encloses the class (andsuperdoes not parse there at all).Fix
static y,static ["y"],static 1) now stays at its position in the class body asstatic { this.y = <initializer>; }, built withmake_static_blockfrom the standard-decorator lowering (madepub(crate)).thisis the class,[[HomeObject]]is the class so everysuperform works natively, strict code,argumentsforbidden), andthis.y = ...keeps the[[Set]]assignment the lowering already used. Nothing in the initializer is rewritten and no runtime helper is needed, so there is no list ofsuperforms to get wrong. It is also the output tsc emits forexperimentalDecorators+useDefineForClassFields: false, and esbuild's.__legacyDecorateClassTSis applied (decorators are still emitted after the class; the existingdecorators randomtest checks that a property decorator sees the assigned value), and instance fields are untouched.static [sym],static [this.k],static [await p]) keeps the previous emission. A computed key is evaluated in the scope enclosing the class, so printing it inside a static block would change whatthis,argumentsorawaitin the key mean (or fail to parse). Evaluating the key once, outside the class, is what js_parser: evaluate the computed key of a legacy-decorated member once #38142 adds; once the key is a hoisted temporary these fields can take the static block too. Until then their initializers keep the old behaviour (this/superin them are still wrong), unchanged by this PR. Thestatic_memberslist inlower_classstays for them. One consequence of the split: in a class that mixes the two kinds, a decorated static with a non-literal computed key now runs after its name-keyed decorated siblings, even if it precedes them in the source (before, all decorated statics ran after the class in source order; tsc runs everything in source order). Hoisting the key resolves this as well.A_1aliasing, which makes all of them see the replacement, is not implemented in either of Bun's lowerings and is independent of this change.test/bundler/transpiler/decorators.test.ts, newdecorated static field initializersblock:thisforms (identity, inherited and own reads, literal and numeric keys, arrow capture, afunctionexpression keeping its ownthis), the combination with a class decorator, a non-literal computed key using the enclosingthisandarguments(pins the unchanged path), source order against plain statics and a static block, everysuperform in a spawned fixture (call with receiver check, getter, computed member, arrow, assignment throughsuperlanding on the subclass, literal computed key, anonymousexport default class), and an inline snapshot of the emitted code showing both emissions. Five of the six fail on the released Bun (TypeError, wrong order,SyntaxError, missing static blocks); the computed-key test passes before and after and exists to keep that path unchanged.decorators.test.ts,decorator-metadata.test.ts,ts-use-define-for-class-fields.test.ts,es-decorators*.test.ts,bundler_decorator_metadata.test.ts,bundler/esbuild/ts.test.ts,transpiler.test.js,bundler_edgecase.test.ts, the nest and typegraphql integration tests; the repro also runs correctly afterbun build,--minifyand--format=cjsunder node.lower_decorators.rs. That lowering has to relocate (standard semantics run static initializers after the decorators) and therefore rewritesthis/super; the legacy lowering runs initializers before decorators, which is why staying in the class body is available here.Background
experimentalDecorators) lowering keeps decorated fields as assignments rather than class fields so that a property decorator which installs an accessor on the constructor or prototype is hit by the initializer ([[Set]]) instead of being shadowed by an own data property ([[Define]]). Instance fields becomethis.x = initin the constructor; static fields are the subject of this PR.static { ... }) runs during class definition, at its position among the other static members, withthisbound to the class andsuper.xresolving through the class's parent: the same environment a static field initializer is evaluated in.static [expr] = ...) is different:expris evaluated in the scope that contains the class, before any static initializer runs, so it may legitimately use that scope'sthis,argumentsorawait.__legacyDecorateClassTSis Bun's runtime equivalent of tsc's__decorate; it is called once per decorated member after the class statement, so member initializers observe the undecorated class in both the old and the new output.Before / after for the repro
Before (
bun build --no-bundle), which fails to load withSyntaxError: super is not valid in this context.:After, printing
shi base-taglike tsc:tsc 5.9,
--experimentalDecorators --target es2022 --useDefineForClassFields false, for the same input:Evaluation order of a mixed class (
static a, decoratedstatic b, a static block, decoratedstatic c,static d): tsc runsa, b, block, c, d; Bun rana, block, d, b, cand now matches tsc.Computed keys: what is and is not changed
is emitted as
The two computed-key fields are emitted exactly as before this PR (same keys defined and decorated as on the released Bun);
plaingets the fix. The first revision of this PR put computed keys inside the block as well, which evaluated them in the wrong scope; that is what thekeep a non-literal computed key in the enclosing scopetest pins.