Skip to content

Commit

Permalink
feat(transformer/class-properties): transform static/instance accesso…
Browse files Browse the repository at this point in the history
…r methods (#8132)

This PR supports transforming private `getter` or `setter` methods, and the output is a little different from Babel's output, For example:

Input:
```js
class Cls {
  #prop = 0;

  get #accessor() {
    return this.#prop;
  }
  set #accessor(v) {
    console.log(arguments);
    this.#prop = v;
  }

  constructor() {
    console.log(this.#accessor)
    [this.#accessor] = [1];
  }
}
```

[Babel's output](https://babeljs.io/repl#?browsers=defaults%2C%20not%20ie%2011%2C%20not%20ie_mob%2011&build=&builtIns=false&corejs=3.21&spec=false&loose=false&code_lz=MYGwhgzhAEDCIwN4ChrQMQAcBOB7T0AvNAAwDcyq0A5gKYAuGYwwtUu2AFAJTQpppsDAK7YAdtHoALAJYQAdFjyYKaAL5UIDJizYQOnAG69-A4LjH6QteSFzVOYbNWEBbWmPoRuqgdLmKOPhE0Ia-GlTmlvTYwsD0BiZUaFFWNnYO_grozKzs2NzJ0ADaWYq5ehwAuiHFAIxV4cgaQA&debug=false&forceAllTransforms=false&modules=false&shippedProposals=false&evaluate=false&fileSize=false&timeTravel=false&sourceType=script&lineWrap=true&presets=&prettier=false&targets=&version=7.26.4&externalPlugins=%40babel%2Fplugin-transform-class-properties%407.25.9%2C%40babel%2Fplugin-transform-private-methods%407.25.9%2C%40babel%2Fplugin-external-helpers%407.25.9&assumptions=%7B%7D):
```js
var _prop = new WeakMap();
var _Cls_brand = new WeakSet();
class Cls {
  constructor() {
    babelHelpers.classPrivateMethodInitSpec(this, _Cls_brand);
    babelHelpers.classPrivateFieldInitSpec(this, _prop, 0);
    console.log(babelHelpers.classPrivateGetter(_Cls_brand, this, _get_accessor));
    [babelHelpers.classPrivateGetter(_Cls_brand, this, _get_accessor)] = [1];
  }
}
function _get_accessor(_this) {
  return babelHelpers.classPrivateFieldGet(_prop, _this);
}
function _set_accessor(_this2, v) {
  var _arguments = [].slice.call(arguments, 1);
  console.log(_arguments);
  babelHelpers.classPrivateFieldSet(_prop, _this2, v);
}
```

Oxc's output:
```js
var _prop = new WeakMap();
var _Cls_brand = new WeakSet();
class Cls {
        constructor() {
                babelHelpers.classPrivateMethodInitSpec(this, _Cls_brand);
                babelHelpers.classPrivateFieldInitSpec(this, _prop, 0);
                console.log(_get_accessor.call(babelHelpers.assertClassBrand(_Cls_brand, this)));
                [babelHelpers.toSetter(_set_accessor.bind(babelHelpers.assertClassBrand(_Cls_brand, this)), [])._] = [1];
        }
}
function _get_accessor() {
        return babelHelpers.classPrivateFieldGet2(_prop, this);
}
function _set_accessor(v) {
        console.log(arguments);
        babelHelpers.classPrivateFieldSet2(_prop, this, v);
}
```

### Main difference
```diff
// Babel
+ console.log(babelHelpers.classPrivateGetter(_Cls_brand, this, _get_accessor));
+ [babelHelpers.classPrivateGetter(_Cls_brand, this, _get_accessor)] = [1];

+ function _get_accessor(_this) {
+   return babelHelpers.classPrivateFieldGet(_prop, _this);
+ }

+ function _set_accessor(_this2, v) {
+   var _arguments = [].slice.call(arguments, 1);
+   console.log(_arguments);
+   babelHelpers.classPrivateFieldSet(_prop, _this2, v);
+ }

// Oxc
-  console.log(_get_accessor.call(babelHelpers.assertClassBrand(_Cls_brand, this)));- -
-  [babelHelpers.toSetter(_set_accessor.bind(babelHelpers.assertClassBrand(_Cls_brand, this)), [])._] = [1];

- function _get_accessor() {
-   return babelHelpers.classPrivateFieldGet2(_prop, this);
- }

- function _set_accessor(v) {
-   console.log(arguments);
-   babelHelpers.classPrivateFieldSet2(_prop, this, v);
- }
```

From the main differences, we can see that Babel handles `getter` and `setter` methods using `classPrivateGetter` and `classPrivateSetter` helper functions, which causes all use `this` and `arguments` needs to rewrite to use a temp var instead in `getter` and `setter` methods. This is unnecessary and is not an efficient transformation for us.

Instead, I adapt binding a `this` instead of passing in `this`, this way we don't need to rewrite anything. We can't control the `helper` library for now, so I just transformed the AST to bind `this`, in the future, we can create a helper function to do the same thing.
  • Loading branch information
Dunqing committed Dec 31, 2024
1 parent e405f79 commit ad77ad5
Show file tree
Hide file tree
Showing 27 changed files with 1,061 additions and 386 deletions.
4 changes: 4 additions & 0 deletions crates/oxc_transformer/src/common/helper_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ pub enum Helper {
ClassPrivateFieldLooseBase,
SuperPropGet,
SuperPropSet,
ReadOnlyError,
WriteOnlyError,
}

impl Helper {
Expand All @@ -187,6 +189,8 @@ impl Helper {
Self::ClassPrivateFieldLooseBase => "classPrivateFieldLooseBase",
Self::SuperPropGet => "superPropGet",
Self::SuperPropSet => "superPropSet",
Self::ReadOnlyError => "readOnlyError",
Self::WriteOnlyError => "writeOnlyError",
}
}
}
Expand Down
44 changes: 32 additions & 12 deletions crates/oxc_transformer/src/es2022/class_properties/class.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! ES2022: Class Properties
//! Transform of class itself.
use indexmap::map::Entry;
use oxc_allocator::{Address, GetAddress};
use oxc_ast::{ast::*, NONE};
use oxc_span::SPAN;
Expand Down Expand Up @@ -93,7 +94,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let binding = ctx.generate_uid_in_current_hoist_scope(&ident.name);
private_props.insert(
ident.name.clone(),
PrivateProp::new(binding, prop.r#static, false, false),
PrivateProp::new(binding, prop.r#static, None, false),
);
}

Expand Down Expand Up @@ -135,10 +136,22 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
ctx.current_block_scope_id(),
SymbolFlags::FunctionScopedVariable,
);
private_props.insert(
ident.name.clone(),
PrivateProp::new(binding, method.r#static, true, false),
);

match private_props.entry(ident.name.clone()) {
Entry::Occupied(mut entry) => {
// If there's already a binding for this private property,
// it's a setter or getter, so store the binding in `binding2`.
entry.get_mut().set_binding2(binding);
}
Entry::Vacant(entry) => {
entry.insert(PrivateProp::new(
binding,
method.r#static,
Some(method.kind),
false,
));
}
}
}
}
ClassElement::AccessorProperty(prop) => {
Expand All @@ -148,7 +161,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let dummy_binding = BoundIdentifier::new(Atom::empty(), SymbolId::new(0));
private_props.insert(
ident.name.clone(),
PrivateProp::new(dummy_binding, prop.r#static, true, true),
PrivateProp::new(dummy_binding, prop.r#static, None, true),
);
}
}
Expand Down Expand Up @@ -443,7 +456,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
self.ctx.statement_injector.insert_many_before(
&stmt_address,
private_props.iter().filter_map(|(name, prop)| {
if prop.is_method || prop.is_accessor {
if prop.is_method() || prop.is_accessor {
return None;
}

Expand All @@ -459,10 +472,10 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
self.ctx.statement_injector.insert_many_before(
&stmt_address,
private_props.values().filter_map(|prop| {
if prop.is_static || (prop.is_method && has_method) || prop.is_accessor {
if prop.is_static || (prop.is_method() && has_method) || prop.is_accessor {
return None;
}
if prop.is_method {
if prop.is_method() {
// `var _C_brand = new WeakSet();`
has_method = true;
let binding = class_details.bindings.brand();
Expand Down Expand Up @@ -583,7 +596,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// TODO(improve-on-babel): Simplify this.
if self.private_fields_as_properties {
exprs.extend(private_props.iter().filter_map(|(name, prop)| {
if prop.is_method || prop.is_accessor {
if prop.is_method() || prop.is_accessor {
return None;
}

Expand All @@ -598,7 +611,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let mut weakmap_symbol_id = None;
let mut has_method = false;
exprs.extend(private_props.values().filter_map(|prop| {
if prop.is_method || prop.is_accessor {
if prop.is_method() || prop.is_accessor {
if prop.is_static || has_method {
return None;
}
Expand Down Expand Up @@ -703,6 +716,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
/// * Extract computed key assignments and insert them before class.
/// * Remove all properties, private methods and static blocks from class body.
fn transform_class_elements(&mut self, class: &mut Class<'a>, ctx: &mut TraverseCtx<'a>) {
let mut class_methods = vec![];
class.body.body.retain_mut(|element| {
match element {
ClassElement::PropertyDefinition(prop) => {
Expand All @@ -721,7 +735,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
}
ClassElement::MethodDefinition(method) => {
self.substitute_temp_var_for_method_computed_key(method, ctx);
if self.convert_private_method(method, ctx) {
if let Some(statement) = self.convert_private_method(method, ctx) {
class_methods.push(statement);
return false;
}
}
Expand All @@ -732,6 +747,11 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

true
});

// All methods are moved to after the class, but need to be before static properties
// TODO(improve-on-babel): Insertion order doesn't matter, and it more clear to insert according to
// definition order.
self.insert_after_stmts.splice(0..0, class_methods);
}

/// Flag that static private fields should be transpiled using temp binding,
Expand Down
158 changes: 140 additions & 18 deletions crates/oxc_transformer/src/es2022/class_properties/class_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,33 @@ impl<'a> ClassDetails<'a> {
pub(super) struct PrivateProp<'a> {
pub binding: BoundIdentifier<'a>,
pub is_static: bool,
pub is_method: bool,
pub method_kind: Option<MethodDefinitionKind>,
pub is_accessor: bool,
// For accessor methods, they have two bindings,
// one for getter and another for setter.
pub binding2: Option<BoundIdentifier<'a>>,
}

impl<'a> PrivateProp<'a> {
pub fn new(
binding: BoundIdentifier<'a>,
is_static: bool,
is_method: bool,
method_kind: Option<MethodDefinitionKind>,
is_accessor: bool,
) -> Self {
Self { binding, is_static, is_method, is_accessor }
Self { binding, is_static, method_kind, is_accessor, binding2: None }
}

pub fn is_method(&self) -> bool {
self.method_kind.is_some()
}

pub fn is_accessor(&self) -> bool {
self.is_accessor || self.method_kind.is_some_and(|kind| kind.is_accessor())
}

pub fn set_binding2(&mut self, binding: BoundIdentifier<'a>) {
self.binding2 = Some(binding);
}
}

Expand Down Expand Up @@ -101,46 +116,153 @@ impl<'a> ClassesStack<'a> {
self.stack.last_mut()
}

/// Lookup details of private property referred to by `ident`.
pub fn find_private_prop<'b>(
fn lookup_private_prop<
'b,
Ret,
RetFn: Fn(&'b PrivateProp<'a>, &'b mut ClassBindings<'a>, bool) -> Ret,
>(
&'b mut self,
ident: &PrivateIdentifier<'a>,
) -> ResolvedPrivateProp<'a, 'b> {
ret_fn: RetFn,
) -> Ret {
// Check for binding in closest class first, then enclosing classes.
// We skip the first, because this is a `NonEmptyStack` with dummy first entry.
// TODO: Check there are tests for bindings in enclosing classes.
for class in self.stack[1..].iter_mut().rev() {
if let Some(private_props) = &mut class.private_props {
if let Some(prop) = private_props.get(&ident.name) {
return ResolvedPrivateProp {
prop_binding: &prop.binding,
class_bindings: &mut class.bindings,
is_static: prop.is_static,
is_method: prop.is_method,
is_accessor: prop.is_accessor,
is_declaration: class.is_declaration,
};
return ret_fn(prop, &mut class.bindings, class.is_declaration);
}
}
}

unreachable!();
}

/// Lookup details of private property referred to by `ident`.
pub fn find_private_prop<'b>(
&'b mut self,
ident: &PrivateIdentifier<'a>,
) -> ResolvedPrivateProp<'a, 'b> {
self.lookup_private_prop(ident, move |prop, class_bindings, is_declaration| {
ResolvedPrivateProp {
prop_binding: &prop.binding,
class_bindings,
is_static: prop.is_static,
is_method: prop.is_method(),
is_accessor: prop.is_accessor(),
is_declaration,
}
})
}

/// Lookup details of readable private property referred to by `ident`.
pub fn find_readable_private_prop<'b>(
&'b mut self,
ident: &PrivateIdentifier<'a>,
) -> Option<ResolvedPrivateProp<'a, 'b>> {
self.lookup_private_prop(ident, move |prop, class_bindings, is_declaration| {
let prop_binding = if matches!(prop.method_kind, Some(MethodDefinitionKind::Set)) {
prop.binding2.as_ref()
} else {
Some(&prop.binding)
};
prop_binding.map(|prop_binding| ResolvedPrivateProp {
prop_binding,
class_bindings,
is_static: prop.is_static,
is_method: prop.is_method(),
is_accessor: prop.is_accessor(),
is_declaration,
})
})
}

/// Lookup details of writeable private property referred to by `ident`.
/// Returns `Some` if it refers to a private prop and setter method
pub fn find_writeable_private_prop<'b>(
&'b mut self,
ident: &PrivateIdentifier<'a>,
) -> Option<ResolvedPrivateProp<'a, 'b>> {
self.lookup_private_prop(ident, move |prop, class_bindings, is_declaration| {
let prop_binding = if matches!(prop.method_kind, Some(MethodDefinitionKind::Set) | None)
{
Some(&prop.binding)
} else {
prop.binding2.as_ref()
};
prop_binding.map(|prop_binding| ResolvedPrivateProp {
prop_binding,
class_bindings,
is_static: prop.is_static,
is_method: prop.is_method(),
is_accessor: prop.is_accessor(),
is_declaration,
})
})
}

/// Look up details of the private property referred to by ident and it can either be read or written.
pub fn find_get_set_private_prop<'b>(
&'b mut self,
ident: &PrivateIdentifier<'a>,
) -> ResolvedGetSetPrivateProp<'a, 'b> {
self.lookup_private_prop(ident, move |prop, class_bindings, is_declaration| {
let (get_binding, set_binding) = match prop.method_kind {
Some(MethodDefinitionKind::Set) => (prop.binding2.as_ref(), Some(&prop.binding)),
Some(_) => (Some(&prop.binding), prop.binding2.as_ref()),
_ => (Some(&prop.binding), Some(&prop.binding)),
};
ResolvedGetSetPrivateProp {
get_binding,
set_binding,
class_bindings,
is_static: prop.is_static,
is_method: prop.is_method(),
is_accessor: prop.is_accessor(),
is_declaration,
}
})
}
}

/// Details of a private property resolved for a private field.
///
/// This is the return value of [`ClassesStack::find_private_prop`].
/// This is the return value of [`ClassesStack::find_private_prop`],
/// [`ClassesStack::find_readable_private_prop`] and
/// [`ClassesStack::find_writeable_private_prop`].
pub(super) struct ResolvedPrivateProp<'a, 'b> {
/// Binding for temp var representing the property
pub prop_binding: &'b BoundIdentifier<'a>,
/// Bindings for class name and temp var for class
pub class_bindings: &'b mut ClassBindings<'a>,
/// `true` if is a static property
pub is_static: bool,
/// `true` if is a private method
/// `true` if is a private method or accessor property
pub is_method: bool,
/// `true` if is a private accessor property or [`PrivateProp::method_kind`] is
/// `Some(MethodDefinitionKind::Get)` or `Some(MethodDefinitionKind::Set)`
pub is_accessor: bool,
/// `true` if class which defines this property is a class declaration
pub is_declaration: bool,
}

/// Details of a private property resolved for a private field.
///
/// This is the return value of [`ClassesStack::find_get_set_private_prop`].
pub(super) struct ResolvedGetSetPrivateProp<'a, 'b> {
/// Binding for temp var representing the property or getter method
pub get_binding: Option<&'b BoundIdentifier<'a>>,
/// Binding for temp var representing the property or setter method
pub set_binding: Option<&'b BoundIdentifier<'a>>,
/// Bindings for class name and temp var for class
pub class_bindings: &'b mut ClassBindings<'a>,
/// `true` if is a static property
pub is_static: bool,
/// `true` if is a private method or accessor property
pub is_method: bool,
/// `true` if is a private accessor property
/// `true` if is a private accessor property or [`PrivateProp::method_kind`] is
/// `Some(MethodDefinitionKind::Get)` or `Some(MethodDefinitionKind::Set)`
#[expect(unused)]
pub is_accessor: bool,
/// `true` if class which defines this property is a class declaration
pub is_declaration: bool,
Expand Down
Loading

0 comments on commit ad77ad5

Please sign in to comment.