Invalid output from async-to-generator transform when async arrow function in class constructor

[overlookmotel]

Input:

class C extends S {
  constructor() {
    super();
    this.f = async () => this;
  }
}

Transformed by async-to-generator:

class C extends S {
  constructor() {
    var _this = this;
    super();
    this.f = _asyncToGenerator(function*() {
      return _this;
    });
  }
}

Oxc Playground

Problem is that this is now accessed before super(), which is an error.

Babel puts _this = this after super():

class C extends S {
  constructor() {
    var _this;
    super();
    _this = this;
    this.f = _asyncToGenerator(function*() {
      return _this;
    });
  }
}

Note it gets a bit more complicated than that: Babel REPL

[overlookmotel]

And I found another bug in Babel: Babel REPL (that would be really hard to fix)

[overlookmotel]

If we fix this, we should be able to remove 2 overrides from class properties transform fixtures:

• https://github.com/oxc-project/oxc/tree/main/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/foobar
• https://github.com/oxc-project/oxc/tree/main/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/private-loose/foobar

[Dunqing]

For this case, I prefer to diverge with Babel's output and always put _this = this before the first async generator function, so it's certainly after super(). I haven't thought about what will break after this change.

[overlookmotel]

That's a good idea.

But if the 1st async function is nested, then you'd need to add it before all generator functions until you find one which is unconditionally executed:

class C extends S {
  constructor() {
    super();

    if (condition) {
      // `_this = this` required here
      this.fn = async () => { return [this, 1]; };
    }

    for (let x in array) {
      // `_this = this` required here
      this.fns.push(async () => { return [this, x]; });
    }

    // `_this = this` required here
    this.otherFn = async () => { return [this, 2]; };
    // `_this = this` NOT required here
    this.yetAnotherFn = async () => { return [this, 3]; };
  }
}

A simpler solution would be that when async arrow function is in a constructor of a class with a super class, convert it to _asyncToGenerator(function*() {}.bind(this)).

That'd be more lengthy output if constructor contains multiple async arrow functions, but it'd be correct, and easier/faster than trying to find all the super()s.

If you do want to find all the super()s, you could copy and simplify the visitor from class properties transform which replaces super() in constructor:

oxc/crates/oxc_transformer/src/es2022/class_properties/constructor.rs

