Define class elements in the correct order (#12723)

* Define class elements in the correct order * Object.gOPDescriptors is not available on Node.js 6 * Handle numeric keys * Update test * Update fixtures
babel · Feb 2, 2021 · 60ef190 · 60ef190
1 parent 20664a4
commit 60ef190
Show file tree

Hide file tree

Showing 21 changed files with 272 additions and 103 deletions.
diff --git a/...es/babel-plugin-proposal-class-properties/test/fixtures/private-loose/canonical/output.js b/...es/babel-plugin-proposal-class-properties/test/fixtures/private-loose/canonical/output.js
@@ -20,16 +20,6 @@ var Point = /*#__PURE__*/function () {
   }
 
   babelHelpers.createClass(Point, [{
-    key: "equals",
-    value: function equals(p) {
-      return babelHelpers.classPrivateFieldLooseBase(this, _x)[_x] === babelHelpers.classPrivateFieldLooseBase(p, _x)[_x] && babelHelpers.classPrivateFieldLooseBase(this, _y)[_y] === babelHelpers.classPrivateFieldLooseBase(p, _y)[_y];
-    }
-  }, {
-    key: "toString",
-    value: function toString() {
-      return `Point<${babelHelpers.classPrivateFieldLooseBase(this, _x)[_x]},${babelHelpers.classPrivateFieldLooseBase(this, _y)[_y]}>`;
-    }
-  }, {
     key: "x",
     get: function () {
       return babelHelpers.classPrivateFieldLooseBase(this, _x)[_x];
@@ -45,6 +35,16 @@ var Point = /*#__PURE__*/function () {
     set: function (value) {
       babelHelpers.classPrivateFieldLooseBase(this, _y)[_y] = +value;
     }
+  }, {
+    key: "equals",
+    value: function equals(p) {
+      return babelHelpers.classPrivateFieldLooseBase(this, _x)[_x] === babelHelpers.classPrivateFieldLooseBase(p, _x)[_x] && babelHelpers.classPrivateFieldLooseBase(this, _y)[_y] === babelHelpers.classPrivateFieldLooseBase(p, _y)[_y];
+    }
+  }, {
+    key: "toString",
+    value: function toString() {
+      return `Point<${babelHelpers.classPrivateFieldLooseBase(this, _x)[_x]},${babelHelpers.classPrivateFieldLooseBase(this, _y)[_y]}>`;
+    }
   }]);
   return Point;
 }();
diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/private/canonical/output.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/private/canonical/output.js
@@ -23,16 +23,6 @@ var Point = /*#__PURE__*/function () {
   }
 
   babelHelpers.createClass(Point, [{
-    key: "equals",
-    value: function equals(p) {
-      return babelHelpers.classPrivateFieldGet(this, _x) === babelHelpers.classPrivateFieldGet(p, _x) && babelHelpers.classPrivateFieldGet(this, _y) === babelHelpers.classPrivateFieldGet(p, _y);
-    }
-  }, {
-    key: "toString",
-    value: function toString() {
-      return `Point<${babelHelpers.classPrivateFieldGet(this, _x)},${babelHelpers.classPrivateFieldGet(this, _y)}>`;
-    }
-  }, {
     key: "x",
     get: function () {
       return babelHelpers.classPrivateFieldGet(this, _x);
@@ -48,6 +38,16 @@ var Point = /*#__PURE__*/function () {
     set: function (value) {
       babelHelpers.classPrivateFieldSet(this, _y, +value);
     }
+  }, {
+    key: "equals",
+    value: function equals(p) {
+      return babelHelpers.classPrivateFieldGet(this, _x) === babelHelpers.classPrivateFieldGet(p, _x) && babelHelpers.classPrivateFieldGet(this, _y) === babelHelpers.classPrivateFieldGet(p, _y);
+    }
+  }, {
+    key: "toString",
+    value: function toString() {
+      return `Point<${babelHelpers.classPrivateFieldGet(this, _x)},${babelHelpers.classPrivateFieldGet(this, _y)}>`;
+    }
   }]);
   return Point;
 }();
diff --git a/...ages/babel-plugin-proposal-class-properties/test/fixtures/public-loose/computed/output.js b/...ages/babel-plugin-proposal-class-properties/test/fixtures/public-loose/computed/output.js
@@ -32,9 +32,6 @@ var MyClass = /*#__PURE__*/function () {
   }
 
   babelHelpers.createClass(MyClass, [{
-    key: _ref2,
-    value: function () {}
-  }, {
     key: "whatever",
     get: function () {},
     set: function (value) {}
@@ -44,9 +41,12 @@ var MyClass = /*#__PURE__*/function () {
   }, {
     key: _computed2,
     set: function (value) {}
-  }], [{
-    key: 10,
+  }, {
+    key: _ref2,
     value: function () {}
+  }], [{
+    key: "10",
+    value: function _() {}
   }]);
   return MyClass;
 }();

diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/public/computed/output.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/public/computed/output.js
@@ -32,9 +32,6 @@ var MyClass = /*#__PURE__*/function () {
   }
 
   babelHelpers.createClass(MyClass, [{
-    key: _ref2,
-    value: function () {}
-  }, {
     key: "whatever",
     get: function () {},
     set: function (value) {}
@@ -44,9 +41,12 @@ var MyClass = /*#__PURE__*/function () {
   }, {
     key: _computed2,
     set: function (value) {}
-  }], [{
-    key: 10,
+  }, {
+    key: _ref2,
     value: function () {}
+  }], [{
+    key: "10",
+    value: function _() {}
   }]);
   return MyClass;
 }();

diff --git a/packages/babel-plugin-transform-classes/package.json b/packages/babel-plugin-transform-classes/package.json
@@ -15,7 +15,6 @@
   "main": "lib/index.js",
   "dependencies": {
     "@babel/helper-annotate-as-pure": "workspace:^7.10.4",
-    "@babel/helper-define-map": "workspace:^7.10.4",
     "@babel/helper-function-name": "workspace:^7.10.4",
     "@babel/helper-optimise-call-expression": "workspace:^7.10.4",
     "@babel/helper-plugin-utils": "workspace:^7.10.4",

diff --git a/packages/babel-plugin-transform-classes/src/transformClass.js b/packages/babel-plugin-transform-classes/src/transformClass.js
@@ -4,7 +4,6 @@ import ReplaceSupers, {
   environmentVisitor,
 } from "@babel/helper-replace-supers";
 import optimiseCall from "@babel/helper-optimise-call-expression";
-import * as defineMap from "@babel/helper-define-map";
 import { traverse, template, types as t } from "@babel/core";
 import annotateAsPure from "@babel/helper-annotate-as-pure";
 
@@ -49,8 +48,6 @@ export default function transformClass(
     userConstructorPath: undefined,
     hasConstructor: false,
 
-    instancePropBody: [],
-    instancePropRefs: {},
     staticPropBody: [],
     body: [],
     superThises: [],
@@ -59,10 +56,21 @@ export default function transformClass(
     protoAlias: null,
     isLoose: false,
 
-    hasInstanceDescriptors: false,
-    hasStaticDescriptors: false,
-    instanceMutatorMap: {},
-    staticMutatorMap: {},
+    methods: {
+      // 'list' is in the same order as the elements appear in the class body.
+      // if there aren't computed keys, we can safely reorder class elements
+      // and use 'map' to merge duplicates.
+      instance: {
+        hasComputed: false,
+        list: [],
+        map: new Map(),
+      },
+      static: {
+        hasComputed: false,
+        list: [],
+        map: new Map(),
+      },
+    },
   };
 
   const setState = newState => {
@@ -78,25 +86,6 @@ export default function transformClass(
     },
   ]);
 
-  function pushToMap(node, enumerable, kind = "value", scope?) {
-    let mutatorMap;
-    if (node.static) {
-      setState({ hasStaticDescriptors: true });
-      mutatorMap = classState.staticMutatorMap;
-    } else {
-      setState({ hasInstanceDescriptors: true });
-      mutatorMap = classState.instanceMutatorMap;
-    }
-
-    const map = defineMap.push(mutatorMap, node, kind, classState.file, scope);
-
-    if (enumerable) {
-      map.enumerable = t.booleanLiteral(true);
-    }
-
-    return map;
-  }
-
   /**
    * Creates a class constructor or bail out if there is none
    */
@@ -202,48 +191,43 @@ export default function transformClass(
     }
   }
 
-  function clearDescriptors() {
-    setState({
-      hasInstanceDescriptors: false,
-      hasStaticDescriptors: false,
-      instanceMutatorMap: {},
-      staticMutatorMap: {},
-    });
-  }
-
   function pushDescriptors() {
     pushInheritsToBody();
 
     const { body } = classState;
 
-    let instanceProps;
-    let staticProps;
-
-    if (classState.hasInstanceDescriptors) {
-      instanceProps = defineMap.toClassObject(classState.instanceMutatorMap);
-    }
+    const props = {
+      instance: null,
+      static: null,
+    };
 
-    if (classState.hasStaticDescriptors) {
-      staticProps = defineMap.toClassObject(classState.staticMutatorMap);
-    }
+    for (const placement of ["static", "instance"]) {
+      if (classState.methods[placement].list.length) {
+        props[placement] = classState.methods[placement].list.map(desc => {
+          const obj = t.objectExpression([
+            t.objectProperty(t.identifier("key"), desc.key),
+          ]);
+
+          for (const kind of ["get", "set", "value"]) {
+            if (desc[kind] != null) {
+              obj.properties.push(
+                t.objectProperty(t.identifier(kind), desc[kind]),
+              );
+            }
+          }
 
-    if (instanceProps || staticProps) {
-      if (instanceProps) {
-        instanceProps = defineMap.toComputedObjectFromClass(instanceProps);
-      }
-      if (staticProps) {
-        staticProps = defineMap.toComputedObjectFromClass(staticProps);
+          return obj;
+        });
       }
+    }
 
+    if (props.instance || props.static) {
       let args = [
         t.cloneNode(classState.classRef), // Constructor
-        t.nullLiteral(), // instanceDescriptors
-        t.nullLiteral(), // staticDescriptors
+        props.instance ? t.arrayExpression(props.instance) : t.nullLiteral(), // instanceDescriptors
+        props.static ? t.arrayExpression(props.static) : t.nullLiteral(), // staticDescriptors
       ];
 
-      if (instanceProps) args[1] = instanceProps;
-      if (staticProps) args[2] = staticProps;
-
       let lastNonNullIndex = 0;
       for (let i = 0; i < args.length; i++) {
         if (!t.isNullLiteral(args[i])) lastNonNullIndex = i;
@@ -256,8 +240,6 @@ export default function transformClass(
         ),
       );
     }
-
-    clearDescriptors();
   }
 
   function wrapSuperCall(bareSuper, superRef, thisRef, body) {
@@ -428,7 +410,45 @@ export default function transformClass(
       if (processMethod(node, scope)) return;
     }
 
-    pushToMap(node, false, null, scope);
+    const placement = node.static ? "static" : "instance";
+    const methods = classState.methods[placement];
+
+    const descKey = node.kind === "method" ? "value" : node.kind;
+    const key =
+      t.isNumericLiteral(node.key) || t.isBigIntLiteral(node.key)
+        ? t.stringLiteral(String(node.key.value))
+        : t.toComputedKey(node);
+
+    let fn = t.toExpression(node);
+
+    if (t.isStringLiteral(key)) {
+      // infer function name
+      if (node.kind === "method") {
+        fn = nameFunction({ id: key, node: node, scope });
+      }
+    } else {
+      methods.hasComputed = true;
+    }
+
+    let descriptor;
+    if (!methods.hasComputed && methods.map.has(key.value)) {
+      descriptor = methods.map.get(key.value);
+      descriptor[descKey] = fn;
+
+      if (descKey === "value") {
+        descriptor.get = null;
+        descriptor.set = null;
+      } else {
+        descriptor.value = null;
+      }
+    } else {
+      descriptor = { key: key, [descKey]: fn };
+      methods.list.push(descriptor);
+
+      if (!methods.hasComputed) {
+        methods.map.set(key.value, descriptor);
+      }
+    }
   }
 
   function processMethod(node, scope) {

diff --git a/packages/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/exec.js b/packages/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/exec.js
@@ -0,0 +1,21 @@
+let x = 1;
+
+class A {
+  get [x]() {}
+  [(x = 3, 2)]() {}
+  set [x](_) {}
+}
+
+expect(Object.getOwnPropertyDescriptors(A.prototype)).toMatchObject({
+  1: {
+    get: expect.any(Function),
+    set: undefined,
+  },
+  2: {
+    value: expect.any(Function),
+  },
+  3: {
+    get: undefined,
+    set: expect.any(Function),
+  }
+});
diff --git a/packages/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/input.js b/packages/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/input.js
@@ -0,0 +1,5 @@
+class A {
+  get [x]() {}
+  [(x = 2, 3)]() {}
+  set [x](_) {}
+}
diff --git a/...ges/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/options.json b/...ges/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/options.json
@@ -0,0 +1,3 @@
+{
+  "minNodeVersion": "8.0.0"
+}
diff --git a/packages/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/output.js b/packages/babel-plugin-transform-classes/test/fixtures/get-set/change-computed-key/output.js
@@ -0,0 +1,26 @@
+function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }
+
+function _defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } }
+
+function _createClass(Constructor, protoProps, staticProps) { if (protoProps) _defineProperties(Constructor.prototype, protoProps); if (staticProps) _defineProperties(Constructor, staticProps); return Constructor; }
+
+let A = /*#__PURE__*/function () {
+  "use strict";
+
+  function A() {
+    _classCallCheck(this, A);
+  }
+
+  _createClass(A, [{
+    key: x,
+    get: function () {}
+  }, {
+    key: (x = 2, 3),
+    value: function () {}
+  }, {
+    key: x,
+    set: function (_) {}
+  }]);
+
+  return A;
+}();
diff --git a/packages/babel-plugin-transform-classes/test/fixtures/get-set/method-conflict/exec.js b/packages/babel-plugin-transform-classes/test/fixtures/get-set/method-conflict/exec.js
@@ -0,0 +1,12 @@
+class A {
+  get x() {}
+  x() {}
+  set x(_) {}
+}
+
+expect(Object.getOwnPropertyDescriptors(A.prototype)).toMatchObject({
+  x: {
+    get: undefined,
+    set: expect.any(Function),
+  }
+});