js_parser: run lowered decorator effects after a super() that is not a statement

src/js_parser/lower/lower_decorators.rs

@@ -101,27 +101,33 @@ fn initializer_group(prop: &Property) -> Option<usize> {
     }
 }
 
-/// Puts `inserted` in `constructor` right after a top-level `super()`
-/// statement returns, which is after the last instance field.
-fn insert_after_super<'a>(
+/// Puts `inserted` after the last instance field: at the top of a base
+/// constructor, after the `super();` statement of a derived one, if it has one.
+fn insert_after_fields<'a>(
     constructor: &mut Property,
+    is_derived: bool,
     inserted: &[Stmt],
     arena: &'a bun_alloc::Arena,
-) {
+) -> bool {
     let func = match &mut constructor.value.as_mut().unwrap().data {
         js_ast::ExprData::EFunction(f) => &mut **f,
         _ => unreachable!(),
     };
     let body: &[Stmt] = func.func.body.stmts.slice();
-    let after_super = body
-        .iter()
-        .position(|stmt| stmt.is_super_call())
-        .map_or(0, |i| i + 1);
+    let after_fields = if is_derived {
+        match body.iter().position(|stmt| stmt.is_super_call()) {
+            Some(super_call) => super_call + 1,
+            None => return false,
+        }
+    } else {
+        0
+    };
     let mut stmts = BumpVec::<'a, Stmt>::with_capacity_in(body.len() + inserted.len(), arena);
-    stmts.extend_from_slice(&body[..after_super]);
+    stmts.extend_from_slice(&body[..after_fields]);
     stmts.extend_from_slice(inserted);
-    stmts.extend_from_slice(&body[after_super..]);
+    stmts.extend_from_slice(&body[after_fields..]);
     func.func.body.stmts = bun_ast::StoreSlice::from_bump(stmts);
+    true
 }
 
 // ── impl P ───────────────────────────────────────────────────────────────────
@@ -211,6 +217,42 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         b"_accessor_storage"
     }
 
