Fix TSParameterProperty getting lost with transform-classes (#8682)

packages/babel-plugin-transform-typescript/src/index.js

@@ -20,6 +20,8 @@ interface State {
   programPath: any;
 }
 
+const PARSED_PARAMS = new WeakSet();
+
 export default declare((api, { jsxPragma = "React" }) => {
   api.assertVersion(7);
 
@@ -96,58 +98,6 @@ export default declare((api, { jsxPragma = "React" }) => {
         if (node.abstract) node.abstract = null;
         if (node.optional) node.optional = null;
 
-        if (node.kind !== "constructor") {
-          return;
-        }
-
-        // Collect parameter properties
-        const parameterProperties = [];
-        for (const param of node.params) {
-          if (param.type === "TSParameterProperty") {
-            parameterProperties.push(param.parameter);
-          }
-        }
-
-        if (!parameterProperties.length) {
-          return;
-        }
-
-        const assigns = parameterProperties.map(p => {
-          let name;
-          if (t.isIdentifier(p)) {
-            name = p.name;
-          } else if (t.isAssignmentPattern(p) && t.isIdentifier(p.left)) {
-            name = p.left.name;
-          } else {
-            throw path.buildCodeFrameError(
-              "Parameter properties can not be destructuring patterns.",
-            );
-          }
-
-          const assign = t.assignmentExpression(
-            "=",
-            t.memberExpression(t.thisExpression(), t.identifier(name)),
-            t.identifier(name),
-          );
-          return t.expressionStatement(assign);
-        });
-
-        const statements = node.body.body;
-
-        const first = statements[0];
-        const startsWithSuperCall =
-          first !== undefined &&
-          t.isExpressionStatement(first) &&
-          t.isCallExpression(first.expression) &&
-          t.isSuper(first.expression.callee);
-
-        // Make sure to put parameter properties *after* the `super` call.
-        // TypeScript will enforce that a 'super()' call is the first statement
-        // when there are parameter properties.
-        node.body.body = startsWithSuperCall
-          ? [first, ...assigns, ...statements.slice(1)]
-          : [...assigns, ...statements];
-
         // Rest handled by Function visitor
       },
 
@@ -182,14 +132,74 @@ export default declare((api, { jsxPragma = "React" }) => {
         if (node.superTypeParameters) node.superTypeParameters = null;
         if (node.implements) node.implements = null;
 
-        // Same logic is used in babel-plugin-transform-flow-strip-types:
-        // We do this here instead of in a `ClassProperty` visitor because the class transform
-        // would transform the class before we reached the class property.
+        // Similar to the logic in `transform-flow-strip-types`, we need to
+        // handle `TSParameterProperty` and `ClassProperty` here because the
+        // class transform would transform the class, causing more specific
+        // visitors to not run.
         path.get("body.body").forEach(child => {
-          if (child.isClassProperty()) {
-            child.node.typeAnnotation = null;
+          const childNode = child.node;
+
+          if (t.isClassMethod(childNode, { kind: "constructor" })) {
+            // Collects parameter properties so that we can add an assignment
+            // for each of them in the constructor body
+            //
+            // We use a WeakSet to ensure an assignment for a parameter
+            // property is only added once. This is necessary for cases like
+            // using `transform-classes`, which causes this visitor to run
+            // twice.
+            const parameterProperties = [];
+            for (const param of childNode.params) {
+              if (
+                param.type === "TSParameterProperty" &&
+                !PARSED_PARAMS.has(param.parameter)
+              ) {
+                PARSED_PARAMS.add(param.parameter);
+                parameterProperties.push(param.parameter);
+              }
+            }
+
+            if (parameterProperties.length) {
+              const assigns = parameterProperties.map(p => {
+                let name;
+                if (t.isIdentifier(p)) {
+                  name = p.name;
+                } else if (t.isAssignmentPattern(p) && t.isIdentifier(p.left)) {
+                  name = p.left.name;
+                } else {
+                  throw path.buildCodeFrameError(
+                    "Parameter properties can not be destructuring patterns.",
+                  );
+                }
+
+                const assign = t.assignmentExpression(
+                  "=",
+                  t.memberExpression(t.thisExpression(), t.identifier(name)),
+                  t.identifier(name),
+                );
+                return t.expressionStatement(assign);
+              });
+
+              const statements = childNode.body.body;
+
+              const first = statements[0];
+
+              const startsWithSuperCall =
+                first !== undefined &&
+                t.isExpressionStatement(first) &&
+                t.isCallExpression(first.expression) &&
+                t.isSuper(first.expression.callee);
+
+              // Make sure to put parameter properties *after* the `super`
+              // call. TypeScript will enforce that a 'super()' call is the
+              // first statement when there are parameter properties.
+              childNode.body.body = startsWithSuperCall
+                ? [first, ...assigns, ...statements.slice(1)]
+                : [...assigns, ...statements];
+            }
+          } else if (child.isClassProperty()) {
+            childNode.typeAnnotation = null;
 
-            if (!child.node.value && !child.node.decorators) {
+            if (!childNode.value && !childNode.decorators) {
               child.remove();
             }
           }

packages/babel-plugin-transform-typescript/test/fixtures/class/parameter-properties-with-class-and-super/input.js

@@ -0,0 +1,5 @@
+class Employee extends Person {
+  constructor(public name: string) {
+    super();
+  }
+}

packages/babel-plugin-transform-typescript/test/fixtures/class/parameter-properties-with-class-and-super/options.json

@@ -0,0 +1,3 @@
+{
+  "plugins": ["transform-typescript", "transform-classes"]
+}

packages/babel-plugin-transform-typescript/test/fixtures/class/parameter-properties-with-class-and-super/output.js

@@ -0,0 +1,31 @@
+function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }
+
+function _possibleConstructorReturn(self, call) { if (call && (typeof call === "object" || typeof call === "function")) { return call; } return _assertThisInitialized(self); }
+
+function _assertThisInitialized(self) { if (self === void 0) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return self; }
+
+function _getPrototypeOf(o) { _getPrototypeOf = Object.setPrototypeOf ? Object.getPrototypeOf : function _getPrototypeOf(o) { return o.__proto__ || Object.getPrototypeOf(o); }; return _getPrototypeOf(o); }
+
+function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function"); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, writable: true, configurable: true } }); if (superClass) _setPrototypeOf(subClass, superClass); }
+
+function _setPrototypeOf(o, p) { _setPrototypeOf = Object.setPrototypeOf || function _setPrototypeOf(o, p) { o.__proto__ = p; return o; }; return _setPrototypeOf(o, p); }
+
+let Employee =
+/*#__PURE__*/
+function (_Person) {
+  "use strict";
+
+  _inherits(Employee, _Person);
+
+  function Employee(name) {
+    var _this;
+
+    _classCallCheck(this, Employee);
+
+    _this = _possibleConstructorReturn(this, _getPrototypeOf(Employee).call(this));
+    _this.name = name;
+    return _this;
+  }
+
+  return Employee;
+}(Person);

packages/babel-plugin-transform-typescript/test/fixtures/class/parameter-properties-with-class/input.js

@@ -0,0 +1,3 @@
+class Person {
+  constructor(public name: string) {}
+}

packages/babel-plugin-transform-typescript/test/fixtures/class/parameter-properties-with-class/options.json

@@ -0,0 +1,3 @@
+{
+  "plugins": ["transform-typescript", "transform-classes"]
+}

packages/babel-plugin-transform-typescript/test/fixtures/class/parameter-properties-with-class/output.js

@@ -0,0 +1,15 @@
+function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }
+
+let Person =
+/*#__PURE__*/
+function () {
+  "use strict";
+
+  function Person(name) {
+    _classCallCheck(this, Person);
+
+    this.name = name;
+  }
+
+  return Person;
+}();