Conversation
…t a statement The standard decorator lowering puts what follows the last instance field (accessor storage, extra initializers, brands) in the constructor, after a top-level `super();` statement. A derived constructor with no such statement (`const made = super();`, `if (x) super(); else super();`, an arrow that calls it, or a constructor that returns an object) got the code at the top, before `this` exists, and threw a ReferenceError on every construction. For that shape the lowering now adds one `#private` field after the last member and puts the code in its initializer. A field runs where `super()` returns. The name is one that no enclosing class declares, and it is registered in the class body scope so the minifier renames it.
|
Status Reproduced on main a22b2aa with a debug build: const dec = (v, ctx) => {};
class B {}
class A extends B { accessor p = 1; constructor() { const made = super(); } }
class C extends B { @dec m() {} constructor(x) { if (x) { super(); } else { super(); } } }
new A().p; new C(1);Both constructions throw Fix: #42663 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. WalkthroughChangesDecorator lowering now distinguishes base and derived constructors. Derived constructors without a usable top-level Decorator lowering
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The changed lowering covers the reported derived-constructor cases, with cache invalidation and relevant JavaScript, TypeScript, bundling, and minification coverage. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes transpiler output and adds per-P state that must stay in lockstep with visit_class, a human look from someone familiar with the lowering pipeline would still be worthwhile.
What was reviewed:
- Traced all three
lower_standard_decorators_implentry points (SClass, SExportDefault→SClass, EClass expr) — each is preceded byvisit_class, which unconditionally setsvisited_class_bodyafter visiting nested properties, so the.take().expect(...)invariant holds and nested classes cannot leave a stale scope for an outer class. - Checked
insert_after_fields: base-class and derived-with-super();-statement paths keep their previous output; only the derived-no-statement case falls through to the new#privatefield. new_private_namewalksscope.parentand checks bothmembersandgeneratedat eachClassBody, and records the new symbol inclass_body.generatedfor the minifier; the nested-class test exercises both collision sources. Cache version is bumped.
Extended reasoning...
Overview
This PR fixes standard-decorator lowering for derived classes whose constructor calls super() somewhere other than a top-level statement. insert_after_super is renamed insert_after_fields, now takes is_derived, and returns false when a derived constructor has no super(); statement; the caller then appends a synthetic #private field whose initializer carries the post-last-field effects, so they run when super() returns wherever it is written. A new P::new_private_name picks a #_/#_2/… name unused by any enclosing class-body scope and records it in generated. visit_class stashes the class-body scope into a new P::visited_class_body just before popping it, and lower_standard_decorators_impl .take()s it. RuntimeTranspilerCache::EXPECTED_VERSION is bumped 32→33. Tests add a nested-class name-collision case (run and --minify bundled) and an extraSections matrix covering accessor/#field/#accessor/field/method/#method with super() written as const made = super(), in both branches of an if, inside an arrow, void super(), never called, and next to an existing #_.
Security risks
None identified. This is transpiler output shaping for a language feature; there is no untrusted input parsing beyond what the JS parser already handles, no I/O, no auth/crypto, and no network. The change cannot be reached without the user opting into standard-decorator lowering on a derived class of this specific shape.
Level of scrutiny
Moderate-to-high. Transpiler output changes affect every user who compiles matching source, and a wrong emit here would surface as a runtime ReferenceError or a #private name collision. The new P::visited_class_body field is per-operation state on a reusable object, so I traced every caller: visit_class sets it unconditionally at line 1460 (after visiting all properties, so nested-class recursion cannot leave a stale inner scope for the outer class), and all three lower_standard_decorators_* entry points are immediately preceded by a visit_class call with no intervening class visits. visit_class has no early return between function entry and the assignment. Stale values from classes that do not lower decorators are harmless because the next visit_class overwrites before any .take().
Other factors
Test coverage is thorough: the fixture matrix covers six member kinds crossed with six super() placements, plus the negative case (constructor returns an object without calling super()), and the standalone test exercises name collision against both an enclosing class's members and the minifier path. The cache version bump satisfies the REVIEW.md rule for serialized-output changes. CODEOWNERS does not cover any changed path. No issues were found by the bug-hunting pass (dry_streak). I am deferring rather than approving because this is a non-trivial control-flow change in the lowering pipeline with new mutable state on P; a maintainer familiar with the parser should confirm the visited_class_body handoff and the scope-walk in new_private_name match their model of scope lifetimes.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Updated 5:57 PM PT - Sep 13th, 2026
✅ @robobun, your commit b90a7e0fc90748ae9d4ebcd37327d8b38ecb49c1 passed in 🧪 To try this PR locally: bunx bun-pr 42663That installs a local version of the PR into your bun-42663 --bun |
This branch has not been deployed
2 participants
Problem
accessorthrows on construction when its constructor callssuper()anywhere but in a statement of its own, for exampleconst made = super();. The error:ReferenceError: 'super()' must be called in derived constructor before accessing |this| or returning non-object.insert_after_super(src/js_parser/lower/lower_decorators.rs:106) looks for a top-levelsuper();statement. With none, it puts the code at the top, beforethisexists.Fix
insert_after_fieldsreports a derived constructor with nosuper();statement. The lowering then adds one#privatefield after the last member, with the code in its initializer:#_ = (effects, undefined). A field runs wheresuper()returns. Every other shape keeps its output.new_private_namepicks a name that no enclosing class declares, in the class body scope thatvisit_classleaves inP::visited_class_body. The minifier renames it with the others.test/bundler/transpiler/es-decorators.test.ts(422 pass, the 4 new tests fail on main), and the other decorator suites.Background
super()returns. Before that,thisis in its temporal dead zone.#privatename is a symbol of the class body scope. The minifier names symbols per scope.Notes
super();statement still get the code in the constructor, and no extra field.if (x) { super(); } else { super(); },(() => super())(),void super();, and a constructor that returns an object and never callssuper().class B {} class A extends B { accessor p = 1; constructor() { const made = super(); } } new A().p. Main a22b2aa throws the ReferenceError above. This branch prints1. The same for@dec x = 1,@dec #x = 1,@dec accessor #x = 1,@dec m() {}and@dec #m() {}as the only member.class C extends B { @dec #m() {} constructor() { if (x) { super(); } else { super(); } } }: the constructor is untouched and the class ends with#_ = (__privateAdd(this, _m), __runInitializers(_init, 5, this), undefined);.TypeErrorwhen the field is added again. A class with anaccessoror a lowered#privatemember already behaves that way.#_, or#_2,#_3when the class or an enclosing class declares the shorter ones. The test coversbun runandbun build --minify.RuntimeTranspilerCacheversion 32 to 33.es-decorators,es-decorators-esbuild,decorators,decorator-metadata,ts-use-define-for-class-fields,bundler_edgecase,transpiler-cache,regression/issue/{27526,27575}..js, as.tsand throughbun build. It wraps each construction intry/catch, so on main only the 4 new tests fail and the other 418 pass.super();statement, an initializer that lands in the constructor still sees the constructor'snew.target(js_parser: substitute undefined for new.target in class field initializers and static blocks #42653 covers that) and its parameters. That part is on the branchrobobun/1f87cd24/native-accessor-storage, which makes accessor storage a native#privatefield. This PR is the part of that branch that is needed whichever way the storage goes.[human-review] gate passed · iteration 0 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 0
evidence per changed file