+    /// A `#private` name for `class_body` that no class around it declares.
+    fn new_private_name(
+        &mut self,
+        mut class_body: js_ast::StoreRef<js_ast::Scope>,
+        base: &'a [u8],
+        kind: js_ast::symbol::Kind,
+    ) -> Ref {
+        let mut name = base;
+        let mut count = 1u32;
+        'unique: loop {
+            let mut scope = Some(class_body);
+            while let Some(current) = scope {
+                let declares = current.kind == js_ast::scope::Kind::ClassBody
+                    && (current.members.contains_key(name)
+                        || current.generated.slice().iter().any(|generated| {
+                            self.symbols[generated.inner_index() as usize]
+                                .original_name
+                                .slice()
+                                == name
+                        }));
+                if declares {
+                    count += 1;
+                    name = bun_alloc::arena_format!(in self.arena, "{}{}", bstr::BStr::new(base), count)
+                        .into_bump_str()
+                        .as_bytes();
+                    continue 'unique;
+                }
+                scope = current.parent;
+            }
+            break;
+        }
+        let ref_ = self.new_symbol(kind, name);
+        VecExt::append(&mut class_body.generated, ref_);
+        ref_
+    }
+
     // ── Private access rewriting ─────────────────────────
 
     fn private_get_expr(&mut self, obj: Expr, info: &PrivateLoweredInfo, l: bun_ast::Loc) -> Expr {
@@ -789,6 +831,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         let p = self;
         let bump = p.arena;
         let temps_before = p.temp_refs_to_declare.len();
+        let class_body = p
+            .visited_class_body
+            .take()
+            .expect("visit_class ran for this class");
 
         let class_decorators: ExprNodeList = bun_alloc::AstAlloc::take(&mut class.ts_decorators);
         let has_class_decorators = class_decorators.len_u32() > 0;
@@ -1247,14 +1293,30 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
 
         let mut new_constructor = None;
         if !constructor_effects.is_empty() {
+            let is_derived = class.extends.is_some();
             match constructor {
                 Some(constructor) => {
                     let stmts = p.effect_stmts(&constructor_effects, loc);
-                    insert_after_super(&mut members[constructor], &stmts, p.arena);
+                    if !insert_after_fields(&mut members[constructor], is_derived, &stmts, p.arena)
+                    {
+                        // No `super();` statement: a last field runs where `super()` returns.
+                        let host = p.new_private_name(
+                            class_body,
+                            b"#_",
+                            js_ast::symbol::Kind::PrivateField,
+                        );
+                        p.record_usage(host);
+                        constructor_effects.push(p.new_expr(E::Undefined {}, loc));
+                        members.push(Property {
+                            key: Some(p.new_expr(E::PrivateIdentifier { ref_: host }, loc)),
+                            initializer: Some(Expr::join_all_with_comma(&constructor_effects)),
+                            ..Default::default()
+                        });
+                    }
                 }
                 None => {
                     new_constructor =
-                        Some(p.new_constructor(class.extends.is_some(), &constructor_effects, loc));
+                        Some(p.new_constructor(is_derived, &constructor_effects, loc));
                 }
             }
         }

src/js_parser/p.rs

@@ -711,6 +711,8 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> {
     /// Name from assignment context for anonymous decorated class expressions.
     /// Set before visitExpr, consumed by lowerStandardDecoratorsImpl.
     pub(crate) decorator_class_name: Option<&'a [u8]>,
+    /// Set by `visit_class`, consumed by `lower_class_body` to declare `#private` names.
+    pub(crate) visited_class_body: Option<js_ast::StoreRef<Scope>>,
 }
 
 // `binding::ToExprWrapper` type-erases `*P` (which is generic over
@@ -9893,6 +9895,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             will_wrap_module_in_try_catch_for_using: false,
             nearest_stmt_list: None,
             decorator_class_name: None,
+            visited_class_body: None,
 
             jsx_transform,
 

src/js_parser/visit/mod.rs

@@ -1457,6 +1457,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 }
             }
 
+            self.visited_class_body = Some(self.current_scope);
+
             // manual restore for the block-level `defer`
             self.pop_scope();
             self.enclosing_class_keyword = old_enclosing_class_keyword;

src/jsc/RuntimeTranspilerCache.rs

@@ -60,7 +60,8 @@ bun_core::declare_scope!(cache, visible);
 /// Version 30: String enum members are stored flat, so folds no longer append onto an inlined member.
 /// Version 31: Standard decorator lowering temporaries have a per-file counter in their name (`_init$1`).
 /// Version 32: Standard decorator lowering keeps class members in place.
-const EXPECTED_VERSION: u32 = 32;
+/// Version 33: Standard decorator lowering does not write before a `super()` that is not a statement.
+const EXPECTED_VERSION: u32 = 33;
 
 /// Source files smaller than this are not written to / read from the on-disk
 /// transpiler cache. Originally 50 KiB, which excluded almost every file in a

test/bundler/transpiler/es-decorators.test.ts

@@ -1393,6 +1393,41 @@ describe("ES Decorators", () => {
       expect(exitCode).toBe(0);
     });
 
