Skip to content

Commit

Permalink
feat(transformer/class-properties): exit faster from super replacemen…
Browse files Browse the repository at this point in the history
…t 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.
  • Loading branch information
overlookmotel committed Dec 20, 2024
1 parent de4c772 commit 897a1a8
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 38 deletions.
74 changes: 38 additions & 36 deletions crates/oxc_transformer/src/es2022/class_properties/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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)
}
}

Expand Down
4 changes: 2 additions & 2 deletions tasks/transform_conformance/snapshots/oxc.snap.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
commit: 54a8389f

Passed: 114/130
Passed: 116/132

# All Passed:
* babel-plugin-transform-class-static-block
Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class C extends S {
prop = 1;
constructor() {
return {};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class C extends S {
constructor() {
var _super = (..._args) => (super(..._args), babelHelpers.defineProperty(this, "prop", 1), this);
return {};
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 897a1a8

Please sign in to comment.