From 1061ad550a4f47470a13ccb4d393db26be26c172 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 7 Dec 2024 10:31:18 +0000 Subject: [PATCH] feat(transformer/class-properties): support `private_fields_as_properties` assumption --- .../src/common/helper_loader.rs | 4 + .../src/compiler_assumptions.rs | 1 - .../src/es2022/class_properties/class.rs | 156 ++++++++++++++---- .../src/es2022/class_properties/mod.rs | 6 + .../src/es2022/class_properties/private.rs | 23 ++- 5 files changed, 154 insertions(+), 36 deletions(-) diff --git a/crates/oxc_transformer/src/common/helper_loader.rs b/crates/oxc_transformer/src/common/helper_loader.rs index 536b5579e737dc..c93b2fc358890f 100644 --- a/crates/oxc_transformer/src/common/helper_loader.rs +++ b/crates/oxc_transformer/src/common/helper_loader.rs @@ -153,6 +153,8 @@ pub enum Helper { ClassPrivateFieldSet2, AssertClassBrand, ToSetter, + ClassPrivateFieldLooseKey, + ClassPrivateFieldLooseBase, } impl Helper { @@ -174,6 +176,8 @@ impl Helper { Self::ClassPrivateFieldSet2 => "classPrivateFieldSet2", Self::AssertClassBrand => "assertClassBrand", Self::ToSetter => "toSetter", + Self::ClassPrivateFieldLooseKey => "classPrivateFieldLooseKey", + Self::ClassPrivateFieldLooseBase => "classPrivateFieldLooseBase", } } } diff --git a/crates/oxc_transformer/src/compiler_assumptions.rs b/crates/oxc_transformer/src/compiler_assumptions.rs index 143b711d06642f..191d756ebfcb68 100644 --- a/crates/oxc_transformer/src/compiler_assumptions.rs +++ b/crates/oxc_transformer/src/compiler_assumptions.rs @@ -66,7 +66,6 @@ pub struct CompilerAssumptions { pub private_fields_as_symbols: bool, #[serde(default)] - #[deprecated = "Not Implemented"] pub private_fields_as_properties: bool, #[serde(default)] diff --git a/crates/oxc_transformer/src/es2022/class_properties/class.rs b/crates/oxc_transformer/src/es2022/class_properties/class.rs index 597d4afd5fbf65..f6a93c8705cf3d 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/class.rs @@ -110,7 +110,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { let private_props = self.private_props_stack.last(); if let Some(private_props) = private_props { let mut weakmap_symbol_id = None; - exprs.extend(private_props.props.values().filter_map(|prop| { + exprs.extend(private_props.props.iter().filter_map(|(name, prop)| { // Insert `var _prop;` declaration. // Do it here rather than when binding was created to maintain same order of `var` // declarations as Babel. `c = class C { #x = 1; static y = 2; }` -> `var _C, _x;` @@ -120,12 +120,9 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { return None; } - // `_prop = new WeakMap()` - Some(create_assignment( - &prop.binding, - create_new_weakmap(&mut weakmap_symbol_id, ctx), - ctx, - )) + // `_prop = new WeakMap()` / `_prop = _classPrivateFieldLooseKey("prop")` + let value = self.create_private_prop_key(name, &mut weakmap_symbol_id, ctx); + Some(create_assignment(&prop.binding, value, ctx)) })); } @@ -220,17 +217,14 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { let mut weakmap_symbol_id = None; self.ctx.statement_injector.insert_many_before( &stmt_address, - private_props.props.values().filter_map(|prop| { + private_props.props.iter().filter_map(|(name, prop)| { if prop.is_static { return None; } - // `var _prop = new WeakMap()` - Some(create_variable_declaration( - &prop.binding, - create_new_weakmap(&mut weakmap_symbol_id, ctx), - ctx, - )) + // `var _prop = new WeakMap();` / `var _prop = _classPrivateFieldLooseKey("prop");` + let value = self.create_private_prop_key(name, &mut weakmap_symbol_id, ctx); + Some(create_variable_declaration(&prop.binding, value, ctx)) }), ); } @@ -242,6 +236,48 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { } } + /// Create prop key for private property. + /// + /// Loose: `_classPrivateFieldLooseKey("prop")` + /// Not loose: `new WeakMap()` + /// + /// Takes an `&mut Option>` which is updated after looking up the binding for `WeakMap`. + /// * `None` = Not looked up yet. + /// * `Some(None)` = Has been looked up, and `WeakMap` is unbound. + /// * `Some(Some(symbol_id))` = Has been looked up, and `WeakMap` has a local binding. + /// + /// This is an optimization to avoid looking up the symbol for `WeakMap` over and over when defining + /// multiple private properties. + #[expect(clippy::option_option)] + fn create_private_prop_key( + &self, + name: &Atom<'a>, + weakmap_symbol_id: &mut Option>, + ctx: &mut TraverseCtx<'a>, + ) -> Expression<'a> { + if self.private_fields_as_properties { + // `_classPrivateFieldLooseKey("prop")` + self.ctx.helper_call_expr( + Helper::ClassPrivateFieldLooseKey, + SPAN, + ctx.ast.vec1(Argument::from(ctx.ast.expression_string_literal( + SPAN, + name.clone(), + None, + ))), + ctx, + ) + } else { + // `new WeakMap()` + let symbol_id = *weakmap_symbol_id.get_or_insert_with(|| { + ctx.scopes().find_binding(ctx.current_scope_id(), "WeakMap") + }); + let ident = + ctx.create_ident_expr(SPAN, Atom::from("WeakMap"), symbol_id, ReferenceFlags::Read); + ctx.ast.expression_new(SPAN, ident, ctx.ast.vec(), NONE) + } + } + /// Main guts of the transform. fn transform_class(&mut self, class: &mut Class<'a>, ctx: &mut TraverseCtx<'a>) { // TODO(improve-on-babel): If outer scope is sloppy mode, all code which is moved to outside @@ -600,12 +636,85 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { self.ctx.helper_call_expr(Helper::DefineProperty, SPAN, arguments, ctx) } - /// Create `_classPrivateFieldInitSpec(this, _prop, value)` to be inserted into class constructor. + /// Create init assignment for private instance prop, to be inserted into class constructor. + /// + /// Loose: `Object.defineProperty(this, _prop, {writable: true, value: value})` + /// Not loose: `_classPrivateFieldInitSpec(this, _prop, value)` fn create_private_instance_init_assignment( &mut self, ident: &PrivateIdentifier<'a>, value: Expression<'a>, ctx: &mut TraverseCtx<'a>, + ) -> Expression<'a> { + if self.private_fields_as_properties { + self.create_private_instance_init_assignment_loose(ident, value, ctx) + } else { + self.create_private_instance_init_assignment_not_loose(ident, value, ctx) + } + } + + /// `Object.defineProperty(this, _prop, {writable: true, value: value})` + fn create_private_instance_init_assignment_loose( + &mut self, + ident: &PrivateIdentifier<'a>, + value: Expression<'a>, + ctx: &mut TraverseCtx<'a>, + ) -> Expression<'a> { + // `Object.defineProperty` + let object_symbol_id = ctx.scopes().find_binding(ctx.current_scope_id(), "Object"); + let object = ctx.create_ident_expr( + SPAN, + Atom::from("Object"), + object_symbol_id, + ReferenceFlags::Read, + ); + let property = ctx.ast.identifier_name(SPAN, "defineProperty"); + let callee = + Expression::from(ctx.ast.member_expression_static(SPAN, object, property, false)); + + // `{writable: true, value: }` + let prop_def = ctx.ast.expression_object( + SPAN, + ctx.ast.vec_from_array([ + ctx.ast.object_property_kind_object_property( + SPAN, + PropertyKind::Init, + ctx.ast.property_key_identifier_name(SPAN, Atom::from("writable")), + ctx.ast.expression_boolean_literal(SPAN, true), + false, + false, + false, + ), + ctx.ast.object_property_kind_object_property( + SPAN, + PropertyKind::Init, + ctx.ast.property_key_identifier_name(SPAN, Atom::from("value")), + value, + false, + false, + false, + ), + ]), + None, + ); + + let private_props = self.private_props_stack.last().unwrap(); + let prop = &private_props.props[&ident.name]; + let arguments = ctx.ast.vec_from_array([ + Argument::from(ctx.ast.expression_this(SPAN)), + Argument::from(prop.binding.create_read_expression(ctx)), + Argument::from(prop_def), + ]); + // TODO: Should this have span of original `PropertyDefinition`? + ctx.ast.expression_call(SPAN, callee, NONE, arguments, false) + } + + /// `_classPrivateFieldInitSpec(this, _prop, value)` + fn create_private_instance_init_assignment_not_loose( + &mut self, + ident: &PrivateIdentifier<'a>, + value: Expression<'a>, + ctx: &mut TraverseCtx<'a>, ) -> Expression<'a> { let private_props = self.private_props_stack.last().unwrap(); let prop = &private_props.props[&ident.name]; @@ -784,20 +893,3 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { binding.create_read_expression(ctx) } } - -/// Create `new WeakMap()` expression. -/// -/// Takes an `&mut Option>` which is updated after looking up the binding for `WeakMap`. -/// * `None` = Not looked up yet. -/// * `Some(None)` = Has been looked up, and `WeakMap` is unbound. -/// * `Some(Some(symbol_id))` = Has been looked up, and `WeakMap` has a local binding. -#[expect(clippy::option_option)] -fn create_new_weakmap<'a>( - symbol_id: &mut Option>, - ctx: &mut TraverseCtx<'a>, -) -> Expression<'a> { - let symbol_id = *symbol_id - .get_or_insert_with(|| ctx.scopes().find_binding(ctx.current_scope_id(), "WeakMap")); - let ident = ctx.create_ident_expr(SPAN, Atom::from("WeakMap"), symbol_id, ReferenceFlags::Read); - ctx.ast.expression_new(SPAN, ident, ctx.ast.vec(), NONE) -} diff --git a/crates/oxc_transformer/src/es2022/class_properties/mod.rs b/crates/oxc_transformer/src/es2022/class_properties/mod.rs index 08f27dc1e9317b..2fb2b6f348ebec 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/mod.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/mod.rs @@ -181,6 +181,8 @@ pub struct ClassProperties<'a, 'ctx> { // /// If `true`, set properties with `=`, instead of `_defineProperty` helper. set_public_class_fields: bool, + /// If `true`, record private properties as string keys + private_fields_as_properties: bool, /// If `true`, transform static blocks. transform_static_blocks: bool, @@ -227,9 +229,13 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { ) -> Self { // TODO: Raise error if these 2 options are inconsistent let set_public_class_fields = options.loose || ctx.assumptions.set_public_class_fields; + // TODO: Raise error if these 2 options are inconsistent + let private_fields_as_properties = + options.loose || ctx.assumptions.private_fields_as_properties; Self { set_public_class_fields, + private_fields_as_properties, transform_static_blocks, ctx, private_props_stack: PrivatePropsStack::default(), diff --git a/crates/oxc_transformer/src/es2022/class_properties/private.rs b/crates/oxc_transformer/src/es2022/class_properties/private.rs index 23347b684b5c53..ebbb73a1c3ceed 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/private.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/private.rs @@ -28,8 +28,11 @@ use super::{ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { /// Transform private field expression. /// - /// Instance prop: `object.#prop` -> `_classPrivateFieldGet2(_prop, object)` - /// Static prop: `object.#prop` -> `_assertClassBrand(Class, object, _prop)._` + /// Loose: `object.#prop` -> `_classPrivateFieldLooseBase(object, _prop)[_prop]` + /// + /// Not loose: + /// * Instance prop: `object.#prop` -> `_classPrivateFieldGet2(_prop, object)` + /// * Static prop: `object.#prop` -> `_assertClassBrand(Class, object, _prop)._` // // `#[inline]` so that compiler sees that `expr` is an `Expression::PrivateFieldExpression`. #[inline] @@ -61,7 +64,21 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> { // TODO: Move this to top of function once `lookup_private_property` does not return `Option` let PrivateFieldExpression { span, object, .. } = field_expr.unbox(); - if is_static { + if self.private_fields_as_properties { + // `_classPrivateFieldLooseBase(object, _prop)[_prop]` + let call_expr = self.ctx.helper_call_expr( + Helper::ClassPrivateFieldLooseBase, + SPAN, + ctx.ast.vec_from_array([Argument::from(object), Argument::from(prop_ident)]), + ctx, + ); + Expression::from(ctx.ast.member_expression_computed( + span, + call_expr, + prop_binding.create_read_expression(ctx), + false, + )) + } else if is_static { // TODO: Ensure there are tests for nested classes with references to private static props // of outer class inside inner class, to make sure we're getting the right `class_bindings`.