Simplify class fields injetion after super() (#16123)

* Add test * Simplify class fields injetion after `super()`
babel · Nov 25, 2023 · 01a0767 · 01a0767
1 parent c5f0a56
commit 01a0767
Show file tree

Hide file tree

Showing 13 changed files with 64 additions and 24 deletions.
diff --git a/packages/babel-helper-create-class-features-plugin/src/decorators.ts b/packages/babel-helper-create-class-features-plugin/src/decorators.ts
@@ -213,7 +213,11 @@ export function buildDecoratedClass(
   }
 
   return {
-    instanceNodes: [template.statement.ast`${t.cloneNode(initializeId)}(this)`],
+    instanceNodes: [
+      template.statement.ast`
+        ${t.cloneNode(initializeId)}(this)
+      ` as t.ExpressionStatement,
+    ],
     wrapClass(path: NodePath<t.Class>) {
       path.replaceWith(replacement);
       return path.get(classPathDesc) as NodePath;

diff --git a/packages/babel-helper-create-class-features-plugin/src/fields.ts b/packages/babel-helper-create-class-features-plugin/src/fields.ts
@@ -585,7 +585,7 @@ function buildPrivateFieldInitLoose(
         writable: true,
         value: ${value}
       });
-    `,
+    ` as t.ExpressionStatement,
     prop,
   );
 }
@@ -607,7 +607,7 @@ function buildPrivateInstanceFieldInitSpec(
           // enumerable is always false for private elements
           writable: true,
           value: ${value},
-        })`,
+        })` as t.ExpressionStatement,
         prop,
       );
     }
@@ -622,7 +622,7 @@ function buildPrivateInstanceFieldInitSpec(
         writable: true,
         value: ${value}
       },
-    )`,
+    )` as t.ExpressionStatement,
     prop,
   );
 }
@@ -689,7 +689,7 @@ function buildPrivateMethodInitLoose(
           // writable is false by default
           value: ${methodId.name}
         });
-      `,
+      ` as t.ExpressionStatement,
       prop,
     );
   }
@@ -709,7 +709,7 @@ function buildPrivateMethodInitLoose(
           get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
           set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
         });
-      `,
+      ` as t.ExpressionStatement,
       prop,
     );
   }
@@ -766,7 +766,7 @@ function buildPrivateAccessorInitialization(
             get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
             set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
           });
-        `,
+        ` as t.ExpressionStatement,
         prop,
       );
     }
@@ -781,7 +781,7 @@ function buildPrivateAccessorInitialization(
         get: ${getId ? getId.name : prop.scope.buildUndefinedNode()},
         set: ${setId ? setId.name : prop.scope.buildUndefinedNode()}
       },
-    )`,
+    )` as t.ExpressionStatement,
     prop,
   );
 }