Lines 659 to 826 in 862838f

	/// Visitor for transforming `super()` in class constructor body.
	struct ConstructorBodySuperReplacer<'a, 'c> {
	/// Scope of class constructor
	constructor_scope_id: ScopeId,
	/// Binding for `_super` function.
	/// Initially `None`. Binding is created if `super()` is found in position other than top-level,
	/// that requires a `_super` function.
	super_binding: Option<BoundIdentifier<'a>>,
	ctx: &'c mut TraverseCtx<'a>,
	}
	impl<'a, 'c> ConstructorBodySuperReplacer<'a, 'c> {
	fn new(constructor_scope_id: ScopeId, ctx: &'c mut TraverseCtx<'a>) -> Self {
	Self { constructor_scope_id, super_binding: None, ctx }
	}
	/// If `super()` found first as a top level statement (`constructor() { let x; super(); }`),
	/// does not alter constructor, and returns `InstanceInitsInsertLocation::ExistingConstructor`
	/// and constructor's `ScopeId`.
	///
	/// Otherwise, replaces any `super()` calls with `_super()` and returns
	/// `InstanceInitsInsertLocation::SuperFnInsideConstructor`, and `ScopeId` for `_super` function.
	fn replace(
	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() {
	// 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);
	}
	}
	}
	// Traverse statement looking for `super()` deeper in the statement
	self.visit_statement(stmt);
	if self.super_binding.is_some() {
	break;
	}
	} else {
	// 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).
	// In reasonable code, we should never get here.
	// Handle this weird case of no `super()` by inserting initializers in a `_super` function
	// which is never called. That is pointless, but not inserting the initializers anywhere
	// would leave `Semantic` in an inconsistent state.
	// 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.
	// 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);
	}
	}
	// `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);
	}
	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(
	Some(self.constructor_scope_id),
	NodeId::DUMMY,
	ScopeFlags::Function | ScopeFlags::Arrow | ScopeFlags::StrictMode,
	)
	}
	}
	impl<'a, 'c> VisitMut<'a> for ConstructorBodySuperReplacer<'a, 'c> {
	/// Replace `super()` with `_super()`.
	// `#[inline]` to make hot path for all other function calls as cheap as possible.
	#[inline]
	fn visit_call_expression(&mut self, call_expr: &mut CallExpression<'a>) {
	if let Expression::Super(super_) = &call_expr.callee {
	let span = super_.span;
	self.replace_super(call_expr, span);
	}
	walk_mut::walk_call_expression(self, call_expr);
	}
	// Stop traversing where scope of current `super` ends
	#[inline]
	fn visit_function(&mut self, _func: &mut Function<'a>, _flags: ScopeFlags) {}
	#[inline]
	fn visit_static_block(&mut self, _block: &mut StaticBlock) {}
	#[inline]
	fn visit_ts_module_block(&mut self, _block: &mut TSModuleBlock<'a>) {}
	#[inline]
	fn visit_property_definition(&mut self, prop: &mut PropertyDefinition<'a>) {
	// `super()` in computed key of property or method refers to super binding of parent class.
	// So visit computed `key`, but not `value`.
	// ```js
	// class Outer extends OuterSuper {
	// constructor() {
	// class Inner extends InnerSuper {
	// [super().foo] = 1; // `super()` refers to `Outer`'s super class
	// [super().bar]() {} // `super()` refers to `Outer`'s super class
	// x = super(); // `super()` refers to `Inner`'s super class, but illegal syntax
	// }
	// }
	// }
	// ```
	// Don't visit `type_annotation` field because can't contain `super()`.
	// TODO: Are decorators in scope?
	self.visit_decorators(&mut prop.decorators);
	if prop.computed {
	self.visit_property_key(&mut prop.key);
	}
	}
	#[inline]
	fn visit_accessor_property(&mut self, prop: &mut AccessorProperty<'a>) {
	// Visit computed `key` but not `value`, for same reasons as `visit_property_definition` above.
	// TODO: Are decorators in scope?
	self.visit_decorators(&mut prop.decorators);
	if prop.computed {
	self.visit_property_key(&mut prop.key);
	}
	}
	}
	impl<'a, 'c> ConstructorBodySuperReplacer<'a, 'c> {
	/// Replace `super(arg1, arg2)` with `_super(arg1, arg2)`
	fn replace_super(&mut self, call_expr: &mut CallExpression<'a>, span: Span) {
	if self.super_binding.is_none() {
	self.super_binding = Some(self.create_super_binding());
	}
	let super_binding = self.super_binding.as_ref().unwrap();
	call_expr.callee = super_binding.create_spanned_read_expression(span, self.ctx);
	}
	/// Create binding for `_super` function
	fn create_super_binding(&mut self) -> BoundIdentifier<'a> {
	self.ctx.generate_uid(
	"super",
	self.constructor_scope_id,
	SymbolFlags::FunctionScopedVariable,
	)
	}
	}

[overlookmotel]

Actually both my suggestions don't work! The async arrow function can be before the super call:

class C extends S {
  constructor() {
    const fn = async () => this;
    super();
    this.fn = fn;
  }
}

so .bind(this) wouldn't work here, and nor would putting _this = this before the async function.

Async function can even be inside super() call!

class C extends S {
  constructor() {
    super(async () => this);
  }
}

Maybe there's another way different from Babel's, but probably it's just as complicated as Babel's method...

At least the visitor (that class properties uses) is not so expensive in most cases, because most of the time super() is a top-level statement near top of the constructor, so visitor finds it quickly and exits without traversing the rest of the constructor.