From 3ebd4943f2c540ebf86642d404f109c9565913b6 Mon Sep 17 00:00:00 2001
From: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Date: Wed, 16 Apr 2025 16:35:09 +0000
Subject: [PATCH] refactor(transformer/class-properties): re-order fields and
 expand comments (#10447)

Follow-on after #10418. Pure refactor.

`private_field_count` is accessed in both the entry and exit phases of the transform, so move that field up to be with the other fields which are in that same category.

Expand the comment which explains how this optimization works.

Also, when reviewing #10418, I had to trace through the logic to make sure that it wasn't possible for decreasing `private_field_count` on exit to get skipped if `is_transform_required` is `false`. Turns out it's not a problem - `is_transform_required` is always `true` if class has any private properties/methods. Add comments to clarify that.
---
 .../src/es2022/class_properties/class.rs               |  6 ++++++
 .../oxc_transformer/src/es2022/class_properties/mod.rs | 10 ++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/crates/oxc_transformer/src/es2022/class_properties/class.rs b/crates/oxc_transformer/src/es2022/class_properties/class.rs
index 4054d8c6813de..7265cf37e7271 100644
--- a/crates/oxc_transformer/src/es2022/class_properties/class.rs
+++ b/crates/oxc_transformer/src/es2022/class_properties/class.rs
@@ -385,6 +385,8 @@ impl<'a> ClassProperties<'a, '_> {
         if class_details.is_transform_required {
             self.transform_class_declaration_on_exit_impl(class, ctx);
         } else {
+            // Note: `is_transform_required` is `true` if class has any private properties/methods,
+            // so no need to touch `private_field_count` here
             debug_assert!(class_details.bindings.temp.is_none());
         }
         // Pop off stack. We're done!
@@ -446,6 +448,7 @@ impl<'a> ClassProperties<'a, '_> {
 
         if let Some(private_props) = &class_details.private_props {
             self.private_field_count -= private_props.len();
+
             if self.private_fields_as_properties {
                 let mut private_props = private_props
                     .iter()
@@ -536,6 +539,8 @@ impl<'a> ClassProperties<'a, '_> {
         if class_details.is_transform_required {
             self.transform_class_expression_on_exit_impl(expr, ctx);
         } else {
+            // Note: `is_transform_required` is `true` if class has any private properties/methods,
+            // so no need to touch `private_field_count` here
             debug_assert!(class_details.bindings.temp.is_none());
         }
 
@@ -591,6 +596,7 @@ impl<'a> ClassProperties<'a, '_> {
         // Also insert `var _prop;` temp var declarations for private static props.
         if let Some(private_props) = &class_details.private_props {
             self.private_field_count -= private_props.len();
+
             // Insert `var _prop;` declarations here rather than when binding was created to maintain
             // same order of `var` declarations as Babel.
             // `c = class C { #x = 1; static y = 2; }` -> `var _C, _x;`
diff --git a/crates/oxc_transformer/src/es2022/class_properties/mod.rs b/crates/oxc_transformer/src/es2022/class_properties/mod.rs
index fbf026a81224e..61c23bf64cf8a 100644
--- a/crates/oxc_transformer/src/es2022/class_properties/mod.rs
+++ b/crates/oxc_transformer/src/es2022/class_properties/mod.rs
@@ -261,6 +261,8 @@ pub struct ClassProperties<'a, 'ctx> {
     /// This problem only affects class expressions. Class declarations aren't affected,
     /// as their exit-phase transform happens in `exit_class`.
     classes_stack: ClassesStack<'a>,
+    /// Count of private fields in current class and parent classes.
+    private_field_count: usize,
 
     // ----- State used only during enter phase -----
     //
@@ -277,8 +279,6 @@ pub struct ClassProperties<'a, 'ctx> {
     /// Keys are symbols' IDs.
     /// Values are initially the original name of binding, later on the name of new UID name.
     clashing_constructor_symbols: FxHashMap<SymbolId, Atom<'a>>,
-    /// Count of private fields in class and also included parent classes.
-    private_field_count: usize,
 
     // ----- State used only during exit phase -----
     //
@@ -309,12 +309,12 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
             transform_static_blocks,
             ctx,
             classes_stack: ClassesStack::new(),
+            private_field_count: 0,
             // Temporary values - overwritten when entering class
             instance_inits_scope_id: ScopeId::new(0),
             instance_inits_constructor_scope_id: None,
             // `Vec`s and `FxHashMap`s which are reused for every class being transformed
             clashing_constructor_symbols: FxHashMap::default(),
-            private_field_count: 0,
             insert_before: vec![],
             insert_after_exprs: vec![],
             insert_after_stmts: vec![],
@@ -342,7 +342,9 @@ impl<'a> Traverse<'a> for ClassProperties<'a, '_> {
     // `#[inline]` for fast exit for expressions which are not any of the transformed types
     #[inline]
     fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
-        // Return early if no private fields are found.
+        // All of transforms below only act on `PrivateFieldExpression`s or `PrivateInExpression`s.
+        // If we're not inside a class which has private fields, `#prop` can't be present here,
+        // so exit early - fast path for common case.
         if self.private_field_count == 0 {
             return;
         }