refactor(compiler): Introduce ConstCollectedExpr, and use it in defer on expressions (angular#52387)

Previously, we supported a `HasConst` trait, allowing an op to be const collected automatically. However, that approach had the shortcoming that each op could only collect a single constant.

Instead, we now provide a `ConstCollectedExpr`, which collects constants at the expression level, allowing ops to have multiple collectible consts.

Then, we use this new abstraction to support the `defer on` conditions.

PR Close angular#52387

Author: dylhunn

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

@@ -16,8 +16,7 @@
           ],
           "failureMessage": "Incorrect template"
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate a deferred block with secondary blocks",
@@ -34,8 +33,7 @@
           ],
           "failureMessage": "Incorrect template"
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate a deferred block with placeholder block parameters",
@@ -181,8 +179,7 @@
           ],
           "failureMessage": "Incorrect template"
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate a deferred block with an interaction trigger in a parent view",
@@ -199,8 +196,7 @@
           ],
           "failureMessage": "Incorrect template"
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate a deferred block with an interaction trigger inside the placeholder",
@@ -217,8 +213,7 @@
           ],
           "failureMessage": "Incorrect template"
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should generate a deferred block with implicit trigger references",

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

@@ -355,6 +355,11 @@ export enum ExpressionKind {
    * properties ($even, $first, etc.).
    */
   DerivedRepeaterVar,
+
+  /**
+   * An expression that will be automatically extracted to the component const array.
+   */
+  ConstCollected,
 }
 
 export enum VariableFlags {

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

@@ -24,7 +24,7 @@ export type Expression = LexicalReadExpr|ReferenceExpr|ContextExpr|NextContextEx
     GetCurrentViewExpr|RestoreViewExpr|ResetViewExpr|ReadVariableExpr|PureFunctionExpr|
     PureFunctionParameterExpr|PipeBindingExpr|PipeBindingVariadicExpr|SafePropertyReadExpr|
     SafeKeyedReadExpr|SafeInvokeFunctionExpr|EmptyExpr|AssignTemporaryExpr|ReadTemporaryExpr|
-    SanitizerExpr|SlotLiteralExpr|ConditionalCaseExpr|DerivedRepeaterVarExpr;
+    SanitizerExpr|SlotLiteralExpr|ConditionalCaseExpr|DerivedRepeaterVarExpr|ConstCollectedExpr;
 
 /**
  * Transformer type which converts expressions into general `o.Expression`s (which may be an
@@ -853,6 +853,38 @@ export class DerivedRepeaterVarExpr extends ExpressionBase {
   }
 }
 
+export class ConstCollectedExpr extends ExpressionBase {
+  override readonly kind = ExpressionKind.ConstCollected;
+
+  constructor(public expr: o.Expression) {
+    super();
+  }
+
+  override transformInternalExpressions(transform: ExpressionTransform, flags: VisitorContextFlag):
+      void {
+    this.expr = transform(this.expr, flags);
+  }
+
+  override visitExpression(visitor: o.ExpressionVisitor, context: any) {
+    this.expr.visitExpression(visitor, context);
+  }
+
+  override isEquivalent(e: o.Expression): boolean {
+    if (!(e instanceof ConstCollectedExpr)) {
+      return false;
+    }
+    return this.expr.isEquivalent(e.expr);
+  }
+
+  override isConstant(): boolean {
+    return this.expr.isConstant();
+  }
+
+  override clone(): ConstCollectedExpr {
+    return new ConstCollectedExpr(this.expr);
+  }
+}
+
 /**
  * Visits all `Expression`s in the AST of `op` with the `visitor` function.
  */
@@ -953,11 +985,19 @@ export function transformExpressionsInOp(
     case OpKind.Repeater:
       op.collection = transformExpressionsInExpression(op.collection, transform, flags);
       break;
+    case OpKind.Defer:
+      if (op.loadingConfig !== null) {
+        op.loadingConfig = transformExpressionsInExpression(op.loadingConfig, transform, flags);
+      }
+      if (op.placeholderConfig !== null) {
+        op.placeholderConfig =
+            transformExpressionsInExpression(op.placeholderConfig, transform, flags);
+      }
+      break;
     case OpKind.Advance:
     case OpKind.Container:
     case OpKind.ContainerEnd:
     case OpKind.ContainerStart:
-    case OpKind.Defer:
     case OpKind.DeferOn:
     case OpKind.DisableBindings:
     case OpKind.Element:

packages/compiler/src/template/pipeline/ir/src/ops/create.ts

@@ -679,12 +679,18 @@ export interface DeferOp extends Op<CreateOp>, ConsumesSlotOpTrait {
 
   errorSlot: SlotHandle|null;
 
+  placeholderMinimumTime: number|null;
+  loadingMinimumTime: number|null;
+  loadingAfterTime: number|null;
+
+  placeholderConfig: o.Expression|null;
+  loadingConfig: o.Expression|null;
+
   sourceSpan: ParseSourceSpan;
 }
 
 export function createDeferOp(
-    xref: XrefId, main: XrefId, mainSlot: SlotHandle, placeholderView: XrefId|null,
-    placeholderSlot: SlotHandle|null, sourceSpan: ParseSourceSpan): DeferOp {
+    xref: XrefId, main: XrefId, mainSlot: SlotHandle, sourceSpan: ParseSourceSpan): DeferOp {
   return {
     kind: OpKind.Defer,
     xref,
@@ -693,13 +699,19 @@ export function createDeferOp(
     mainSlot,
     loadingView: null,
     loadingSlot: null,
+    loadingConfig: null,
+    loadingMinimumTime: null,
+    loadingAfterTime: null,
     placeholderView: null,
-    placeholderSlot,
+    placeholderSlot: null,
+    placeholderConfig: null,
+    placeholderMinimumTime: null,
     errorView: null,
     errorSlot: null,
     sourceSpan,
     ...NEW_OP,
     ...TRAIT_CONSUMES_SLOT,
+    numSlotsUsed: 2,
   };
 }
 interface DeferTriggerBase {
@@ -724,7 +736,7 @@ interface DeferInteractionTrigger extends DeferTriggerBase {
    * The slot index of the named reference, inside the view provided below. This slot may not be
    * inside the current view, and is handled specially as a result.
    */
-  targetSlot: number|null;
+  targetSlot: SlotHandle|null;
 
   targetView: XrefId|null;
 

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

@@ -32,11 +32,6 @@ export const ConsumesVarsTrait = Symbol('ConsumesVars');
  */
 export const UsesVarOffset = Symbol('UsesVarOffset');
 
-/**
- * Marker symbol for `HasConst` trait.
- */
-export const HasConst = Symbol('HasConst');
-
 /**
  * Marks an operation as requiring allocation of one or more data slots for storage.
  */
@@ -102,30 +97,6 @@ export interface UsesVarOffsetTrait {
   varOffset: number|null;
 }
 
-/**
- * Marker trait indicating that an op or expression has some data which should be collected into the
- * component constant array.
- *
- */
-export interface HasConstTrait {
-  [HasConst]: true;
-
-  /**
-   * The constant to be collected into the const array, if non-null.
-   */
-  constValue: unknown|null;
-
-  /**
-   * The index of the collected constant, after processing.
-   */
-  constIndex: number|null;
-
-  /**
-   * A callback which converts the constValue into an o.Expression for the const array.
-   */
-  makeExpression: (value: unknown) => o.Expression;
-}
-
 /**
  * Default values for most `ConsumesSlotOpTrait` fields (used with the spread operator to initialize
  * implementors of the trait).
@@ -161,15 +132,6 @@ export const TRAIT_USES_VAR_OFFSET: UsesVarOffsetTrait = {
   varOffset: null,
 } as const;
 
-/**
- * Default values for `HasConst` fields (used with the spread operator to initialize
- * implementors of this trait).
- */
-export const TRAIT_HAS_CONST: Omit<HasConstTrait, 'constValue'|'makeExpression'> = {
-  [HasConst]: true,
-  constIndex: null,
-} as const;
-
 /**
  * Test whether an operation implements `ConsumesSlotOpTrait`.
  */
@@ -202,12 +164,3 @@ export function hasUsesVarOffsetTrait<ExprT extends Expression>(expr: ExprT): ex
     UsesVarOffsetTrait {
   return (expr as Partial<UsesVarOffsetTrait>)[UsesVarOffset] === true;
 }
-
-/**
- * Test whether an operation or expression implements `HasConstTrait`.
- */
-export function hasConstTrait<ExprT extends Expression>(expr: ExprT): expr is ExprT&HasConstTrait;
-export function hasConstTrait<OpT extends Op<OpT>>(op: OpT): op is OpT&HasConstTrait;
-export function hasConstTrait(value: any): boolean {
-  return (value as Partial<HasConstTrait>)[HasConst] === true;
-}

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

@@ -54,9 +54,11 @@ export function prefixWithNamespace(strippedTag: string, namespace: ir.Namespace
   return `:${keyForNamespace(namespace)}:${strippedTag}`;
 }
 
-export function literalOrArrayLiteral(value: any): o.Expression {
+export type LiteralType = string|number|boolean|null|Array<LiteralType>;
+
+export function literalOrArrayLiteral(value: LiteralType): o.Expression {
   if (Array.isArray(value)) {
     return o.literalArr(value.map(literalOrArrayLiteral));
   }
-  return o.literal(value, o.INFERRED_TYPE);
+  return o.literal(value);
 }

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

@@ -28,7 +28,7 @@ import {phaseFormatI18nParams} from './phases/format_i18n_params';
 import {phaseGenerateAdvance} from './phases/generate_advance';
 import {phaseGenerateProjectionDef} from './phases/generate_projection_def';
 import {phaseGenerateVariables} from './phases/generate_variables';
-import {phaseConstTraitCollection} from './phases/has_const_trait_collection';
+import {phaseConstExpressionCollection} from './phases/has_const_expression_collection';
 import {phaseHostStylePropertyParsing} from './phases/host_style_property_parsing';
 import {phaseI18nConstCollection} from './phases/i18n_const_collection';
 import {phaseI18nMessageExtraction} from './phases/i18n_message_extraction';
@@ -69,6 +69,8 @@ import {phaseTrackVariables} from './phases/track_variables';
 import {phaseVarCounting} from './phases/var_counting';
 import {phaseVariableOptimization} from './phases/variable_optimization';
 import {phaseWrapIcus} from './phases/wrap_icus';
+import {phaseDeferResolveTargets} from './phases/defer_resolve_targets';
+import {phaseDeferConfigs} from './phases/defer_configs';
 
 type Phase = {
   fn: (job: CompilationJob) => void; kind: Kind.Both | Kind.Host | Kind.Tmpl;
@@ -96,6 +98,7 @@ const phases: Phase[] = [
   {kind: Kind.Both, fn: phaseOrdering},
   {kind: Kind.Tmpl, fn: phaseConditionals},
   {kind: Kind.Tmpl, fn: phasePipeCreation},
+  {kind: Kind.Tmpl, fn: phaseDeferConfigs},
   {kind: Kind.Tmpl, fn: phaseI18nTextExtraction},
   {kind: Kind.Tmpl, fn: phaseIcuExtraction},
   {kind: Kind.Tmpl, fn: phaseApplyI18nExpressions},
@@ -109,6 +112,7 @@ const phases: Phase[] = [
   {kind: Kind.Tmpl, fn: phaseRepeaterDerivedVars},
   {kind: Kind.Tmpl, fn: phaseTrackVariables},
   {kind: Kind.Both, fn: phaseResolveNames},
+  {kind: Kind.Tmpl, fn: phaseDeferResolveTargets},
   {kind: Kind.Tmpl, fn: phaseTrackFnOptimization},
   {kind: Kind.Both, fn: phaseResolveContexts},
   {kind: Kind.Tmpl, fn: phaseResolveSanitizers},  // TODO: run in both
@@ -124,7 +128,7 @@ const phases: Phase[] = [
   {kind: Kind.Tmpl, fn: phaseFormatI18nParams},
   {kind: Kind.Tmpl, fn: phaseTrackFnGeneration},
   {kind: Kind.Tmpl, fn: phaseI18nConstCollection},
-  {kind: Kind.Tmpl, fn: phaseConstTraitCollection},
+  {kind: Kind.Tmpl, fn: phaseConstExpressionCollection},
   {kind: Kind.Both, fn: phaseConstCollection},
   {kind: Kind.Tmpl, fn: phaseAssignI18nSlotDependencies},
   {kind: Kind.Both, fn: phaseVarCounting},

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

@@ -363,9 +363,14 @@ function ingestDeferBlock(unit: ViewCompilationUnit, deferBlock: t.DeferredBlock
 
   // Create the main defer op, and ops for all secondary views.
   const deferXref = unit.job.allocateXrefId();
-  const deferOp = ir.createDeferOp(
-      deferXref, main.xref, main.slot, placeholder?.xref ?? null, placeholder?.slot ?? null,
-      deferBlock.sourceSpan);
+  const deferOp = ir.createDeferOp(deferXref, main.xref, main.slot, deferBlock.sourceSpan);
+  deferOp.placeholderView = placeholder?.xref ?? null;
+  deferOp.placeholderSlot = placeholder?.slot ?? null;
+  deferOp.loadingSlot = loading?.slot ?? null;
+  deferOp.errorSlot = error?.slot ?? null;
+  deferOp.placeholderMinimumTime = deferBlock.placeholder?.minimumTime ?? null;
+  deferOp.loadingMinimumTime = deferBlock.loading?.minimumTime ?? null;
+  deferOp.loadingAfterTime = deferBlock.loading?.afterTime ?? null;
   unit.create.push(deferOp);
 
   // Configure all defer `on` conditions.

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

@@ -185,20 +185,22 @@ export function text(
 
 export function defer(
     selfSlot: number, primarySlot: number, dependencyResolverFn: null, loadingSlot: number|null,
-    placeholderSlot: number|null, errorSlot: number|null, loadingConfigIndex: number|null,
-    placeholderConfigIndex: number|null, sourceSpan: ParseSourceSpan|null): ir.CreateOp {
-  const args = [
+    placeholderSlot: number|null, errorSlot: number|null, loadingConfig: o.Expression|null,
+    placeholderConfig: o.Expression|null, sourceSpan: ParseSourceSpan|null): ir.CreateOp {
+  const args: Array<o.Expression> = [
     o.literal(selfSlot),
     o.literal(primarySlot),
     o.literal(dependencyResolverFn),
     o.literal(loadingSlot),
     o.literal(placeholderSlot),
     o.literal(errorSlot),
-    o.literal(loadingConfigIndex),
-    o.literal(placeholderConfigIndex),
+    loadingConfig ?? o.literal(null),
+    placeholderConfig ?? o.literal(null),
   ];
 
-  while (args[args.length - 1].value === null) {
+  let expr: o.Expression;
+  while ((expr = args[args.length - 1]) !== null && expr instanceof o.LiteralExpr &&
+         expr.value === null) {
     args.pop();
   }
 

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

@@ -0,0 +1,31 @@
+/**
+ * @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 o from '../../../../output/output_ast';
+import * as ir from '../../ir';
+import type {ViewCompilationUnit, ComponentCompilationJob} from '../compilation';
+import {literalOrArrayLiteral} from '../conversion';
+
+export function phaseDeferConfigs(job: ComponentCompilationJob): void {
+  for (const unit of job.units) {
+    for (const op of unit.create) {
+      if (op.kind !== ir.OpKind.Defer) {
+        continue;
+      }
+
+      if (op.placeholderMinimumTime !== null) {
+        op.placeholderConfig =
+            new ir.ConstCollectedExpr(literalOrArrayLiteral([op.placeholderMinimumTime]));
+      }
+      if (op.loadingMinimumTime !== null || op.loadingAfterTime !== null) {
+        op.loadingConfig = new ir.ConstCollectedExpr(
+            literalOrArrayLiteral([op.loadingMinimumTime, op.loadingAfterTime]));
+      }
+    }
+  }
+}