fix(transformer/class-properties): create temp var for class where re…

…quired

crates/oxc_transformer/src/es2022/class_properties/class.rs

@@ -18,7 +18,7 @@ use super::{
         create_assignment, create_underscore_ident_name, create_variable_declaration,
         exprs_into_stmts,
     },
-    ClassName, ClassProperties, FxIndexMap, PrivateProp, PrivateProps,
+    ClassProperties, FxIndexMap, PrivateProp, PrivateProps,
 };
 
 impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
@@ -57,10 +57,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             return 0;
         }
 
-        self.class_name = ClassName::Name(match &class.id {
-            Some(id) => id.name.as_str(),
-            None => "Class",
-        });
         self.is_declaration = false;
 
         self.transform_class(class, ctx);
@@ -103,10 +99,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         // TODO: Deduct static private props from `expr_count`.
         // Or maybe should store count and increment it when create private static props?
         // They're probably pretty rare, so it'll be rarely used.
-        expr_count += match &self.class_name {
-            ClassName::Binding(_) => 2,
-            ClassName::Name(_) => 1,
-        };
+        expr_count += 1 + usize::from(self.class_temp_binding.is_some());
 
         let mut exprs = ctx.ast.vec_with_capacity(expr_count);
 
@@ -140,7 +133,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
 
         // Insert class + static property assignments + static blocks
         let class_expr = ctx.ast.move_expression(expr);
-        if let ClassName::Binding(binding) = &self.class_name {
+        if let Some(binding) = &self.class_temp_binding {
             // `_Class = class {}`
             let assignment = create_assignment(binding, class_expr, ctx);
             exprs.push(assignment);
@@ -178,10 +171,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             return;
         }
 
-        // Class declarations are always named, except for `export default class {}`, which is handled separately
-        let ident = class.id.as_ref().unwrap();
-        self.class_name = ClassName::Binding(BoundIdentifier::from_binding_ident(ident));
-
         self.transform_class_declaration_impl(class, stmt_address, ctx);
     }
 