@@ -798,7 +798,7 @@ function buildPrivateInstanceMethodInitialization(
   if (!process.env.BABEL_8_BREAKING) {
     if (!state.availableHelper("classPrivateMethodInitSpec")) {
       return inheritPropComments(
-        template.statement.ast`${id}.add(${ref})`,
+        template.statement.ast`${id}.add(${ref})` as t.ExpressionStatement,
         prop,
       );
     }
@@ -809,7 +809,7 @@ function buildPrivateInstanceMethodInitialization(
     template.statement.ast`${helper}(
       ${t.thisExpression()},
       ${t.cloneNode(id)}
-    )`,
+    )` as t.ExpressionStatement,
     prop,
   );
 }
@@ -1095,7 +1095,8 @@ export function buildFieldsInitNodes(
   let classRefFlags = ClassRefFlag.None;
   let injectSuperRef: t.Identifier;
   const staticNodes: t.Statement[] = [];
-  const instanceNodes: t.Statement[] = [];
+  const instanceNodes: t.ExpressionStatement[] = [];
+  let lastInstanceNodeReturnsThis = false;
   // These nodes are pure and can be moved to the closest statement position
   const pureStaticNodes: t.FunctionDeclaration[] = [];
   let classBindingNode: t.ExpressionStatement | null = null;
@@ -1156,6 +1157,8 @@ export function buildFieldsInitNodes(
       }
     }
 
+    lastInstanceNodeReturnsThis = false;
+
     // TODO(ts): there are so many `ts-expect-error` inside cases since
     // ts can not infer type from pre-computed values (or a case test)
     // even change `isStaticBlock` to `t.isStaticBlock(prop)` will not make prop
@@ -1301,6 +1304,7 @@ export function buildFieldsInitNodes(
         instanceNodes.push(buildPublicFieldInitLoose(t.thisExpression(), prop));
         break;
       case isInstance && isPublic && isField && !setPublicClassFields:
+        lastInstanceNodeReturnsThis = true;
         instanceNodes.push(
           // @ts-expect-error checked in switch
           buildPublicFieldInitSpec(t.thisExpression(), prop, file),
@@ -1324,6 +1328,7 @@ export function buildFieldsInitNodes(
   return {
     staticNodes: staticNodes.filter(Boolean),
     instanceNodes: instanceNodes.filter(Boolean),
+    lastInstanceNodeReturnsThis,
     pureStaticNodes: pureStaticNodes.filter(Boolean),
     classBindingNode,
     wrapClass(path: NodePath<t.Class>) {

diff --git a/packages/babel-helper-create-class-features-plugin/src/index.ts b/packages/babel-helper-create-class-features-plugin/src/index.ts
@@ -238,7 +238,8 @@ export function createClassFeaturePlugin({
 
         let keysNodes: t.Statement[],
           staticNodes: t.Statement[],
-          instanceNodes: t.Statement[],
+          instanceNodes: t.ExpressionStatement[],
+          lastInstanceNodeReturnsThis: boolean,
           pureStaticNodes: t.FunctionDeclaration[],
           classBindingNode: t.Statement | null,
           wrapClass: (path: NodePath<t.Class>) => NodePath;
@@ -258,6 +259,7 @@ export function createClassFeaturePlugin({
               staticNodes,
               pureStaticNodes,
               instanceNodes,
+              lastInstanceNodeReturnsThis,
               classBindingNode,
               wrapClass,
             } = buildFieldsInitNodes(
@@ -278,6 +280,7 @@ export function createClassFeaturePlugin({
             staticNodes,
             pureStaticNodes,
             instanceNodes,
+            lastInstanceNodeReturnsThis,
             classBindingNode,
             wrapClass,
           } = buildFieldsInitNodes(
@@ -308,6 +311,7 @@ export function createClassFeaturePlugin({
                 prop.traverse(referenceVisitor, state);
               }
             },
+            lastInstanceNodeReturnsThis,
           );
         }
 

diff --git a/packages/babel-helper-create-class-features-plugin/src/misc.ts b/packages/babel-helper-create-class-features-plugin/src/misc.ts
@@ -64,8 +64,9 @@ interface RenamerState {
 export function injectInitialization(
   path: NodePath<t.Class>,
   constructor: NodePath<t.ClassMethod> | undefined,
-  nodes: t.Statement[],
+  nodes: t.ExpressionStatement[],
   renamer?: (visitor: Visitor<RenamerState>, state: RenamerState) => void,
+  lastReturnsThis?: boolean,
 ) {
   if (!nodes.length) return;
 
@@ -99,10 +100,19 @@ export function injectInitialization(
     let isFirst = true;
     for (const bareSuper of bareSupers) {
       if (isFirst) {
-        bareSuper.insertAfter(nodes);
         isFirst = false;
       } else {
-        bareSuper.insertAfter(nodes.map(n => t.cloneNode(n)));
+        nodes = nodes.map(n => t.cloneNode(n));
+      }
+      if (!bareSuper.parentPath.isExpressionStatement()) {
+        const allNodes: t.Expression[] = [
+          bareSuper.node,
+          ...nodes.map(n => t.toExpression(n)),
+        ];
+        if (!lastReturnsThis) allNodes.push(t.thisExpression());
+        bareSuper.replaceWith(t.sequenceExpression(allNodes));
+      } else {
+        bareSuper.insertAfter(nodes);
       }
     }
   } else {

diff --git a/...m-class-properties/test/fixtures/public/derived-super-in-default-params-complex/output.js b/...m-class-properties/test/fixtures/public/derived-super-in-default-params-complex/output.js
@@ -1,3 +1,3 @@
 class Foo extends Bar {
-  constructor(x = test ? (() => (super(), babelHelpers.defineProperty(this, "bar", "foo"), this))() : 0) {}
+  constructor(x = test ? (super(), babelHelpers.defineProperty(this, "bar", "foo")) : 0) {}
 }
diff --git a/...-class-properties/test/fixtures/public/derived-super-in-default-params-in-arrow/output.js b/...-class-properties/test/fixtures/public/derived-super-in-default-params-in-arrow/output.js
@@ -1,5 +1,5 @@
 class Foo extends Bar {
   constructor(x = () => {
-    check((super(), babelHelpers.defineProperty(this, "bar", "foo"), this));
+    check((super(), babelHelpers.defineProperty(this, "bar", "foo")));
   }) {}
 }
diff --git a/...transform-class-properties/test/fixtures/public/derived-super-in-default-params/output.js b/...transform-class-properties/test/fixtures/public/derived-super-in-default-params/output.js
@@ -1,3 +1,3 @@
 class Foo extends Bar {
-  constructor(x = (() => (super(), babelHelpers.defineProperty(this, "bar", "foo"), this))()) {}
+  constructor(x = (super(), babelHelpers.defineProperty(this, "bar", "foo"))) {}
 }
diff --git a/...s/babel-plugin-transform-class-properties/test/fixtures/public/super-expression/output.js b/...s/babel-plugin-transform-class-properties/test/fixtures/public/super-expression/output.js
@@ -6,7 +6,7 @@ var Foo = /*#__PURE__*/function (_Bar) {
   function Foo() {
     var _this;
     babelHelpers.classCallCheck(this, Foo);
-    foo((_this = _super.call(this), babelHelpers.defineProperty(babelHelpers.assertThisInitialized(_this), "bar", "foo"), babelHelpers.assertThisInitialized(_this)));
+    foo((_this = _super.call(this), babelHelpers.defineProperty(babelHelpers.assertThisInitialized(_this), "bar", "foo")));
     return _this;
   }
   return babelHelpers.createClass(Foo);

diff --git a/packages/babel-plugin-transform-class-properties/test/fixtures/regression/7371/output.js b/packages/babel-plugin-transform-class-properties/test/fixtures/regression/7371/output.js
@@ -45,7 +45,7 @@ class ComputedMethod extends Obj {
         super();
         expect(this.field).toBeUndefined();
       }
-      [(super(), babelHelpers.defineProperty(this, "field", 1), this)]() {}
+      [(super(), babelHelpers.defineProperty(this, "field", 1))]() {}
     }
     expect(this.field).toBe(1);
     new B();
@@ -57,7 +57,7 @@ new ComputedMethod();
 class ComputedField extends Obj {
   constructor() {
     let _ref;
-    _ref = (super(), babelHelpers.defineProperty(this, "field", 1), this);
+    _ref = (super(), babelHelpers.defineProperty(this, "field", 1));
     class B extends Obj {
       constructor() {
         super();

diff --git a/...in-transform-class-properties/test/fixtures/regression/multiple-super-in-termary/input.js b/...in-transform-class-properties/test/fixtures/regression/multiple-super-in-termary/input.js
@@ -0,0 +1,6 @@
+class A extends B {
+  x = 2;
+  constructor() {
+    x ? super(a) : super(b)
+  }
+}
diff --git a/...ransform-class-properties/test/fixtures/regression/multiple-super-in-termary/options.json b/...ransform-class-properties/test/fixtures/regression/multiple-super-in-termary/options.json
@@ -0,0 +1,3 @@
+{
+  "plugins": ["transform-class-properties"]
+}
diff --git a/...n-transform-class-properties/test/fixtures/regression/multiple-super-in-termary/output.js b/...n-transform-class-properties/test/fixtures/regression/multiple-super-in-termary/output.js
@@ -0,0 +1,5 @@
+class A extends B {
+  constructor() {
+    x ? (super(a), babelHelpers.defineProperty(this, "x", 2)) : (super(b), babelHelpers.defineProperty(this, "x", 2));
+  }
+}
diff --git a/packages/babel-plugin-transform-typescript/src/index.ts b/packages/babel-plugin-transform-typescript/src/index.ts
@@ -205,7 +205,7 @@ export default declare((api, opts: Options) => {
       // property is only added once. This is necessary for cases like
       // using `transform-classes`, which causes this visitor to run
       // twice.
-      const assigns = [];
+      const assigns: t.ExpressionStatement[] = [];
       const { scope } = path;
       for (const paramPath of path.get("params")) {
         const param = paramPath.node;
@@ -226,8 +226,11 @@ export default declare((api, opts: Options) => {
               "Parameter properties can not be destructuring patterns.",
             );
           }
-          assigns.push(template.statement.ast`
-          this.${t.cloneNode(id)} = ${t.cloneNode(id)}`);
+          assigns.push(
+            template.statement.ast`
+              this.${t.cloneNode(id)} = ${t.cloneNode(id)}
+            ` as t.ExpressionStatement,
+          );
 
           paramPath.replaceWith(paramPath.get("parameter"));
           scope.registerBinding("param", paramPath);