fix: Computed properties should keep order

babel · nicolo-ribaudo · Dec 5, 2022 · Nov 28, 2022 · Nov 28, 2022 · Nov 29, 2022
commit 4061dae823033df27c31323d060f026b8fd7026a
diff --git a/packages/babel-helpers/src/helpers-generated.ts b/packages/babel-helpers/src/helpers-generated.ts
@@ -45,6 +45,10 @@ export default Object.freeze({
     "7.20.5",
     'export default function _checkInRHS(value){if(Object(value)!==value)throw TypeError("right-hand side of \'in\' should be an object, got "+(null!==value?typeof value:"null"));return value}',
   ),
+  defineAccessor: helper(
+    "7.20.6",
+    "export default function _defineAccessor(obj,key,type,fn){var desc={configurable:!0,enumerable:!0};return desc[type]=fn,Object.defineProperty(obj,key,desc)}",
+  ),
   iterableToArrayLimit: helper(
     "7.0.0-beta.0",
     'export default function _iterableToArrayLimit(arr,i){var _i=null==arr?null:"undefined"!=typeof Symbol&&arr[Symbol.iterator]||arr["@@iterator"];if(null!=_i){var _s,_e,_x,_r,_arr=[],_n=!0,_d=!1;try{if(_x=(_i=_i.call(arr)).next,0===i){if(Object(_i)!==_i)return;_n=!1}else for(;!(_n=(_s=_x.call(_i)).done)&&(_arr.push(_s.value),_arr.length!==i);_n=!0);}catch(err){_d=!0,_e=err}finally{try{if(!_n&&null!=_i.return&&(_r=_i.return(),Object(_r)!==_r))return}finally{if(_d)throw _e}}return _arr}}',

diff --git a/packages/babel-helpers/src/helpers/defineAccessor.js b/packages/babel-helpers/src/helpers/defineAccessor.js
@@ -0,0 +1,8 @@
+/* @minVersion 7.20.6 */
+
+export default function _defineAccessor(obj, key, type, fn) {
+  var desc = { configurable: true, enumerable: true };
+  // type should be "get" or "set"
+  desc[type] = fn;
+  return Object.defineProperty(obj, key, desc);
+}
diff --git a/packages/babel-plugin-transform-computed-properties/src/index.ts b/packages/babel-plugin-transform-computed-properties/src/index.ts
@@ -1,5 +1,6 @@
+import { types as t } from "@babel/core";
+import type { PluginPass } from "@babel/core";
 import { declare } from "@babel/helper-plugin-utils";
-import { template, types as t, type PluginPass } from "@babel/core";
 import type { Scope } from "@babel/traverse";
 
 export interface Options {
@@ -12,7 +13,6 @@ type PropertyInfo = {
   body: t.Statement[];
   computedProps: t.ObjectMember[];
   initPropExpression: t.ObjectExpression;
-  getMutatorId: () => t.Identifier;
   state: PluginPass;
 };
 
@@ -26,11 +26,6 @@ export default declare((api, options: Options) => {
     ? pushComputedPropsLoose
     : pushComputedPropsSpec;
 
-  const buildMutatorMapAssign = template.statements(`
-    MUTATOR_MAP_REF[KEY] = MUTATOR_MAP_REF[KEY] || {};
-    MUTATOR_MAP_REF[KEY].KIND = VALUE;
-  `);
-
   /**
    * Get value of an object member under object expression.
    * Returns a function expression if prop is a ObjectMethod.
@@ -72,31 +67,34 @@ export default declare((api, options: Options) => {
     );
   }
 
-  function pushMutatorDefine(
-    { body, getMutatorId, scope }: PropertyInfo,
+  function pushAccessorDefine(
+    { body, computedProps, initPropExpression, objId, state }: PropertyInfo,
     prop: t.ObjectMethod,
   ) {
-    let key =
+    const key =
       !prop.computed && t.isIdentifier(prop.key)
         ? t.stringLiteral(prop.key.name)
         : prop.key;
 
-    const maybeMemoise = scope.maybeGenerateMemoised(key);
-    if (maybeMemoise) {
+    if (computedProps.length === 1) {
+      return t.callExpression(state.addHelper("defineAccessor"), [
+        initPropExpression,
+        key,
+        t.stringLiteral(prop.kind),
+        getValue(prop),
+      ]);
+    } else {
       body.push(
-        t.expressionStatement(t.assignmentExpression("=", maybeMemoise, key)),
+        t.expressionStatement(
+          t.callExpression(state.addHelper("defineAccessor"), [
+            t.cloneNode(objId),
+            key,
+            t.stringLiteral(prop.kind),
+            getValue(prop),
+          ]),
+        ),
       );
-      key = maybeMemoise;
     }
-
-    body.push(
-      ...buildMutatorMapAssign({
-        MUTATOR_MAP_REF: getMutatorId(),
-        KEY: t.cloneNode(key),
-        VALUE: getValue(prop),
-        KIND: t.identifier(prop.kind),
-      }),
-    );
   }
 
   function pushComputedPropsLoose(info: PropertyInfo) {
@@ -105,7 +103,8 @@ export default declare((api, options: Options) => {
         t.isObjectMethod(prop) &&
         (prop.kind === "get" || prop.kind === "set")
       ) {
-        pushMutatorDefine(info, prop);
+        const single = pushAccessorDefine(info, prop);
+        if (single) return single;
       } else {
         pushAssign(t.cloneNode(info.objId), prop, info.body);
       }
@@ -123,7 +122,8 @@ export default declare((api, options: Options) => {
         t.isObjectMethod(prop) &&
         (prop.kind === "get" || prop.kind === "set")
       ) {
-        pushMutatorDefine(info, prop);
+        const single = pushAccessorDefine(info, prop);
+        if (single) return single;
       } else {
         // the value of ObjectProperty in ObjectExpression must be an expression
         const value = getValue(prop) as t.Expression;
@@ -195,44 +195,15 @@ export default declare((api, options: Options) => {
             ]),
           );
 
-          let mutatorRef: t.Identifier;
-
-          const getMutatorId = function () {
-            if (!mutatorRef) {
-              mutatorRef = scope.generateUidIdentifier("mutatorMap");
-
-              body.push(
-                t.variableDeclaration("var", [
-                  t.variableDeclarator(mutatorRef, t.objectExpression([])),
-                ]),
-              );
-            }
-
-            return t.cloneNode(mutatorRef);
-          };
-
           const single = pushComputedProps({
             scope,
             objId,
             body,
             computedProps,
             initPropExpression,
-            getMutatorId,
             state,
           });
 
-          if (mutatorRef) {
-            body.push(
-              t.expressionStatement(
-                t.callExpression(
-                  state.addHelper("defineEnumerableProperties"),
-                  [t.cloneNode(objId), t.cloneNode(mutatorRef)],
-                ),
-              ),
-            );
-          }
-
-          // @ts-expect-error todo(flow->ts) `void` should not be used as variable
           if (single) {
             path.replaceWith(single);
           } else {

diff --git a/...rm-computed-properties/test/fixtures/assumption-setComputedProperties/accessors/output.js b/...rm-computed-properties/test/fixtures/assumption-setComputedProperties/accessors/output.js
@@ -1,10 +1,10 @@
-var _foobar, _foobar2, _test, _test2, _obj, _mutatorMap;
-var obj = (_obj = {}, _foobar = foobar, _mutatorMap = {}, _mutatorMap[_foobar] = _mutatorMap[_foobar] || {}, _mutatorMap[_foobar].get = function () {
+var _obj;
+var obj = (_obj = {}, babelHelpers.defineAccessor(_obj, foobar, "get", function () {
   return "foobar";
-}, _foobar2 = foobar, _mutatorMap[_foobar2] = _mutatorMap[_foobar2] || {}, _mutatorMap[_foobar2].set = function (x) {
+}), babelHelpers.defineAccessor(_obj, foobar, "set", function (x) {
   console.log(x);
-}, _test = "test", _mutatorMap[_test] = _mutatorMap[_test] || {}, _mutatorMap[_test].get = function () {
+}), babelHelpers.defineAccessor(_obj, "test", "get", function () {
   return "regular getter after computed property";
-}, _test2 = "test", _mutatorMap[_test2] = _mutatorMap[_test2] || {}, _mutatorMap[_test2].set = function (x) {
+}), babelHelpers.defineAccessor(_obj, "test", "set", function (x) {
   console.log(x);
-}, babelHelpers.defineEnumerableProperties(_obj, _mutatorMap), _obj);
+}), _obj);
diff --git a/...mputed-properties/test/fixtures/assumption-setComputedProperties/single-accessor/input.js b/...mputed-properties/test/fixtures/assumption-setComputedProperties/single-accessor/input.js
@@ -0,0 +1,3 @@
+var obj = {
+  get ["x" + foo]() { return "heh"; }
+};
diff --git a/...puted-properties/test/fixtures/assumption-setComputedProperties/single-accessor/output.js b/...puted-properties/test/fixtures/assumption-setComputedProperties/single-accessor/output.js
@@ -0,0 +1,3 @@
+var obj = babelHelpers.defineAccessor({}, "x" + foo, "get", function () {
+  return "heh";
+});
diff --git a/...sform-computed-properties/test/fixtures/assumption-setComputedProperties/symbol/output.js b/...sform-computed-properties/test/fixtures/assumption-setComputedProperties/symbol/output.js
@@ -1,5 +1,5 @@
-var _foo, _mutatorMap;
+var _foo;
 var k = Symbol();
-var foo = (_foo = {}, _foo[Symbol.iterator] = "foobar", _mutatorMap = {}, _mutatorMap[k] = _mutatorMap[k] || {}, _mutatorMap[k].get = function () {
+var foo = (_foo = {}, _foo[Symbol.iterator] = "foobar", babelHelpers.defineAccessor(_foo, k, "get", function () {
   return k;
-}, babelHelpers.defineEnumerableProperties(_foo, _mutatorMap), _foo);
+}), _foo);
diff --git a/packages/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/exec.js b/packages/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/exec.js
@@ -0,0 +1,3 @@
+var x = { x, get x() { return 0; }, x };
+x.x = 1;
+expect(x.x).toBe(1);
diff --git a/packages/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/input.js b/packages/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/input.js
@@ -0,0 +1,2 @@
+var x = { x, get x() { return 0; }, x };
+x.x = 1;
diff --git a/...es/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/options.json b/...es/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/options.json
@@ -0,0 +1,6 @@
+{
+  "plugins": [
+    "transform-duplicate-keys",
+    "transform-computed-properties"
+  ]
+}
diff --git a/packages/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/output.js b/packages/babel-plugin-transform-computed-properties/test/fixtures/regression/15140/output.js
@@ -0,0 +1,7 @@
+var _x;
+var x = (_x = {
+  x
+}, babelHelpers.defineAccessor(_x, "x", "get", function () {
+  return 0;
+}), babelHelpers.defineProperty(_x, "x", x), _x);
+x.x = 1;
diff --git a/packages/babel-plugin-transform-computed-properties/test/fixtures/spec/accessors/output.js b/packages/babel-plugin-transform-computed-properties/test/fixtures/spec/accessors/output.js
@@ -1,10 +1,10 @@
-var _foobar, _foobar2, _test, _test2, _obj, _mutatorMap;
-var obj = (_obj = {}, _foobar = foobar, _mutatorMap = {}, _mutatorMap[_foobar] = _mutatorMap[_foobar] || {}, _mutatorMap[_foobar].get = function () {
+var _obj;
+var obj = (_obj = {}, babelHelpers.defineAccessor(_obj, foobar, "get", function () {
   return "foobar";
-}, _foobar2 = foobar, _mutatorMap[_foobar2] = _mutatorMap[_foobar2] || {}, _mutatorMap[_foobar2].set = function (x) {
+}), babelHelpers.defineAccessor(_obj, foobar, "set", function (x) {
   console.log(x);
-}, _test = "test", _mutatorMap[_test] = _mutatorMap[_test] || {}, _mutatorMap[_test].get = function () {
+}), babelHelpers.defineAccessor(_obj, "test", "get", function () {
   return "regular getter after computed property";
-}, _test2 = "test", _mutatorMap[_test2] = _mutatorMap[_test2] || {}, _mutatorMap[_test2].set = function (x) {
+}), babelHelpers.defineAccessor(_obj, "test", "set", function (x) {
   console.log(x);
-}, babelHelpers.defineEnumerableProperties(_obj, _mutatorMap), _obj);
+}), _obj);
diff --git a/...es/babel-plugin-transform-computed-properties/test/fixtures/spec/single-accessor/input.js b/...es/babel-plugin-transform-computed-properties/test/fixtures/spec/single-accessor/input.js
@@ -0,0 +1,3 @@
+var obj = {
+  get ["x" + foo]() { return "heh"; }
+};
diff --git a/...s/babel-plugin-transform-computed-properties/test/fixtures/spec/single-accessor/output.js b/...s/babel-plugin-transform-computed-properties/test/fixtures/spec/single-accessor/output.js
@@ -0,0 +1,3 @@
+var obj = babelHelpers.defineAccessor({}, "x" + foo, "get", function () {
+  return "heh";
+});
diff --git a/packages/babel-plugin-transform-computed-properties/test/fixtures/spec/symbol/output.js b/packages/babel-plugin-transform-computed-properties/test/fixtures/spec/symbol/output.js
@@ -1,5 +1,5 @@
-var _foo, _mutatorMap;
+var _foo;
 var k = Symbol();
-var foo = (_foo = {}, babelHelpers.defineProperty(_foo, Symbol.iterator, "foobar"), _mutatorMap = {}, _mutatorMap[k] = _mutatorMap[k] || {}, _mutatorMap[k].get = function () {
+var foo = (_foo = {}, babelHelpers.defineProperty(_foo, Symbol.iterator, "foobar"), babelHelpers.defineAccessor(_foo, k, "get", function () {
   return k;
-}, babelHelpers.defineEnumerableProperties(_foo, _mutatorMap), _foo);
+}), _foo);
diff --git a/packages/babel-runtime-corejs2/package.json b/packages/babel-runtime-corejs2/package.json
@@ -90,6 +90,15 @@
       "./helpers/checkInRHS.js"
     ],
     "./helpers/esm/checkInRHS": "./helpers/esm/checkInRHS.js",
+    "./helpers/defineAccessor": [
+      {
+        "node": "./helpers/defineAccessor.js",
+        "import": "./helpers/esm/defineAccessor.js",
+        "default": "./helpers/defineAccessor.js"
+      },
+      "./helpers/defineAccessor.js"
+    ],
+    "./helpers/esm/defineAccessor": "./helpers/esm/defineAccessor.js",
     "./helpers/iterableToArrayLimit": [
       {
         "node": "./helpers/iterableToArrayLimit.js",

diff --git a/packages/babel-runtime-corejs3/package.json b/packages/babel-runtime-corejs3/package.json
@@ -89,6 +89,15 @@
       "./helpers/checkInRHS.js"
     ],
     "./helpers/esm/checkInRHS": "./helpers/esm/checkInRHS.js",
+    "./helpers/defineAccessor": [
+      {
+        "node": "./helpers/defineAccessor.js",
+        "import": "./helpers/esm/defineAccessor.js",
+        "default": "./helpers/defineAccessor.js"
+      },
+      "./helpers/defineAccessor.js"
+    ],
+    "./helpers/esm/defineAccessor": "./helpers/esm/defineAccessor.js",
     "./helpers/iterableToArrayLimit": [
       {
         "node": "./helpers/iterableToArrayLimit.js",

diff --git a/packages/babel-runtime/package.json b/packages/babel-runtime/package.json
@@ -89,6 +89,15 @@
       "./helpers/checkInRHS.js"
     ],
     "./helpers/esm/checkInRHS": "./helpers/esm/checkInRHS.js",
+    "./helpers/defineAccessor": [
+      {
+        "node": "./helpers/defineAccessor.js",
+        "import": "./helpers/esm/defineAccessor.js",
+        "default": "./helpers/defineAccessor.js"
+      },
+      "./helpers/defineAccessor.js"
+    ],
+    "./helpers/esm/defineAccessor": "./helpers/esm/defineAccessor.js",
     "./helpers/iterableToArrayLimit": [
       {
         "node": "./helpers/iterableToArrayLimit.js",
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		var x = { x, get x() { return 0; }, x };
		x.x = 1;