@@ -195,20 +184,14 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         stmt_address: Address,
         ctx: &mut TraverseCtx<'a>,
     ) {
-        // Class declarations as default export may not have a name
-        self.class_name = match class.id.as_ref() {
-            Some(ident) => ClassName::Binding(BoundIdentifier::from_binding_ident(ident)),
-            None => ClassName::Name("Class"),
-        };
-
         self.transform_class_declaration_impl(class, stmt_address, ctx);
 
         // If class was unnamed `export default class {}`, and a binding is required, set its name.
         // e.g. `export default class { static x = 1; }` -> `export default class _Class {}; _Class.x = 1;`
         // TODO(improve-on-babel): Could avoid this if treated `export default class {}` as a class expression
         // instead of a class declaration.
         if class.id.is_none() {
-            if let ClassName::Binding(binding) = &self.class_name {
+            if let Some(binding) = &self.class_name_binding {
                 class.id = Some(binding.create_binding_identifier(ctx));
             }
         }
@@ -226,6 +209,28 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
 
         // TODO: Run other transforms on inserted statements. How?
 
+        if let Some(temp_binding) = &self.class_temp_binding {
+            // Binding for class name is required
+            if let Some(ident) = &class.id {
+                // Insert `_Class = Class` after class.
+                // TODO(improve-on-babel): Could just insert `var _Class = Class;` after class,
+                // rather than separate `var _Class` declaration.
+                let class_name = ctx.create_bound_ident_expr(
+                    SPAN,
+                    ident.name.clone(),
+                    ident.symbol_id(),
+                    ReferenceFlags::Read,
+                );
+                let expr = create_assignment(temp_binding, class_name, ctx);
+                let stmt = ctx.ast.statement_expression(SPAN, expr);
+                self.insert_after_stmts.insert(0, stmt);
+            } else {
+                // Class must be default export `export default class {}`, as all other class declarations
+                // always have a name. Set class name.
+                class.id = Some(temp_binding.create_binding_identifier(ctx));
+            }
+        }
+
         // Insert expressions before/after class
         if !self.insert_before.is_empty() {
             self.ctx.statement_injector.insert_many_before(
@@ -280,6 +285,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         // Maybe force transform of static blocks if any static properties?
         // Or alternatively could insert static property initializers into static blocks.
 
+        // Initialize class binding vars
+        self.class_name_binding = class.id.as_ref().map(BoundIdentifier::from_binding_ident);
+        self.class_temp_binding = None;
+
         // Check if class has any properties and get index of constructor (if class has one)
         let mut instance_prop_count = 0;
         let mut has_static_prop_or_static_block = false;
@@ -306,9 +315,9 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
                     }
 
                     if prop.r#static {
-                        // TODO(improve-on-babel): Even though private static properties may not access
-                        // class name, Babel still creates a temp var for class. That's unnecessary.
-                        self.initialize_class_name_binding(ctx);
+                        if !self.is_declaration || self.class_name_binding.is_none() {
+                            self.initialize_class_temp_binding(ctx);
+                        }
 
                         has_static_prop_or_static_block = true;
                     } else {
@@ -342,13 +351,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         if private_props.is_empty() {
             self.private_props_stack.push(None);
         } else {
-            let class_name_binding = match &self.class_name {
-                ClassName::Binding(binding) => Some(binding.clone()),
-                ClassName::Name(_) => None,
-            };
             self.private_props_stack.push(Some(PrivateProps {
                 props: private_props,
-                class_name_binding,
+                class_name_binding: self.class_name_binding.clone(),
+                class_temp_binding: self.class_temp_binding.clone(),
                 is_declaration: self.is_declaration,
             }));
         }
@@ -460,31 +466,46 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             self.insert_private_static_init_assignment(ident, value, ctx);
         } else {
             // Convert to assignment or `_defineProperty` call, depending on `loose` option
-            let ClassName::Binding(class_name_binding) = &self.class_name else {
-                // Binding is initialized in 1st pass in `transform_class` when a static prop is found
-                unreachable!();
+            let class_name_binding = if self.is_declaration {
+                self.class_name_binding.as_ref().unwrap()
+            } else {
+                self.class_temp_binding.as_ref().unwrap()
             };
+
             let assignee = class_name_binding.create_read_expression(ctx);
             let init_expr = self.create_init_assignment(prop, value, assignee, true, ctx);
             self.insert_expr_after_class(init_expr, ctx);
         }
     }
 
-    /// Create a binding for class name, if there isn't one already.
-    fn initialize_class_name_binding(&mut self, ctx: &mut TraverseCtx<'a>) -> &BoundIdentifier<'a> {
-        if let ClassName::Name(name) = &self.class_name {
-            let binding = if self.is_declaration {
-                ctx.generate_uid_in_current_scope(name, SymbolFlags::Class)
+    /// Create a binding for class temp var, if there isn't one already.
+    fn initialize_class_temp_binding(&mut self, ctx: &mut TraverseCtx<'a>) -> &BoundIdentifier<'a> {
+        if self.class_temp_binding.is_none() {
+            // Base temp binding name on class name, or "Class" if no name.
+            // Create a `var _Class;` statement unless this is `export default class {}`,
+            // in which case the class itself will receive this name (`export default class _Class {}`).
+            let (name, flags) = if let Some(binding) = self.class_name_binding.as_ref() {
+                (binding.name.as_str(), SymbolFlags::FunctionScopedVariable)
             } else {
-                let flags = SymbolFlags::FunctionScopedVariable;
-                let binding = ctx.generate_uid_in_current_scope(name, flags);
-                self.ctx.var_declarations.insert_var(&binding, None, ctx);
-                binding
+                // Class declaration can only be nameless if it's `export default class {}`
+                let flags = if self.is_declaration {
+                    SymbolFlags::Class
+                } else {
+                    SymbolFlags::FunctionScopedVariable
+                };
+                ("Class", flags)
             };
-            self.class_name = ClassName::Binding(binding);
+
+            let binding = ctx.generate_uid_in_current_scope(name, flags);
+            if flags == SymbolFlags::FunctionScopedVariable {
+                self.ctx.var_declarations.insert_var(&binding, None, ctx);
+            } else {
+                self.class_name_binding = Some(binding.clone());
+            }
+            self.class_temp_binding = Some(binding);
         }
-        let ClassName::Binding(binding) = &self.class_name else { unreachable!() };
-        binding
+
+        self.class_temp_binding.as_ref().unwrap()
     }
 
     /// `assignee.foo = value` or `_defineProperty(assignee, "foo", value)`

crates/oxc_transformer/src/es2022/class_properties/mod.rs

@@ -201,10 +201,11 @@ pub struct ClassProperties<'a, 'ctx> {
     //
     /// `true` for class declaration, `false` for class expression
     is_declaration: bool,
-    /// Var for class.
-    /// e.g. `X` in `class X {}`.
+    /// Binding for class name, if class has name
+    class_name_binding: Option<BoundIdentifier<'a>>,
+    /// Temp var for class.
     /// e.g. `_Class` in `_Class = class {}, _Class.x = 1, _Class`
-    class_name: ClassName<'a>,
+    class_temp_binding: Option<BoundIdentifier<'a>>,
     /// Expressions to insert before class
     insert_before: Vec<Expression<'a>>,
     /// Expressions to insert after class expression
@@ -213,22 +214,15 @@ pub struct ClassProperties<'a, 'ctx> {
     insert_after_stmts: Vec<Statement<'a>>,
 }
 
-/// Representation of binding for class name.
-enum ClassName<'a> {
-    /// Class has a name. This is the binding.
-    Binding(BoundIdentifier<'a>),
-    /// Class is anonymous.
-    /// This is the name it would have if we need to set class name, in order to reference it.
-    Name(&'a str),
-}
-
 /// Details of private properties for a class.
 struct PrivateProps<'a> {
     /// Private properties for class. Indexed by property name.
     // TODO(improve-on-babel): Order that temp vars are created in is not important. Use `FxHashMap` instead.
     props: FxIndexMap<Atom<'a>, PrivateProp<'a>>,
-    /// Binding for class name
+    /// Binding for class name, if class has name
     class_name_binding: Option<BoundIdentifier<'a>>,
+    /// Temp var for class
+    class_temp_binding: Option<BoundIdentifier<'a>>,
     /// `true` for class declaration, `false` for class expression
     is_declaration: bool,
 }
@@ -257,7 +251,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             class_expression_addresses_stack: NonEmptyStack::new(Address::DUMMY),
             // Temporary values - overwritten when entering class
             is_declaration: false,
-            class_name: ClassName::Name(""),
+            class_name_binding: None,
+            class_temp_binding: None,
             // `Vec`s and `FxHashMap`s which are reused for every class being transformed
             insert_before: vec![],
             insert_after_exprs: vec![],

crates/oxc_transformer/src/es2022/class_properties/private.rs

@@ -1048,7 +1048,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             if let Some(prop) = private_props.props.get(&ident.name) {
                 return Some((
                     prop,
-                    &private_props.class_name_binding,
+                    &private_props.class_temp_binding,
                     private_props.is_declaration,
                 ));
             }

crates/oxc_transformer/src/es2022/class_properties/static_prop.rs

@@ -8,7 +8,7 @@ use oxc_ast::{
 use oxc_syntax::scope::ScopeFlags;
 use oxc_traverse::{BoundIdentifier, TraverseCtx};
 
-use super::{ClassName, ClassProperties};
+use super::ClassProperties;
 
 impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
     /// Transform any `this` in static property initializer to reference to class name,
@@ -18,32 +18,41 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
         value: &mut Expression<'a>,
         ctx: &mut TraverseCtx<'a>,
     ) {
-        // TODO: Insert temp var if class binding is mutated.
+        // TODO: Replace references to class name with temp var
 
-        let ClassName::Binding(class_name_binding) = &self.class_name else {
-            // Binding is initialized in 1st pass in `transform_class` when a static prop is found
-            unreachable!();
-        };
         // Unfortunately have to clone, because also pass `&mut self` to `StaticInitializerVisitor::new`
-        let class_name_binding = class_name_binding.clone();
+        let this_replacement_binding = if self.is_declaration {
+            self.class_name_binding.clone().unwrap()
+        } else {
+            self.class_temp_binding.clone().unwrap()
+        };
 
-        let mut replacer = StaticInitializerVisitor::new(class_name_binding, self, ctx);
+        let mut replacer = StaticInitializerVisitor::new(this_replacement_binding, self, ctx);
         replacer.visit_expression(value);
     }
 }
 
 /// Visitor to transform:
 ///
-/// 1. `this` to class name.
-///    `class C { static x = this.y; }` -> `class C {}; C.x = C.y;`
+/// 1. `this` to class temp var.
+///    * Class declaration: `class C { static x = this.y; }`
+///      -> `var _C; class C {}; _C = C; C.x = _C.y;`
+///    * Class expression: `x = class C { static x = this.y; }`
+///      -> `var _C; x = (_C = class C {}, _C.x = _C.y)`
 /// 2. Private fields which refer to private props of this class.
-///    `class C { static #x = 123; static.#y = this.#x; }`
-///    -> `class C {}; var _x = { _: 123 }; _defineProperty(C, "y", _assertClassBrand(C, C, _x)._);`
+///    * Class declaration: `class C { static #x = 123; static y = this.#x; }`
+///      -> `var _C; class C {}; _C = C; var _x = { _: 123 }; C.y = _assertClassBrand(_C, _C, _x)._;`
+///    * Class expression: `x = class C { static #x = 123; static y = this.#x; }`
+///      -> `var _C, _x; x = (_C = class C {}, _x = { _: 123 }, _C.y = _assertClassBrand(_C, _C, _x)._), _C)`
 ///
 /// Reason we need to do this is because the initializer is being moved from inside the class to outside.
 /// `this` outside the class refers to a different `this`, and private fields are only valid within the
 /// class body. So we need to transform them.
 ///
+/// Note that for class declarations, assignments are made to properties of original class name `C`,
+/// but temp var `_C` is used in replacements for `this` and private fields. This is because class binding
+/// `C` can be mutated, and the initializer may contain functions which are not executed immediately.
+///
 /// If this class defines no private properties, we only need to transform `this`, so can skip traversing
 /// into functions and other contexts which have their own `this`.
 ///
@@ -52,8 +61,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
 //
 // TODO: Also re-parent child scopes.
 struct StaticInitializerVisitor<'a, 'ctx, 'v> {
-    /// Binding for class name.
-    class_name_binding: BoundIdentifier<'a>,
+    /// Binding
+    this_replacement_binding: BoundIdentifier<'a>,
     /// `true` if class has private properties.
     class_has_private_props: bool,
     /// Incremented when entering a different `this` context, decremented when exiting it.
@@ -67,12 +76,12 @@ struct StaticInitializerVisitor<'a, 'ctx, 'v> {
 
 impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
     fn new(
-        class_name_binding: BoundIdentifier<'a>,
+        this_replacement_binding: BoundIdentifier<'a>,
         class_properties: &'v mut ClassProperties<'a, 'ctx>,
         ctx: &'v mut TraverseCtx<'a>,
     ) -> Self {
         Self {
-            class_name_binding,
+            this_replacement_binding,
             class_has_private_props: class_properties.private_props_stack.last().is_some(),
             this_depth: 0,
             class_properties,
@@ -219,7 +228,7 @@ impl<'a, 'ctx, 'v> StaticInitializerVisitor<'a, 'ctx, 'v> {
     /// Replace `this` with reference to class name binding.
     fn replace_this_with_class_name(&mut self, expr: &mut Expression<'a>, span: Span) {
         if self.this_depth == 0 {
-            *expr = self.class_name_binding.create_spanned_read_expression(span, self.ctx);
+            *expr = self.this_replacement_binding.create_spanned_read_expression(span, self.ctx);
         }
     }