From 897a1a894660861335301abd127d3eb20b1ab7db Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 20 Dec 2024 03:10:22 +0000 Subject: [PATCH] feat(transformer/class-properties): exit faster from super replacement visitor (#8028) When inserting instance property initializers into class constructor, need to search for and transform `super()`. Exit that visit earlier in some cases, for better performance and smaller output. --- .../es2022/class_properties/constructor.rs | 74 ++++++++++--------- .../snapshots/oxc.snap.md | 4 +- .../super-in-constructor-missing/input.js | 6 ++ .../super-in-constructor-missing/output.js | 6 ++ .../super-in-constructor-nested/input.js | 46 ++++++++++++ .../super-in-constructor-nested/output.js | 47 ++++++++++++ 6 files changed, 145 insertions(+), 38 deletions(-) create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/input.js create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/output.js create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/input.js create mode 100644 tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/output.js diff --git a/crates/oxc_transformer/src/es2022/class_properties/constructor.rs b/crates/oxc_transformer/src/es2022/class_properties/constructor.rs index 84737c1116ecb..08429e3a2796a 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/constructor.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/constructor.rs @@ -682,30 +682,43 @@ impl<'a, 'c> ConstructorBodySuperReplacer<'a, 'c> { mut self, constructor: &mut Function<'a>, ) -> (ScopeId, InstanceInitsInsertLocation<'a>) { - let body_stmts = &mut constructor.body.as_mut().unwrap().statements; - let mut body_stmts_iter = body_stmts.iter_mut(); - - loop { - let mut body_stmts_iter_enumerated = body_stmts_iter.by_ref().enumerate(); - if let Some((index, stmt)) = body_stmts_iter_enumerated.next() { + // This is not a real loop. It always breaks on 1st iteration. + // Only here so that can break out of it from within inner `for` loop. + #[expect(clippy::never_loop)] + 'outer: loop { + let body_stmts = &mut constructor.body.as_mut().unwrap().statements; + for (index, stmt) in body_stmts.iter_mut().enumerate() { // If statement is standalone `super()`, insert inits after `super()`. // We can avoid a `_super` function for this common case. - if let Statement::ExpressionStatement(expr_stmt) = &*stmt { - if let Expression::CallExpression(call_expr) = &expr_stmt.expression { - if call_expr.callee.is_super() { - let insert_location = - InstanceInitsInsertLocation::ExistingConstructor(index + 1); - return (self.constructor_scope_id, insert_location); + if let Statement::ExpressionStatement(expr_stmt) = stmt { + if let Expression::CallExpression(call_expr) = &mut expr_stmt.expression { + if let Expression::Super(super_) = &call_expr.callee { + // Found `super()` as top-level statement + if self.super_binding.is_none() { + // This is the first `super()` found. + // So can just insert initializers after it - no need for `_super` function. + let insert_location = + InstanceInitsInsertLocation::ExistingConstructor(index + 1); + return (self.constructor_scope_id, insert_location); + } + + // `super()` was previously found in nested position before this. + // So we do need a `_super` function. + // But we don't need to look any further for any other `super()` calls, + // because calling `super()` after this would be an immediate error. + let span = super_.span; + self.replace_super(call_expr, span); + + break 'outer; } } } // Traverse statement looking for `super()` deeper in the statement self.visit_statement(stmt); - if self.super_binding.is_some() { - break; - } - } else { + } + + if self.super_binding.is_none() { // No `super()` anywhere in constructor. // This is weird, but legal code. It would be a runtime error if the class is constructed // (unless the constructor returns early). @@ -716,34 +729,23 @@ impl<'a, 'c> ConstructorBodySuperReplacer<'a, 'c> { // What we get is completely legal output and correct `Semantic`, just longer than it // could be. But this should very rarely happen in practice, and minifier will delete // the `_super` function as dead code. + // So set `super_binding` and exit the loop, so it's treated as if `super()` was found + // in a nested position. // TODO: Delete the initializers instead. - let super_func_scope_id = self.create_super_func_scope(); - let super_binding = self.create_super_binding(); - let insert_location = - InstanceInitsInsertLocation::SuperFnInsideConstructor(super_binding); - return (super_func_scope_id, insert_location); + self.super_binding = Some(self.create_super_binding()); } - } - // `super()` found in nested position. There may be more `super()`s in constructor. - // Convert them all to `_super()`. - for stmt in body_stmts_iter { - self.visit_statement(stmt); + break; } - let super_func_scope_id = self.create_super_func_scope(); - let super_binding = self.super_binding.unwrap(); - let insert_location = InstanceInitsInsertLocation::SuperFnInsideConstructor(super_binding); - (super_func_scope_id, insert_location) - } - - /// Create scope for `_super` function inside constructor body - fn create_super_func_scope(&mut self) -> ScopeId { - self.ctx.scopes_mut().add_scope( + let super_func_scope_id = self.ctx.scopes_mut().add_scope( Some(self.constructor_scope_id), NodeId::DUMMY, ScopeFlags::Function | ScopeFlags::Arrow | ScopeFlags::StrictMode, - ) + ); + let super_binding = self.super_binding.unwrap(); + let insert_location = InstanceInitsInsertLocation::SuperFnInsideConstructor(super_binding); + (super_func_scope_id, insert_location) } } diff --git a/tasks/transform_conformance/snapshots/oxc.snap.md b/tasks/transform_conformance/snapshots/oxc.snap.md index a0c97595264cf..c0be81644c67f 100644 --- a/tasks/transform_conformance/snapshots/oxc.snap.md +++ b/tasks/transform_conformance/snapshots/oxc.snap.md @@ -1,6 +1,6 @@ commit: 54a8389f -Passed: 114/130 +Passed: 116/132 # All Passed: * babel-plugin-transform-class-static-block @@ -16,7 +16,7 @@ Passed: 114/130 * regexp -# babel-plugin-transform-class-properties (15/19) +# babel-plugin-transform-class-properties (17/21) * static-super-assignment-target/input.js x Output mismatch diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/input.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/input.js new file mode 100644 index 0000000000000..450a35d47080b --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/input.js @@ -0,0 +1,6 @@ +class C extends S { + prop = 1; + constructor() { + return {}; + } +} diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/output.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/output.js new file mode 100644 index 0000000000000..d501ddf3161b7 --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-missing/output.js @@ -0,0 +1,6 @@ +class C extends S { + constructor() { + var _super = (..._args) => (super(..._args), babelHelpers.defineProperty(this, "prop", 1), this); + return {}; + } +} diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/input.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/input.js new file mode 100644 index 0000000000000..0097df94f93df --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/input.js @@ -0,0 +1,46 @@ +class C extends S { + prop = 1; + constructor() { + // Insert prop initializer after `super()` + super(); + this.self = this; + } +} + +class C2 extends S { + prop = 1; + constructor() { + class Inner extends S { + constructor() { + // Don't transform - different `super` + super(); + } + } + + // Insert prop initializer after `super()` + super(); + this.self = this; + } +} + +class C3 extends S { + prop = 1; + constructor() { + if (condition) { + // Transform to `_super()` + super(1, 2); + return this; + } + + // Transform to `_super()` + super(3, 4); + + if (condition2) { + // Don't transform + super(5, 6); + } + + // Don't transform + super(7, 8); + } +} diff --git a/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/output.js b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/output.js new file mode 100644 index 0000000000000..e1c75dfaab154 --- /dev/null +++ b/tasks/transform_conformance/tests/babel-plugin-transform-class-properties/test/fixtures/super-in-constructor-nested/output.js @@ -0,0 +1,47 @@ +class C extends S { + constructor() { + // Insert prop initializer after `super()` + super(); + babelHelpers.defineProperty(this, "prop", 1); + this.self = this; + } +} + +class C2 extends S { + constructor() { + class Inner extends S { + constructor() { + // Don't transform - different `super` + super(); + } + } + + // Insert prop initializer after `super()` + super(); + babelHelpers.defineProperty(this, "prop", 1); + this.self = this; + } +} + +class C3 extends S { + constructor() { + var _super = (..._args) => (super(..._args), babelHelpers.defineProperty(this, "prop", 1), this); + + if (condition) { + // Transform to `_super()` + _super(1, 2); + return this; + } + + // Transform to `_super()` + _super(3, 4); + + if (condition2) { + // Don't transform + super(5, 6); + } + + // Don't transform + super(7, 8); + } +}