Fix 2021-12 decorators application order (#14244)

* decorated class and class init are returned last

* fix: class decs eval earlier than member decs

* fix: decorators are applied from right to left

* add es2015 test cases

* fix: ensure applyDecs is ES5 compliant

* bump applyDecs minVersion

* fix: do not return stub initializer when empty

* Revert "fix: class decs eval earlier than member decs"

This reverts commit 4ef1699.

# Conflicts:
#	packages/babel-helpers/src/helpers-generated.ts

* fix: memoise non-static decorators / keys

* fix: move insertBefore after path queries

* test: add computed keys to ordering test

* make node 14 happy

* add es2015 test

* cleanup

* test: apply static block for node 14

packages/babel-helpers/src/helpers/applyDecs.js

@@ -18,7 +18,7 @@
 
 function createMetadataMethodsForProperty(metadataMap, kind, property) {
   return {
-    getMetadata(key) {
+    getMetadata: function (key) {
       if (typeof key !== "symbol") {
         throw new TypeError("Metadata keys must be symbols, received: " + key);
       }
@@ -41,7 +41,7 @@ function createMetadataMethodsForProperty(metadataMap, kind, property) {
         return metadataForKey.constructor;
       }
     },
-    setMetadata(key, value) {
+    setMetadata: function (key, value) {
       if (typeof key !== "symbol") {
         throw new TypeError("Metadata keys must be symbols, received: " + key);
       }
@@ -317,7 +317,7 @@ function applyMemberDec(
       }
     }
   } else {
-    for (var i = 0; i < decs.length; i++) {
+    for (var i = decs.length - 1; i >= 0; i--) {
       var dec = decs[i];
 
       newValue = dec(value, ctx);
@@ -421,8 +421,8 @@ function applyMemberDecs(
   staticMetadataMap,
   decInfos
 ) {
-  var protoInitializers;
-  var staticInitializers;
+  var protoInitializers = [];
+  var staticInitializers = [];
 
   var existingProtoNonFields = new Map();
   var existingStaticNonFields = new Map();
@@ -447,19 +447,11 @@ function applyMemberDecs(
       metadataMap = staticMetadataMap;
       kind = kind - 5 /* STATIC */;
 
-      if (!staticInitializers) {
-        staticInitializers = [];
-      }
-
       initializers = staticInitializers;
     } else {
       base = Class.prototype;
       metadataMap = protoMetadataMap;
 
-      if (!protoInitializers) {
-        protoInitializers = [];
-      }
-
       initializers = protoInitializers;
     }
 
@@ -499,11 +491,11 @@ function applyMemberDecs(
     );
   }
 
-  if (protoInitializers) {
+  if (protoInitializers.length > 0) {
     pushInitializers(ret, protoInitializers);
   }
 
-  if (staticInitializers) {
+  if (staticInitializers.length > 0) {
     pushInitializers(ret, staticInitializers);
   }
 }
@@ -541,7 +533,7 @@ function applyClassDecs(ret, targetClass, metadataMap, classDecs) {
     createMetadataMethodsForProperty(metadataMap, 0 /* CONSTRUCTOR */, name)
   );
 
-  for (var i = 0; i < classDecs.length; i++) {
+  for (var i = classDecs.length - 1; i >= 0; i--) {
     newClass = classDecs[i](newClass, ctx) || newClass;
   }
 

packages/babel-plugin-proposal-decorators/src/transformer-2021-12.ts

@@ -1,4 +1,4 @@
-import type { NodePath } from "@babel/traverse";
+import type { NodePath, Scope } from "@babel/traverse";
 import { types as t, template } from "@babel/core";
 import syntaxDecorators from "@babel/plugin-syntax-decorators";
 import ReplaceSupers from "@babel/helper-replace-supers";
@@ -506,23 +506,38 @@ function transformClass(
   let constructorPath: NodePath<t.ClassMethod> | undefined;
   let requiresProtoInit = false;
   let requiresStaticInit = false;
-  let hasComputedProps = false;
   const decoratedPrivateMethods = new Set<string>();
 
   let protoInitLocal: t.Identifier,
     staticInitLocal: t.Identifier,
     classInitLocal: t.Identifier,
     classLocal: t.Identifier;
+  const assignments: t.AssignmentExpression[] = [];
+  const scopeParent: Scope = path.scope.parent;
+
+  const memoiseExpression = (expression: t.Expression, hint: string) => {
+    const localEvaluatedId = scopeParent.generateDeclaredUidIdentifier(hint);
+    assignments.push(t.assignmentExpression("=", localEvaluatedId, expression));
+    return t.cloneNode(localEvaluatedId);
+  };
 
   if (classDecorators) {
-    classInitLocal =
-      path.scope.parent.generateDeclaredUidIdentifier("initClass");
+    classInitLocal = scopeParent.generateDeclaredUidIdentifier("initClass");
 
     const [localId, classPath] = replaceClassWithVar(path);
     path = classPath;
     classLocal = localId;
 
     path.node.decorators = null;
+
+    for (const classDecorator of classDecorators) {
+      if (!scopeParent.isStatic(classDecorator.expression)) {
+        classDecorator.expression = memoiseExpression(
+          classDecorator.expression,
+          "dec",
+        );
+      }
+    }
   } else {
     if (!path.node.id) {
       path.node.id = path.scope.generateUidIdentifier("Class");
@@ -536,43 +551,50 @@ function transformClass(
         continue;
       }
 
-      let { key } = element.node;
-      const kind = getElementKind(element);
+      const { node } = element;
       const decorators = element.get("decorators");
 
-      const isPrivate = key.type === "PrivateName";
+      const hasDecorators = Array.isArray(decorators) && decorators.length > 0;
+
+      if (hasDecorators) {
+        for (const decoratorPath of decorators) {
+          if (!scopeParent.isStatic(decoratorPath.node.expression)) {
+            decoratorPath.node.expression = memoiseExpression(
+              decoratorPath.node.expression,
+              "dec",
+            );
+          }
+        }
+      }
+
       const isComputed =
         "computed" in element.node && element.node.computed === true;
+      if (isComputed) {
+        if (!scopeParent.isStatic(node.key)) {
+          node.key = memoiseExpression(node.key as t.Expression, "computedKey");
+        }
+      }
+
+      const kind = getElementKind(element);
+      const { key } = node;
+
+      const isPrivate = key.type === "PrivateName";
+
       const isStatic = !!element.node.static;
 
       let name = "computedKey";
 
       if (isPrivate) {
         name = (key as t.PrivateName).id.name;
-      } else if (key.type === "Identifier") {
+      } else if (!isComputed && key.type === "Identifier") {
         name = key.name;
       }
 
       if (element.isClassMethod({ kind: "constructor" })) {
         constructorPath = element;
       }
 
-      if (isComputed) {
-        const keyPath = element.get("key");
-        const localComputedNameId =
-          keyPath.scope.parent.generateDeclaredUidIdentifier(name);
-        keyPath.replaceWith(localComputedNameId);
-
-        elementDecoratorInfo.push({
-          localComputedNameId: t.cloneNode(localComputedNameId),
-          keyNode: t.cloneNode(key as t.Expression),
-        });
-
-        key = localComputedNameId;
-        hasComputedProps = true;
-      }
-
-      if (Array.isArray(decorators) && decorators.length > 0) {
+      if (hasDecorators) {
         let locals: t.Identifier | t.Identifier[];
         let privateMethods: t.FunctionExpression | t.FunctionExpression[];
 
@@ -730,34 +752,6 @@ function transformClass(
     }
   }
 
-  if (hasComputedProps) {
-    const assignments: t.AssignmentExpression[] = [];
-
-    for (const info of elementDecoratorInfo) {
-      if (isDecoratorInfo(info)) {
-        const { decorators } = info;
-        const newDecorators: t.Identifier[] = [];
-
-        for (const decorator of decorators) {
-          const localComputedNameId =
-            path.scope.parent.generateDeclaredUidIdentifier("dec");
-          assignments.push(
-            t.assignmentExpression("=", localComputedNameId, decorator),
-          );
-          newDecorators.push(t.cloneNode(localComputedNameId));
-        }
-
-        info.decorators = newDecorators;
-      } else {
-        assignments.push(
-          t.assignmentExpression("=", info.localComputedNameId, info.keyNode),
-        );
-      }
-    }
-
-    path.insertBefore(assignments);
-  }
-
   const elementDecorations = generateDecorationExprs(elementDecoratorInfo);
   const classDecorations = t.arrayExpression(
     (classDecorators || []).map(d => d.expression),
@@ -766,13 +760,8 @@ function transformClass(
   const locals: t.Identifier[] =
     extractElementLocalAssignments(elementDecoratorInfo);
 
-  if (classDecorators) {
-    locals.push(classLocal, classInitLocal);
-  }
-
   if (requiresProtoInit) {
-    protoInitLocal =
-      path.scope.parent.generateDeclaredUidIdentifier("initProto");
+    protoInitLocal = scopeParent.generateDeclaredUidIdentifier("initProto");
     locals.push(protoInitLocal);
 
     const protoInitCall = t.callExpression(t.cloneNode(protoInitLocal), [
@@ -833,8 +822,7 @@ function transformClass(
   }
 
   if (requiresStaticInit) {
-    staticInitLocal =
-      path.scope.parent.generateDeclaredUidIdentifier("initStatic");
+    staticInitLocal = scopeParent.generateDeclaredUidIdentifier("initStatic");
     locals.push(staticInitLocal);
   }
 
@@ -879,6 +867,7 @@ function transformClass(
   const originalClass = path.node;
 
   if (classDecorators) {
+    locals.push(classLocal, classInitLocal);
     const statics = [];
     let staticBlocks: t.StaticBlock[] = [];
     path.get("body.body").forEach(element => {
@@ -982,6 +971,10 @@ function transformClass(
     ),
   );
 
+  // When path is a ClassExpression, path.insertBefore will convert `path`
+  // into a SequenceExpression
+  path.insertBefore(assignments);
+
   // Recrawl the scope to make sure new identifiers are properly synced
   path.scope.crawl();
 

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/private/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   @dec
   accessor #a;

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/private/output.js

@@ -1,5 +1,7 @@
 var _init_a, _get_a, _set_a, _init_b, _get_b, _set_b, _initProto;
 
+const dec = () => {};
+
 var _A = /*#__PURE__*/new WeakMap();
 
 var _a = /*#__PURE__*/new WeakMap();

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/public/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   @dec
   accessor a;

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/public/output.js

@@ -1,9 +1,8 @@
-var _init_a, _init_b, _computedKey, _init_computedKey, _dec, _dec2, _dec3, _initProto;
+var _init_a, _init_b, _computedKey, _init_computedKey, _initProto;
+
+const dec = () => {};
 
-_dec = dec
-_dec2 = dec
 _computedKey = 'c'
-_dec3 = dec
 
 var _A = /*#__PURE__*/new WeakMap();
 
@@ -54,5 +53,5 @@ class Foo {
 }
 
 (() => {
-  [_init_a, _init_b, _init_computedKey, _initProto] = babelHelpers.applyDecs(Foo, [[_dec, 1, "a"], [_dec2, 1, "b"], [_dec3, 1, _computedKey]], []);
+  [_init_a, _init_b, _init_computedKey, _initProto] = babelHelpers.applyDecs(Foo, [[dec, 1, "a"], [dec, 1, "b"], [dec, 1, _computedKey]], []);
 })();

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/static-private/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   @dec
   static accessor #a;

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/static-private/output.js

@@ -1,5 +1,7 @@
 var _init_a, _get_a, _set_a, _init_b, _get_b, _set_b, _initStatic;
 
+const dec = () => {};
+
 var _a = /*#__PURE__*/new WeakMap();
 
 var _b = /*#__PURE__*/new WeakMap();

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/static-public/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   @dec
   static accessor a;

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/static-public/output.js

@@ -1,9 +1,8 @@
-var _init_a, _init_b, _computedKey, _init_computedKey, _dec, _dec2, _dec3, _initStatic;
+var _init_a, _init_b, _computedKey, _init_computedKey, _initStatic;
+
+const dec = () => {};
 
-_dec = dec
-_dec2 = dec
 _computedKey = 'c'
-_dec3 = dec
 
 class Foo {
   static get a() {
@@ -33,7 +32,7 @@ class Foo {
 }
 
 (() => {
-  [_init_a, _init_b, _init_computedKey, _initStatic] = babelHelpers.applyDecs(Foo, [[_dec, 6, "a"], [_dec2, 6, "b"], [_dec3, 6, _computedKey]], []);
+  [_init_a, _init_b, _init_computedKey, _initStatic] = babelHelpers.applyDecs(Foo, [[dec, 6, "a"], [dec, 6, "b"], [dec, 6, _computedKey]], []);
 
   _initStatic(Foo);
 })();

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/undecorated-private/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   accessor #a;
 

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/undecorated-private/output.js

@@ -1,3 +1,5 @@
+const dec = () => {};
+
 var _A = /*#__PURE__*/new WeakMap();
 
 var _a = /*#__PURE__*/new WeakMap();

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/undecorated-public/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   accessor a;
 

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/undecorated-public/output.js

@@ -1,3 +1,5 @@
+const dec = () => {};
+
 var _A = /*#__PURE__*/new WeakMap();
 
 var _B = /*#__PURE__*/new WeakMap();

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/undecorated-static-private/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   static accessor #a;
 

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/undecorated-static-private/output.js

@@ -1,3 +1,5 @@
+const dec = () => {};
+
 class Foo {}
 
 function _get_a() {

packages/babel-plugin-proposal-decorators/test/fixtures/2021-12-accessors--to-es2015/undecorated-static-public/input.js

@@ -1,3 +1,4 @@
+const dec = () => {}; 
 class Foo {
   static accessor a;