mirror of
https://github.com/aphrody-labs/oxc.git
synced 2026-10-14 22:29:17 +00:00
fix(transformer/class-properties): keep private methods in class-expression scope (#26308)
Private methods lowered from a class expression inside a concise arrow
were inserted next to the arrow's outer statement, while private field
storage remained inside the arrow. Starting with Oxc 0.143, this could
leave the generated helper unable to resolve the private field and throw
a `ReferenceError` at runtime.
Concise arrow bodies have no statement list where the helper declaration
can be inserted. In that context, the helper is now emitted as an
assignment in the class-expression sequence, with its `var` binding
inside the expanded arrow body. Ordinary statement-backed class
expressions keep block-local function declarations so classes created by
repeated block evaluation retain their own captured values. The
generated symbol flags and scope parents match the emitted form in both
cases.
## Example
```js
const makeClass = () => class {
#field = 41;
#read() { return this.#field; }
run() { return this.#read() + 1; }
};
new (makeClass())().run();
```
Before Oxc emitted a helper outside `makeClass`, and the call threw
`ReferenceError: _field is not defined`. After this change it returns
`42`.
Verified with the private-method transform conformance suite, runtime
regressions for concise arrows and repeated block evaluation, `cargo
test -p oxc_transformer`, and transformer clippy.
Fixes the upstream cause of
[rolldown/rolldown#10818](https://github.com/rolldown/rolldown/issues/10818).
AI assistance: Cursor was used for the original patch; OpenAI Codex was
used for follow-up review, fixes, and testing.
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Dunqing <dengqing0821@gmail.com>
This commit is contained in:
13 files changed
+185
-18
No files matched your search
@@ -73,6 +73,8 @@ impl<'a> ClassProperties<'a> {
|
||||
let mut class_name_binding = class.id().as_ref().map(BoundIdentifier::from_binding_ident);
|
||||
let class_scope_id = class.scope_id().get().unwrap();
|
||||
let has_super_class = class.heritage().is_some();
|
||||
let private_method_helpers_in_class_sequence =
|
||||
!is_declaration && Self::should_emit_private_method_helpers_in_class_sequence(ctx);
|
||||
|
||||
// Check if class has any properties, private methods, or static blocks
|
||||
let mut instance_prop_count = 0;
|
||||
@@ -128,11 +130,12 @@ impl<'a> ClassProperties<'a> {
|
||||
MethodDefinitionKind::Set => &format!("set_{}", ident.name),
|
||||
MethodDefinitionKind::Constructor => unreachable!(),
|
||||
};
|
||||
let binding = ctx.generate_uid(
|
||||
name,
|
||||
ctx.current_block_scope_id(),
|
||||
SymbolFlags::Function,
|
||||
);
|
||||
let (scope_id, flags) = if private_method_helpers_in_class_sequence {
|
||||
(ctx.current_hoist_scope_id(), SymbolFlags::FunctionScopedVariable)
|
||||
} else {
|
||||
(ctx.current_block_scope_id(), SymbolFlags::Function)
|
||||
};
|
||||
let binding = ctx.generate_uid(name, scope_id, flags);
|
||||
|
||||
if let Some(prop) = private_props.get_mut(&ident.name) {
|
||||
// If there's already a binding for this private property,
|
||||
@@ -178,6 +181,7 @@ impl<'a> ClassProperties<'a> {
|
||||
{
|
||||
self.classes_stack.push(ClassDetails {
|
||||
is_declaration,
|
||||
private_method_helpers_in_class_sequence,
|
||||
is_transform_required: false,
|
||||
private_props: if private_props.is_empty() { None } else { Some(private_props) },
|
||||
bindings: ClassBindings::dummy(),
|
||||
@@ -233,6 +237,7 @@ impl<'a> ClassProperties<'a> {
|
||||
// Add entry to `classes_stack`
|
||||
self.classes_stack.push(ClassDetails {
|
||||
is_declaration,
|
||||
private_method_helpers_in_class_sequence,
|
||||
is_transform_required: true,
|
||||
private_props: if private_props.is_empty() { None } else { Some(private_props) },
|
||||
bindings: class_bindings,
|
||||
@@ -633,7 +638,13 @@ impl<'a> ClassProperties<'a> {
|
||||
// They're probably pretty rare, so it'll be rarely used.
|
||||
let class_details = self.classes_stack.last();
|
||||
|
||||
let private_method_helpers_in_class_sequence =
|
||||
class_details.private_method_helpers_in_class_sequence;
|
||||
let mut expr_count = self.insert_before.len() + self.insert_after_exprs.len();
|
||||
if private_method_helpers_in_class_sequence {
|
||||
// Private method helpers are emitted into the class-expression sequence (see below).
|
||||
expr_count += self.insert_after_stmts.len();
|
||||
}
|
||||
if let Some(private_props) = &class_details.private_props {
|
||||
expr_count += private_props.len();
|
||||
}
|
||||
@@ -704,9 +715,27 @@ impl<'a> ClassProperties<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
// Insert private methods
|
||||
if !self.insert_after_stmts.is_empty() {
|
||||
// Find `Address` of statement containing class expression
|
||||
// Insert private methods.
|
||||
if private_method_helpers_in_class_sequence {
|
||||
// A concise arrow has no statement list to inject a function declaration into.
|
||||
// Emit `_m = function () {}` into the class sequence instead. Adding `var _m`
|
||||
// expands the concise arrow to a block, so the binding is fresh for every call.
|
||||
for stmt in self.insert_after_stmts.drain(..) {
|
||||
let Statement::FunctionDeclaration(mut func) = stmt else {
|
||||
unreachable!(
|
||||
"class expression private methods are always function declarations"
|
||||
);
|
||||
};
|
||||
let id = func.id.take().expect("private method binding always has an id");
|
||||
let binding = BoundIdentifier::from_binding_ident(&id);
|
||||
ctx.state.var_declarations.insert_var(&binding, &ctx.ast);
|
||||
func.r#type = FunctionType::FunctionExpression;
|
||||
let assignment =
|
||||
create_assignment(&binding, Expression::FunctionExpression(func), SPAN, ctx);
|
||||
exprs.push(assignment);
|
||||
}
|
||||
} else if !self.insert_after_stmts.is_empty() {
|
||||
// Find `Address` of statement containing class expression.
|
||||
let mut stmt_address = Address::DUMMY;
|
||||
for ancestor in ctx.ancestors() {
|
||||
if ancestor.is_parent_of_statement() {
|
||||
|
||||
@@ -10,6 +10,8 @@ use super::{ClassBindings, ClassProperties, IdentIndexMap};
|
||||
pub(super) struct ClassDetails<'a> {
|
||||
/// `true` for class declaration, `false` for class expression
|
||||
pub is_declaration: bool,
|
||||
/// `true` if private method helpers must be emitted in the class expression sequence.
|
||||
pub private_method_helpers_in_class_sequence: bool,
|
||||
/// `true` if class requires no transformation
|
||||
pub is_transform_required: bool,
|
||||
/// Private properties.
|
||||
@@ -28,6 +30,7 @@ impl ClassDetails<'_> {
|
||||
pub fn dummy(is_declaration: bool) -> Self {
|
||||
Self {
|
||||
is_declaration,
|
||||
private_method_helpers_in_class_sequence: false,
|
||||
is_transform_required: false,
|
||||
private_props: None,
|
||||
bindings: ClassBindings::dummy(),
|
||||
|
||||
@@ -64,16 +64,26 @@ impl<'a> ClassProperties<'a> {
|
||||
function.span = *span;
|
||||
function.id = Some(temp_binding.create_binding_identifier(ctx));
|
||||
function.r#type = FunctionType::FunctionDeclaration;
|
||||
sync_function_symbol_flags(&function, ctx);
|
||||
let helper_in_class_sequence =
|
||||
self.current_class().private_method_helpers_in_class_sequence;
|
||||
if !helper_in_class_sequence {
|
||||
sync_function_symbol_flags(&function, ctx);
|
||||
}
|
||||
|
||||
// Change parent scope of function to current scope id and remove
|
||||
// strict mode flag if parent scope is not strict mode.
|
||||
// Change parent scope of function to the scope where it will be emitted, and remove
|
||||
// strict mode flag if that parent scope is not strict mode.
|
||||
//
|
||||
// Class expressions in concise arrows emit the helper as a function-expression assignment
|
||||
// at the class expression's lexical location. Other classes emit a function declaration
|
||||
// after the containing statement. A static property initializer is moved into the hoist
|
||||
// scope later.
|
||||
let scope_id = function.scope_id();
|
||||
let new_parent_id = if Self::is_inside_static_property_initializer(ctx) {
|
||||
ctx.current_hoist_scope_id()
|
||||
} else {
|
||||
ctx.current_scope_id()
|
||||
};
|
||||
let new_parent_id =
|
||||
if !helper_in_class_sequence && Self::is_inside_static_property_initializer(ctx) {
|
||||
ctx.current_hoist_scope_id()
|
||||
} else {
|
||||
ctx.current_scope_id()
|
||||
};
|
||||
ctx.scoping_mut().change_scope_parent_id(scope_id, Some(new_parent_id));
|
||||
let make_sloppy_mode = !ctx.scoping().scope_flags(new_parent_id).is_strict_mode();
|
||||
let flags = ctx.scoping_mut().scope_flags_mut(scope_id);
|
||||
@@ -85,6 +95,19 @@ impl<'a> ClassProperties<'a> {
|
||||
Some(Statement::FunctionDeclaration(function))
|
||||
}
|
||||
|
||||
/// Whether private method helpers need to be part of the class expression itself.
|
||||
///
|
||||
/// A concise arrow body has no statement list where a function declaration can be injected.
|
||||
/// Stop at the first statement boundary so ordinary class expressions keep block-local
|
||||
/// function declarations, which provide a fresh closure when a block runs repeatedly.
|
||||
pub(super) fn should_emit_private_method_helpers_in_class_sequence(
|
||||
ctx: &TraverseCtx<'a>,
|
||||
) -> bool {
|
||||
ctx.ancestors()
|
||||
.take_while(|ancestor| !ancestor.is_parent_of_statement())
|
||||
.any(|ancestor| matches!(ancestor, Ancestor::ArrowFunctionExpressionBody(_)))
|
||||
}
|
||||
|
||||
fn is_inside_static_property_initializer(ctx: &TraverseCtx<'a>) -> bool {
|
||||
ctx.ancestors().any(|ancestor| {
|
||||
matches!(ancestor, Ancestor::PropertyDefinitionValue(property) if *property.r#static())
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
commit: 1eac4481
|
||||
|
||||
Passed: 274/402
|
||||
Passed: 276/404
|
||||
|
||||
# All Passed:
|
||||
* babel-plugin-transform-class-static-block
|
||||
|
||||
@@ -2,7 +2,7 @@ commit: 1eac4481
|
||||
|
||||
node: v26.5.0
|
||||
|
||||
Passed: 18 of 20 (90.00%)
|
||||
Passed: 20 of 22 (90.91%)
|
||||
|
||||
Failures:
|
||||
|
||||
|
||||
+11
@@ -0,0 +1,11 @@
|
||||
const makeClass = () => class {
|
||||
#field = 41;
|
||||
*#read() {
|
||||
yield this.#field;
|
||||
}
|
||||
run() {
|
||||
return this.#read().next().value + 1;
|
||||
}
|
||||
};
|
||||
|
||||
expect(new (makeClass())().run()).toBe(42);
|
||||
+13
@@ -0,0 +1,13 @@
|
||||
const makeClass = () => class {
|
||||
#field = 41;
|
||||
*#read() {
|
||||
yield this.#field;
|
||||
}
|
||||
run() {
|
||||
return this.#read().next().value + 1;
|
||||
}
|
||||
};
|
||||
|
||||
export function get() {
|
||||
return new (makeClass())().run();
|
||||
}
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"plugins": [
|
||||
"transform-class-properties",
|
||||
"transform-private-methods"
|
||||
]
|
||||
}
|
||||
+17
@@ -0,0 +1,17 @@
|
||||
const makeClass = () => {
|
||||
var _field, _Class_brand, _read;
|
||||
return _field = /* @__PURE__ */ new WeakMap(), _Class_brand = /* @__PURE__ */ new WeakSet(), _read = function* () {
|
||||
yield babelHelpers.classPrivateFieldGet2(_field, this);
|
||||
}, class {
|
||||
constructor() {
|
||||
babelHelpers.classPrivateMethodInitSpec(this, _Class_brand);
|
||||
babelHelpers.classPrivateFieldInitSpec(this, _field, 41);
|
||||
}
|
||||
run() {
|
||||
return babelHelpers.assertClassBrand(_Class_brand, this, _read).call(this).next().value + 1;
|
||||
}
|
||||
};
|
||||
};
|
||||
export function get() {
|
||||
return new (makeClass())().run();
|
||||
}
|
||||
+18
@@ -0,0 +1,18 @@
|
||||
function makeClasses() {
|
||||
const Classes = [];
|
||||
for (let captured = 0; captured < 2; captured++) {
|
||||
Classes.push(class {
|
||||
*#read() {
|
||||
yield captured;
|
||||
}
|
||||
run() {
|
||||
return this.#read().next().value + 1;
|
||||
}
|
||||
});
|
||||
}
|
||||
return Classes;
|
||||
}
|
||||
|
||||
const Classes = makeClasses();
|
||||
expect(new Classes[0]().run()).toBe(1);
|
||||
expect(new Classes[1]().run()).toBe(2);
|
||||
+19
@@ -0,0 +1,19 @@
|
||||
function makeClasses() {
|
||||
const Classes = [];
|
||||
for (let captured = 0; captured < 2; captured++) {
|
||||
Classes.push(class {
|
||||
*#read() {
|
||||
yield captured;
|
||||
}
|
||||
run() {
|
||||
return this.#read().next().value + 1;
|
||||
}
|
||||
});
|
||||
}
|
||||
return Classes;
|
||||
}
|
||||
|
||||
export function get() {
|
||||
const Classes = makeClasses();
|
||||
return [new Classes[0]().run(), new Classes[1]().run()];
|
||||
}
|
||||
+6
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"plugins": [
|
||||
"transform-class-properties",
|
||||
"transform-private-methods"
|
||||
]
|
||||
}
|
||||
+22
@@ -0,0 +1,22 @@
|
||||
function makeClasses() {
|
||||
const Classes = [];
|
||||
for (let captured = 0; captured < 2; captured++) {
|
||||
var _Class_brand;
|
||||
Classes.push((_Class_brand = /* @__PURE__ */ new WeakSet(), class {
|
||||
constructor() {
|
||||
babelHelpers.classPrivateMethodInitSpec(this, _Class_brand);
|
||||
}
|
||||
run() {
|
||||
return babelHelpers.assertClassBrand(_Class_brand, this, _read).call(this).next().value + 1;
|
||||
}
|
||||
}));
|
||||
function* _read() {
|
||||
yield captured;
|
||||
}
|
||||
}
|
||||
return Classes;
|
||||
}
|
||||
export function get() {
|
||||
const Classes = makeClasses();
|
||||
return [new Classes[0]().run(), new Classes[1]().run()];
|
||||
}
|
||||
Reference in new issue
Block a user