refactor(compiler): Generalize the ordering phase to also order create mode (#51498)

`syntheticHostListener` and `listener` have ordering dependencies. We reuse the existing ordering phase, and generalize it to also order create mode instructions.

PR Close #51498

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/host_bindings/TEST_CASES.json

@@ -224,8 +224,7 @@
             "chain_synthetic_listeners_mixed.js"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should handle host bindings with the same name as a primitive value",

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json

@@ -163,8 +163,7 @@
           ],
           "failureMessage": "Incorrect event listener"
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate the $event argument if it is being used in a host listener",
@@ -267,8 +266,7 @@
           ],
           "failureMessage": "Incorrect event listener"
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate the view restoration statements if a keyed write is used in an event listener from within an ng-template",

packages/compiler/src/template/pipeline/ir/src/operations.ts

@@ -87,7 +87,14 @@ export class OpList<OpT extends Op<OpT>> {
   /**
    * Push a new operation to the tail of the list.
    */
-  push(op: OpT): void {
+  push(op: OpT|Array<OpT>): void {
+    if (Array.isArray(op)) {
+      for (const o of op) {
+        this.push(o);
+      }
+      return;
+    }
+
     OpList.assertIsNotEnd(op);
     OpList.assertIsUnowned(op);
 
@@ -267,7 +274,14 @@ export class OpList<OpT extends Op<OpT>> {
   /**
    * Insert `op` before `target`.
    */
-  static insertBefore<OpT extends Op<OpT>>(op: OpT, target: OpT): void {
+  static insertBefore<OpT extends Op<OpT>>(op: OpT|OpT[], target: OpT): void {
+    if (Array.isArray(op)) {
+      for (const o of op) {
+        this.insertBefore(o, target);
+      }
+      return;
+    }
+
     OpList.assertIsOwned(target);
     if (target.prev === null) {
       throw new Error(`AssertionError: illegal operation on list start`);

packages/compiler/src/template/pipeline/src/emit.ts

@@ -37,7 +37,7 @@ import {phaseNullishCoalescing} from './phases/nullish_coalescing';
 import {phaseParseExtractedStyles} from './phases/parse_extracted_styles';
 import {phasePipeCreation} from './phases/pipe_creation';
 import {phasePipeVariadic} from './phases/pipe_variadic';
-import {phasePropertyOrdering} from './phases/property_ordering';
+import {phaseOrdering} from './phases/ordering';
 import {phasePureFunctionExtraction} from './phases/pure_function_extraction';
 import {phasePureLiteralStructures} from './phases/pure_literal_structures';
 import {phaseReify} from './phases/reify';
@@ -96,7 +96,7 @@ export function transformTemplate(job: ComponentCompilationJob): void {
   phaseNonbindable(job);
   phasePureFunctionExtraction(job);
   phaseAlignPipeVariadicVarOffset(job);
-  phasePropertyOrdering(job);
+  phaseOrdering(job);
   phaseReify(job);
   phaseChaining(job);
 }
@@ -124,7 +124,7 @@ export function transformHostBinding(job: HostBindingCompilationJob): void {
   // phaseResolveSanitizers(job);
   phaseNaming(job);
   phasePureFunctionExtraction(job);
-  phasePropertyOrdering(job);
+  phaseOrdering(job);
   phaseReify(job);
   phaseChaining(job);
 }

packages/compiler/src/template/pipeline/src/phases/ordering.ts

@@ -0,0 +1,112 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import * as ir from '../../ir';
+import type {CompilationJob} from '../compilation';
+
+function kindTest(kind: ir.OpKind): (op: ir.UpdateOp) => boolean {
+  return (op: ir.UpdateOp) => op.kind === kind;
+}
+
+interface Rule<T extends ir.CreateOp|ir.UpdateOp> {
+  test: (op: T) => boolean;
+  transform?: (ops: Array<T>) => Array<T>;
+}
+
+/**
+ * Defines the groups based on `OpKind` that ops will be divided into, for the various create
+ * binding kinds. Ops will be collected into groups, then optionally transformed, before recombining
+ * the groups in the order defined here.
+ */
+const CREATE_ORDERING: Array<Rule<ir.CreateOp>> = [
+  {test: op => op.kind === ir.OpKind.Listener && op.hostListener && op.isAnimationListener},
+  {test: op => op.kind === ir.OpKind.Listener && !(op.hostListener && op.isAnimationListener)},
+];
+
+/**
+ * As above, but for update ops.
+ */
+const UPDATE_ORDERING: Array<Rule<ir.UpdateOp>> = [
+  {test: op => op.kind === ir.OpKind.HostProperty && op.expression instanceof ir.Interpolation},
+  {test: op => op.kind === ir.OpKind.HostProperty && !(op.expression instanceof ir.Interpolation)},
+  {test: kindTest(ir.OpKind.StyleMap), transform: keepLast},
+  {test: kindTest(ir.OpKind.ClassMap), transform: keepLast},
+  {test: kindTest(ir.OpKind.StyleProp)},
+  {test: kindTest(ir.OpKind.ClassProp)},
+  {test: op => op.kind === ir.OpKind.Property && op.expression instanceof ir.Interpolation},
+  {test: op => op.kind === ir.OpKind.Property && !(op.expression instanceof ir.Interpolation)},
+  {test: kindTest(ir.OpKind.Attribute)},
+];
+
+/**
+ * The set of all op kinds we handle in the reordering phase.
+ */
+const handledOpKinds = new Set([
+  ir.OpKind.Listener, ir.OpKind.StyleMap, ir.OpKind.ClassMap, ir.OpKind.StyleProp,
+  ir.OpKind.ClassProp, ir.OpKind.Property, ir.OpKind.HostProperty, ir.OpKind.Attribute
+]);
+
+export function phaseOrdering(job: CompilationJob) {
+  for (const unit of job.units) {
+    // First, we pull out ops that need to be ordered. Then, when we encounter an op that shouldn't
+    // be reordered, put the ones we've pulled so far back in the correct order. Finally, if we
+    // still have ops pulled at the end, put them back in the correct order.
+
+    // Create mode:
+    let opsToOrder = [];
+    for (const op of unit.create) {
+      if (handledOpKinds.has(op.kind)) {
+        opsToOrder.push(op);
+        ir.OpList.remove(op);
+      } else {
+        ir.OpList.insertBefore(reorder(opsToOrder, CREATE_ORDERING), op);
+        opsToOrder = [];
+      }
+    }
+    unit.create.push(reorder(opsToOrder, CREATE_ORDERING));
+
+
+    // Update mode:
+    opsToOrder = [];
+    for (const op of unit.update) {
+      if (handledOpKinds.has(op.kind)) {
+        opsToOrder.push(op);
+        ir.OpList.remove(op);
+      } else {
+        ir.OpList.insertBefore(reorder(opsToOrder, UPDATE_ORDERING), op);
+        opsToOrder = [];
+      }
+    }
+    unit.update.push(reorder(opsToOrder, UPDATE_ORDERING));
+  }
+}
+
+/**
+ * Reorders the given list of ops according to the ordering defined by `ORDERING`.
+ */
+function reorder<T extends ir.CreateOp|ir.UpdateOp>(
+    ops: Array<T>, ordering: Array<Rule<T>>): Array<T> {
+  // Break the ops list into groups based on OpKind.
+  const groups = Array.from(ordering, () => new Array<T>());
+  for (const op of ops) {
+    const groupIndex = ordering.findIndex(o => o.test(op));
+    groups[groupIndex].push(op);
+  }
+  // Reassemble the groups into a single list, in the correct order.
+  return groups.flatMap((group, i) => {
+    const transform = ordering[i].transform;
+    return transform ? transform(group) : group;
+  });
+}
+
+/**
+ * Keeps only the last op in a list of ops.
+ */
+function keepLast<T>(ops: Array<T>) {
+  return ops.slice(ops.length - 1);
+}

packages/compiler/src/template/pipeline/src/phases/property_ordering.ts

@@ -1,117 +0,0 @@
-/**
- * @license
- * Copyright Google LLC All Rights Reserved.
- *
- * Use of this source code is governed by an MIT-style license that can be
- * found in the LICENSE file at https://angular.io/license
- */
-
-import * as ir from '../../ir';
-import type {CompilationJob} from '../compilation';
-
-function kindTest(kind: ir.OpKind): (op: ir.UpdateOp) => boolean {
-  return (op: ir.UpdateOp) => op.kind === kind;
-}
-
-/**
- * Defines the groups based on `OpKind` that ops will be divided into. Ops will be collected into
- * groups, then optionally transformed, before recombining the groups in the order defined here.
- */
-const ORDERING: {
-  test: (op: ir.UpdateOp) => boolean,
-  transform?: (ops: Array<ir.UpdateOp>) => Array<ir.UpdateOp>
-}[] =
-    [
-      {
-        test: (op: ir.UpdateOp) =>
-            op.kind === ir.OpKind.HostProperty && op.expression instanceof ir.Interpolation
-      },
-      {
-        test: (op: ir.UpdateOp) =>
-            op.kind === ir.OpKind.HostProperty && !(op.expression instanceof ir.Interpolation)
-      },
-      {test: kindTest(ir.OpKind.StyleMap), transform: keepLast},
-      {test: kindTest(ir.OpKind.ClassMap), transform: keepLast},
-      {test: kindTest(ir.OpKind.StyleProp)},
-      {test: kindTest(ir.OpKind.ClassProp)},
-      {
-        test: (op: ir.UpdateOp) =>
-            op.kind === ir.OpKind.Property && op.expression instanceof ir.Interpolation
-      },
-      {
-        test: (op: ir.UpdateOp) =>
-            op.kind === ir.OpKind.Property && !(op.expression instanceof ir.Interpolation)
-      },
-      {test: kindTest(ir.OpKind.Attribute)},
-    ];
-
-/**
- * The set of all op kinds we handle in the reordering phase.
- */
-const handledOpKinds = new Set([
-  ir.OpKind.StyleMap,
-  ir.OpKind.ClassMap,
-  ir.OpKind.StyleProp,
-  ir.OpKind.ClassProp,
-  ir.OpKind.Property,
-  ir.OpKind.HostProperty,
-  ir.OpKind.Attribute,
-]);
-
-/**
- * Reorders property and attribute ops according to the following ordering:
- * 1. styleMap & styleMapInterpolate (drops all but the last op in the group)
- * 2. classMap & classMapInterpolate (drops all but the last op in the group)
- * 3. styleProp & stylePropInterpolate (ordering preserved within group)
- * 4. classProp (ordering preserved within group)
- * 5. propertyInterpolate (ordering preserved within group)
- * 6. property (ordering preserved within group)
- * 7. attribute & attributeInterpolate (ordering preserve within group)
- */
-export function phasePropertyOrdering(cpl: CompilationJob) {
-  for (const unit of cpl.units) {
-    let opsToOrder = [];
-    for (const op of unit.update) {
-      if (handledOpKinds.has(op.kind)) {
-        // Pull out ops that need o be ordered.
-        opsToOrder.push(op);
-        ir.OpList.remove(op);
-      } else {
-        // When we encounter an op that shouldn't be reordered, put the ones we've pulled so far
-        // back in the correct order.
-        for (const orderedOp of reorder(opsToOrder)) {
-          ir.OpList.insertBefore(orderedOp, op);
-        }
-        opsToOrder = [];
-      }
-    }
-    // If we still have ops pulled at the end, put them back in the correct order.
-    for (const orderedOp of reorder(opsToOrder)) {
-      unit.update.push(orderedOp);
-    }
-  }
-}
-
-/**
- * Reorders the given list of ops according to the ordering defined by `ORDERING`.
- */
-function reorder(ops: Array<ir.UpdateOp>): Array<ir.UpdateOp> {
-  // Break the ops list into groups based on OpKind.
-  const groups = Array.from(ORDERING, () => new Array<ir.UpdateOp>());
-  for (const op of ops) {
-    const groupIndex = ORDERING.findIndex(o => o.test(op));
-    groups[groupIndex].push(op);
-  }
-  // Reassemble the groups into a single list, in the correct order.
-  return groups.flatMap((group, i) => {
-    const transform = ORDERING[i].transform;
-    return transform ? transform(group) : group;
-  });
-}
-
-/**
- * Keeps only the last op in a list of ops.
- */
-function keepLast<T>(ops: Array<T>) {
-  return ops.slice(ops.length - 1);
-}