From 5089ddff9d81c85df32b2bd7d9b3e3b998b31e52 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Fri, 18 Apr 2025 14:55:33 +0800 Subject: [PATCH 1/2] fix(transformer/class-properties): align setPublicClassFields with TypeScript's useDefineForClassFields --- .../src/es2022/class_properties/class.rs | 6 ++++ .../src/es2022/class_properties/prop_decl.rs | 34 ++++++++++++++----- .../output.js | 1 - .../public-loose/instance-undefined/output.js | 6 +--- .../public-loose/preserve-comments/output.js | 7 +--- .../public-loose/static-undefined/output.js | 3 +- .../snapshots/babel_exec.snap.md | 6 +++- 7 files changed, 40 insertions(+), 23 deletions(-) diff --git a/crates/oxc_transformer/src/es2022/class_properties/class.rs b/crates/oxc_transformer/src/es2022/class_properties/class.rs index aa81f38ec2261..412b85b06cef8 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/class.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/class.rs @@ -276,6 +276,12 @@ impl<'a> ClassProperties<'a, '_> { } } + // When `self.set_public_class_fields` is true, we don't need to convert instance properties + // without initializers, that means `instance_prop_count != 0` but `instance_inits` may be empty. + if instance_inits.is_empty() { + return; + } + // Scope that instance property initializers will be inserted into. // This is usually class constructor, but can also be a `_super` function which is created. let instance_inits_scope_id; diff --git a/crates/oxc_transformer/src/es2022/class_properties/prop_decl.rs b/crates/oxc_transformer/src/es2022/class_properties/prop_decl.rs index 7ea12a4458983..7cd8f5422b5fd 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/prop_decl.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/prop_decl.rs @@ -24,11 +24,20 @@ impl<'a> ClassProperties<'a, '_> { ctx: &mut TraverseCtx<'a>, ) { // Get value - let value = prop.value.take().unwrap_or_else(|| ctx.ast.void_0(SPAN)); + let value = prop.value.take(); let init_expr = if let PropertyKey::PrivateIdentifier(ident) = &mut prop.key { + let value = value.unwrap_or_else(|| ctx.ast.void_0(SPAN)); self.create_private_instance_init_assignment(ident, value, ctx) } else { + let value = match value { + Some(value) => value, + // Do not need to convert property to `assignee.prop = void 0` if no initializer exists. + // This is to align `TypeScript` with `useDefineForClassFields: false`. + None if self.private_fields_as_properties => return, + None => ctx.ast.void_0(SPAN), + }; + // Convert to assignment or `_defineProperty` call, depending on `loose` option let this = ctx.ast.expression_this(SPAN); self.create_init_assignment(prop, value, this, false, ctx) @@ -85,17 +94,23 @@ impl<'a> ClassProperties<'a, '_> { // Get value. // Transform it to replace `this` and references to class name with temp var for class. // Also transform `super`. - let value = match prop.value.take() { - Some(mut value) => { - self.transform_static_initializer(&mut value, ctx); - value - } - None => ctx.ast.void_0(SPAN), - }; + let value = prop.value.take().map(|mut value| { + self.transform_static_initializer(&mut value, ctx); + value + }); if let PropertyKey::PrivateIdentifier(ident) = &mut prop.key { + let value = value.unwrap_or_else(|| ctx.ast.void_0(SPAN)); self.insert_private_static_init_assignment(ident, value, ctx); } else { + let value = match value { + Some(value) => value, + // Do not need to convert property to `assignee.prop = void 0` if no initializer exists. + // This is to align `TypeScript` with `useDefineForClassFields: false`. + None if self.private_fields_as_properties => return, + None => ctx.ast.void_0(SPAN), + }; + // Convert to assignment or `_defineProperty` call, depending on `loose` option let class_details = self.current_class(); let class_binding = if class_details.is_declaration { @@ -200,6 +215,9 @@ impl<'a> ClassProperties<'a, '_> { // Used for both instance and static properties impl<'a> ClassProperties<'a, '_> { /// `assignee.prop = value` or `_defineProperty(assignee, "prop", value)` + /// `#[inline]` because the caller has been checked `self.set_public_class_fields`. + /// After inlining, the two `self.set_public_class_fields` checks may be folded into one. + #[inline] fn create_init_assignment( &mut self, prop: &mut PropertyDefinition<'a>, diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/compile-to-class/constructor-collision-ignores-types-loose/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/compile-to-class/constructor-collision-ignores-types-loose/output.js index 64cb70e748e64..846221d7dea34 100644 --- a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/compile-to-class/constructor-collision-ignores-types-loose/output.js +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/compile-to-class/constructor-collision-ignores-types-loose/output.js @@ -1,6 +1,5 @@ class C { constructor(T) { - this.x = void 0; this.y = 0; } } diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/instance-undefined/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/instance-undefined/output.js index 5da5e4cd518d5..2462eedcd30ab 100644 --- a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/instance-undefined/output.js +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/instance-undefined/output.js @@ -1,5 +1 @@ -class Foo { - constructor() { - this.bar = void 0; - } -} +class Foo { } diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/preserve-comments/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/preserve-comments/output.js index 91543cf29c8a9..b0a84db03d1e9 100644 --- a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/preserve-comments/output.js +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/preserve-comments/output.js @@ -1,6 +1 @@ -class C { - constructor() { - this.a = void 0; - } -} -C.b = void 0; +class C { } diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js index ebc7c89b43f1c..f72abf33fa2a1 100644 --- a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js @@ -1,2 +1 @@ -class Foo {} -Foo.bar = void 0; +class Foo {} \ No newline at end of file diff --git a/tasks/transform_conformance/snapshots/babel_exec.snap.md b/tasks/transform_conformance/snapshots/babel_exec.snap.md index 73e9207a9dfb1..bd96f4fa66f00 100644 --- a/tasks/transform_conformance/snapshots/babel_exec.snap.md +++ b/tasks/transform_conformance/snapshots/babel_exec.snap.md @@ -2,7 +2,7 @@ commit: 578ac4df node: v22.14.0 -Passed: 320 of 406 (78.82%) +Passed: 319 of 406 (78.57%) Failures: @@ -58,6 +58,10 @@ AssertionError: expected function to throw an error, but it didn't AssertionError: expected '_Class' to be 'Foo' // Object.is equality at ./tasks/transform_conformance/fixtures/babel/babel-plugin-transform-class-properties-test-fixtures-public-loose-static-infer-name-exec.test.js:8:19 +./fixtures/babel/babel-plugin-transform-class-properties-test-fixtures-public-loose-static-undefined-exec.test.js +AssertionError: expected false to be true // Object.is equality + at ./tasks/transform_conformance/fixtures/babel/babel-plugin-transform-class-properties-test-fixtures-public-loose-static-undefined-exec.test.js:5:23 + ./fixtures/babel/babel-plugin-transform-class-properties-test-fixtures-public-static-infer-name-exec.test.js AssertionError: expected '_Class' to be 'Foo' // Object.is equality at ./tasks/transform_conformance/fixtures/babel/babel-plugin-transform-class-properties-test-fixtures-public-static-infer-name-exec.test.js:9:19 From b9f2d6347bbe4d3f9a6c1497155325c361be739d Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Tue, 22 Apr 2025 12:23:30 +0100 Subject: [PATCH 2/2] Add line break to end of test fixture --- .../test/fixtures/public-loose/static-undefined/output.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js index f72abf33fa2a1..4e6a6de65314d 100644 --- a/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js +++ b/tasks/transform_conformance/overrides/babel-plugin-transform-class-properties/test/fixtures/public-loose/static-undefined/output.js @@ -1 +1 @@ -class Foo {} \ No newline at end of file +class Foo {}