+    test.concurrent("the field added after the last one gets a #private name of its own", async () => {
+      // A derived constructor without a `super();` statement: what follows the
+      // last field rides in one more `#private` field. The class, and a class
+      // around it that the body refers to, already use the names it would take.
+      using dir = tempDir("es-dec-added-private-name", {
+        "entry.js": `
+          const dec = (v, ctx) => { ctx.addInitializer(function () { this.ran = true; }); };
+          class Base {}
+          class Outer {
+            #_ = "outer";
+            static make() {
+              return class extends Base {
+                #_2 = "own";
+                @dec x = "x";
+                constructor() { const made = super(); }
+                read(o) { return [o.#_, this.#_2, this.x, this.ran]; }
+              };
+            }
+          }
+          console.log(JSON.stringify(new (Outer.make())().read(new Outer())));
+        `,
+      });
+      const expected = '["outer","own","x",true]\n';
+      const ran = await runIn(String(dir), ["entry.js"]);
+      expect(filterStderr(ran.stderr)).toBe("");
+      expect(ran.stdout).toBe(expected);
+      expect(ran.exitCode).toBe(0);
+      const build = await runIn(String(dir), ["build", "--target=bun", "--minify", "entry.js", "--outfile=out.js"]);
+      expect({ stderr: filterStderr(build.stderr), exitCode: build.exitCode }).toEqual({ stderr: "", exitCode: 0 });
+      const minified = await runIn(String(dir), ["out.js"]);
+      expect(filterStderr(minified.stderr)).toBe("");
+      expect(minified.stdout).toBe(expected);
+      expect(minified.exitCode).toBe(0);
+    });
+
     test.concurrent("super resolves from the class as written when a class decorator replaces it", async () => {
       const { stdout, stderr, exitCode } = await runDecorator(`
         const dec = (v, ctx) => {};
@@ -2391,6 +2426,30 @@ const extraSections = `
   const desc = Object.getOwnPropertyDescriptor(N.prototype, "k");
   out.accessorStorage = [nn.a, N.a, nn.priv(), nn.k, mm.k, n, typeof desc.get, typeof desc.set, Outer.make().a];
 }
+
+// What follows the last instance field runs when \`super()\` returns,
+// wherever that call is written, and not at all when it is never called.
+{
+  const seen = [];
+  const see = (tag) => (seen.push(tag), tag);
+  const extra = (value, ctx) => { ctx.addInitializer(function () { seen.push("extra " + String(ctx.name) + ":" + (this instanceof Base3)); }); };
+  class Base3 {}
+  const classes = [
+    class extends Base3 { @extra accessor p = see("accessor"); constructor() { const made = super(); seen.push("body"); } },
+    class extends Base3 { accessor p = see("plain accessor"); constructor(x) { if (x) { super(); } else { super(); } seen.push("body"); } },
+    class extends Base3 { @extra #p = see("#field"); constructor() { (() => super())(); seen.push("body"); } },
+    class extends Base3 { @extra accessor #p = see("#accessor"); constructor() { void super(); seen.push("body"); } },
+    class extends Base3 { @extra p = see("field"); constructor() { const made = super(); seen.push("body"); } },
+    class extends Base3 { @extra m() {} constructor() { const made = super(); seen.push("body"); } },
+    class extends Base3 { @extra #m() {} constructor() { const made = super(); seen.push("body"); } },
+    class extends Base3 { @extra p = see("never"); constructor() { return { made: true }; } },
+    class extends Base3 { #_ = see("own #_"); @extra p = see("next to #_"); constructor() { const made = super(); seen.push(this.#_); } },
+  ];
+  for (const C of classes) {
+    try { new C(1); } catch (e) { seen.push(e.constructor.name); }
+  }
+  out.superNotAStatement = seen;
+}
 `;
 
 const extraExpected = {
@@ -2434,6 +2493,30 @@ const extraExpected = {
   accessorKeys: [11, 2, 3, 4],
   issue31921: ["a", 1, "a", "b", [5, "a", "block", "b"]],
   accessorStorage: [12, 13, 1, 14, 15, 2, "function", "function", 6],
+  superNotAStatement: [
+    "accessor",
+    "extra p:true",
+    "body",
+    "plain accessor",
+    "body",
+    "#field",
+    "extra #p:true",
+    "body",
+    "#accessor",
+    "extra #p:true",
+    "body",
+    "field",
+    "extra p:true",
+    "body",
+    "extra m:true",
+    "body",
+    "extra #m:true",
+    "body",
+    "own #_",
+    "next to #_",
+    "extra p:true",
+    "own #_",
+  ],
 };
 
 function buildFixture() {