refactor(compiler): Keep a TemplateKind on various binding ops (#53405)

Previously, binding ops only knew whether they applied to a structural template (and even this was actually very misleading!). Now, binding ops have full information about what kind of template they apply to, if any (e.g. plain template, structural template, etc). Additionally, each binding knows whether it `IsStructuralTemplateAttribute`, which is a property of the binding rather than the template target. In the future, we should refactor this to unify the various flags that can describe binding types, as well as the flags that describe template targets, into a single and comprehensive field on binding ops. PR Close #53405
2026-09-14 13:54:52 +08:00 · 2023-12-06 15:40:32 -08:00
parent 16b7fcaafd
commit 24177e973b
10 changed files with 130 additions and 48 deletions
@@ -54,10 +54,16 @@
          "extraChecks": [
            "verifyPlaceholdersIntegrity",
            "verifyUniqueConsts"
+          ],
+          "files": [
+            {
+              "expected": "ng-template_interpolation_template.js",
+              "templatePipelineExpected": "ng-template_interpolation_template.pipeline.js",
+              "generated": "ng-template_interpolation.js"
+            }
          ]
        }
-      ],
-      "skipForTemplatePipeline": true
+      ]
    },
    {
      "description": "should support i18n attributes with interpolations on explicit <ng-template> elements with structural directives",
@@ -69,10 +75,16 @@
          "extraChecks": [
            "verifyPlaceholdersIntegrity",
            "verifyUniqueConsts"
+          ],
+          "files": [
+            {
+              "expected": "ng-template_interpolation_structural_template.js",
+              "templatePipelineExpected": "ng-template_interpolation_structural_template.pipeline.js",
+              "generated": "ng-template_interpolation_structural.js"
+            }
          ]
        }
-      ],
-      "skipForTemplatePipeline": true
+      ]
    },
    {
      "description": "should not create translations for empty attributes",
@@ -0,0 +1,29 @@
+function MyComponent_0_Template(rf, ctx) {
+	if (rf & 1) {
+	  $r3$.ɵɵtemplate(0, MyComponent_0_ng_template_0_Template, 0, 0, "ng-template", 2);
+	  $r3$.ɵɵi18nAttributes(1, 0);
+	}
+	if (rf & 2) {
+	  const $ctx_r2$ = $r3$.ɵɵnextContext();
+	  $r3$.ɵɵi18nExp($ctx_r2$.name);
+	  $r3$.ɵɵi18nApply(1);
+	}
+  }
+  …
+  consts: () => {
+	__i18nMsg__('Hello {$interpolation}', [['interpolation', String.raw`\uFFFD0\uFFFD`]], {original_code: {'interpolation': '{{ name }}'}}, {})
+	return [
+	  ["title", $i18n_0$],
+	  [__AttributeMarker.Template__, "ngIf"],
+	  [__AttributeMarker.Bindings__, "title"]
+	];
+  },
+  template: function MyComponent_Template(rf, ctx) {
+	if (rf & 1) {
+	  $r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 1, null, 1);
+	}
+	if (rf & 2) {
+	  $r3$.ɵɵproperty("ngIf", true);
+	}
+  },
+  
@@ -0,0 +1,18 @@
+consts: () => {
+	__i18nMsg__('Hello {$interpolation}', [['interpolation', String.raw`\uFFFD0\uFFFD`]], {original_code: {'interpolation': '{{ name }}'}}, {})
+	return [
+	   ["title", $i18n_0$],
+	   [__AttributeMarker.Bindings__, "title"]
+	];
+  },
+  template: function MyComponent_Template(rf, ctx) {
+	if (rf & 1) {
+	  $r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 1);
+	  $r3$.ɵɵi18nAttributes(1, 0);
+	}
+	if (rf & 2) {
+	  $r3$.ɵɵi18nExp(ctx.name);
+	  $r3$.ɵɵi18nApply(1);
+	}
+  }
+  
@@ -10,7 +10,7 @@ import {SecurityContext} from '../../../../../core';
 import * as i18n from '../../../../../i18n/i18n_ast';
 import * as o from '../../../../../output/output_ast';
 import {ParseSourceSpan} from '../../../../../parse_util';
-import {BindingKind, I18nExpressionFor, I18nParamResolutionTime, OpKind} from '../enums';
+import {BindingKind, I18nExpressionFor, I18nParamResolutionTime, OpKind, TemplateKind} from '../enums';
 import type {ConditionalCaseExpr} from '../expression';
 import {SlotHandle} from '../handle';
 import {Op, XrefId} from '../operations';
@@ -121,10 +121,12 @@ export interface BindingOp extends Op<UpdateOp> {
   */
  isTextAttribute: boolean;

+  isStructuralTemplateAttribute: boolean;
+
  /**
   * Whether this binding is on a structural template.
   */
-  isStructuralTemplate: boolean;
+  templateKind: TemplateKind|null;

  i18nContext: XrefId|null;
  i18nMessage: i18n.Message|null;
@@ -138,8 +140,8 @@ export interface BindingOp extends Op<UpdateOp> {
 export function createBindingOp(
    target: XrefId, kind: BindingKind, name: string, expression: o.Expression|Interpolation,
    unit: string|null, securityContext: SecurityContext, isTextAttribute: boolean,
-    isStructuralTemplate: boolean, i18nMessage: i18n.Message|null,
-    sourceSpan: ParseSourceSpan): BindingOp {
+    isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null,
+    i18nMessage: i18n.Message|null, sourceSpan: ParseSourceSpan): BindingOp {
  return {
    kind: OpKind.Binding,
    bindingKind: kind,
@@ -149,7 +151,8 @@ export function createBindingOp(
    unit,
    securityContext,
    isTextAttribute,
-    isStructuralTemplate: isStructuralTemplate,
+    isStructuralTemplateAttribute,
+    templateKind,
    i18nContext: null,
    i18nMessage,
    sourceSpan,
@@ -193,10 +196,13 @@ export interface PropertyOp extends Op<UpdateOp>, ConsumesVarsTrait, DependsOnSl
   */
  sanitizer: o.Expression|null;

+  isStructuralTemplateAttribute: boolean;
+
  /**
-   * Whether this binding is on a structural template.
+   * The kind of template targeted by the binding, or null if this binding does not target a
+   * template.
   */
-  isStructuralTemplate: boolean;
+  templateKind: TemplateKind|null;

  i18nContext: XrefId|null;
  i18nMessage: i18n.Message|null;
@@ -209,7 +215,8 @@ export interface PropertyOp extends Op<UpdateOp>, ConsumesVarsTrait, DependsOnSl
 */
 export function createPropertyOp(
    target: XrefId, name: string, expression: o.Expression|Interpolation,
-    isAnimationTrigger: boolean, securityContext: SecurityContext, isStructuralTemplate: boolean,
+    isAnimationTrigger: boolean, securityContext: SecurityContext,
+    isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null,
    i18nContext: XrefId|null, i18nMessage: i18n.Message|null,
    sourceSpan: ParseSourceSpan): PropertyOp {
  return {
@@ -220,7 +227,8 @@ export function createPropertyOp(
    isAnimationTrigger,
    securityContext,
    sanitizer: null,
-    isStructuralTemplate,
+    isStructuralTemplateAttribute,
+    templateKind,
    i18nContext,
    i18nMessage,
    sourceSpan,
@@ -424,10 +432,13 @@ export interface AttributeOp extends Op<UpdateOp> {
   */
  isTextAttribute: boolean;

+  isStructuralTemplateAttribute: boolean;
+
  /**
-   * Whether this binding is on a structural template.
+   * The kind of template targeted by the binding, or null if this binding does not target a
+   * template.
   */
-  isStructuralTemplate: boolean;
+  templateKind: TemplateKind|null;

  /**
   * The i18n context, if this is an i18n attribute.
@@ -444,7 +455,8 @@ export interface AttributeOp extends Op<UpdateOp> {
 */
 export function createAttributeOp(
    target: XrefId, name: string, expression: o.Expression|Interpolation,
-    securityContext: SecurityContext, isTextAttribute: boolean, isStructuralTemplate: boolean,
+    securityContext: SecurityContext, isTextAttribute: boolean,
+    isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null,
    i18nMessage: i18n.Message|null, sourceSpan: ParseSourceSpan): AttributeOp {
  return {
    kind: OpKind.Attribute,
@@ -454,7 +466,8 @@ export function createAttributeOp(
    securityContext,
    sanitizer: null,
    isTextAttribute,
-    isStructuralTemplate,
+    isStructuralTemplateAttribute,
+    templateKind,
    i18nContext: null,
    i18nMessage,
    sourceSpan,
@@ -510,7 +523,8 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
  targetSlot: SlotHandle;

  /**
-   * The main test expression (for a switch), or `null` (for an if, which has no test expression).
+   * The main test expression (for a switch), or `null` (for an if, which has no test
+   * expression).
   */
  test: o.Expression|null;

@@ -527,8 +541,8 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
  processed: o.Expression|null;

  /**
-   * Control flow conditionals can accept a context value (this is a result of specifying an alias).
-   * This expression will be passed to the conditional instruction's context parameter.
+   * Control flow conditionals can accept a context value (this is a result of specifying an
+   * alias). This expression will be passed to the conditional instruction's context parameter.
   */
  contextValue: o.Expression|null;

@@ -626,8 +640,8 @@ export function createDeferWhenOp(
 /**
 * An op that represents an expression in an i18n message.
 *
- * TODO: This can represent expressions used in both i18n attributes and normal i18n content. We may
- * want to split these into two different op types, deriving from the same base class.
+ * TODO: This can represent expressions used in both i18n attributes and normal i18n content. We
+ * may want to split these into two different op types, deriving from the same base class.
 */
 export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
                                          DependsOnSlotContextOpTrait {
@@ -641,11 +655,11 @@ export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
  /**
   * The Xref of the op that we need to `advance` to.
   *
-   * In an i18n block, this is initially the i18n start op, but will eventually correspond to the
-   * final slot consumer in the owning i18n block.
-   * TODO: We should make text i18nExpressions target the i18nEnd instruction, instead the last slot
-   * consumer in the i18n block. This makes them resilient to that last consumer being deleted. (Or
-   * new slot consumers being added!)
+   * In an i18n block, this is initially the i18n start op, but will eventually correspond to
+   * the final slot consumer in the owning i18n block.
+   * TODO: We should make text i18nExpressions target the i18nEnd instruction, instead the last
+   * slot consumer in the i18n block. This makes them resilient to that last consumer being
+   * deleted. (Or new slot consumers being added!)
   *
   * In an i18n attribute, this is the xref of the corresponding elementStart/element.
   */
@@ -733,9 +747,9 @@ export interface I18nApplyOp extends Op<UpdateOp> {
  owner: XrefId;

  /**
-   * A handle for the slot that i18n apply instruction should apply to. In an i18n block, this is
-   * the slot of the i18n block this expression belongs to. In an i18n attribute, this is the slot
-   * of the corresponding i18nAttributes instruction.
+   * A handle for the slot that i18n apply instruction should apply to. In an i18n block, this
+   * is the slot of the i18n block this expression belongs to. In an i18n attribute, this is the
+   * slot of the corresponding i18nAttributes instruction.
   */
  handle: SlotHandle;

@@ -92,7 +92,7 @@ export function ingestHostProperty(
      job.root.xref, bindingKind, property.name, expression, null,
      SecurityContext
          .NONE /* TODO: what should we pass as security context? Passing NONE for now. */,
-      isTextAttribute, false, /* TODO: How do Host bindings handle i18n attrs? */ null,
+      isTextAttribute, false, null, /* TODO: How do Host bindings handle i18n attrs? */ null,
      property.sourceSpan));
 }

@@ -100,6 +100,7 @@ export function ingestHostAttribute(
    job: HostBindingCompilationJob, name: string, value: o.Expression): void {
  const attrBinding = ir.createBindingOp(
      job.root.xref, ir.BindingKind.Attribute, name, value, null, SecurityContext.NONE, true, false,
+      null,
      /* TODO */ null,
      /* TODO: host attribute source spans */ null!);
  job.root.update.push(attrBinding);
@@ -165,7 +166,7 @@ function ingestElement(unit: ViewCompilationUnit, element: t.Element): void {
      element.startSourceSpan);
  unit.create.push(startOp);

-  ingestBindings(unit, startOp, element);
+  ingestBindings(unit, startOp, element, null);
  ingestReferences(startOp, element);

  // Start i18n, if needed, goes after the element create and bindings, but before the nodes
@@ -220,7 +221,7 @@ function ingestTemplate(unit: ViewCompilationUnit, tmpl: t.Template): void {
      i18nPlaceholder, tmpl.startSourceSpan);
  unit.create.push(templateOp);

-  ingestBindings(unit, templateOp, tmpl);
+  ingestBindings(unit, templateOp, tmpl, templateKind);
  ingestReferences(templateOp, tmpl);
  ingestNodes(childView, tmpl.children);

@@ -251,7 +252,7 @@ function ingestContent(unit: ViewCompilationUnit, content: t.Content): void {
  for (const attr of content.attributes) {
    ingestBinding(
        unit, op.xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
-        SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, attr.i18n);
+        SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, null, attr.i18n);
  }
  unit.create.push(op);
 }
@@ -754,7 +755,8 @@ function isPlainTemplate(tmpl: t.Template) {
 * to their IR representation.
 */
 function ingestBindings(
-    unit: ViewCompilationUnit, op: ir.ElementOpBase, element: t.Element|t.Template): void {
+    unit: ViewCompilationUnit, op: ir.ElementOpBase, element: t.Element|t.Template,
+    templateKind: ir.TemplateKind|null): void {
  let flags: BindingFlags = BindingFlags.None;
  let hasI18nAttributes = false;

@@ -771,12 +773,12 @@ function ingestBindings(
        ingestBinding(
            unit, op.xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
            SecurityContext.NONE, attr.sourceSpan, templateAttrFlags | BindingFlags.TextValue,
-            attr.i18n);
+            templateKind, attr.i18n);
        hasI18nAttributes ||= attr.i18n !== undefined;
      } else {
        ingestBinding(
            unit, op.xref, attr.name, attr.value, attr.type, attr.unit, attr.securityContext,
-            attr.sourceSpan, templateAttrFlags, attr.i18n);
+            attr.sourceSpan, templateAttrFlags, templateKind, attr.i18n);
        hasI18nAttributes ||= attr.i18n !== undefined;
      }
    }
@@ -788,13 +790,14 @@ function ingestBindings(
    // `BindingType.Attribute`.
    ingestBinding(
        unit, op.xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
-        SecurityContext.NONE, attr.sourceSpan, flags | BindingFlags.TextValue, attr.i18n);
+        SecurityContext.NONE, attr.sourceSpan, flags | BindingFlags.TextValue, templateKind,
+        attr.i18n);
    hasI18nAttributes ||= attr.i18n !== undefined;
  }
  for (const input of element.inputs) {
    ingestBinding(
        unit, op.xref, input.name, input.value, input.type, input.unit, input.securityContext,
-        input.sourceSpan, flags, input.i18n);
+        input.sourceSpan, flags, templateKind, input.i18n);
    hasI18nAttributes ||= input.i18n !== undefined;
  }

@@ -888,7 +891,8 @@ enum BindingFlags {
 function ingestBinding(
    view: ViewCompilationUnit, xref: ir.XrefId, name: string, value: e.AST|o.Expression,
    type: e.BindingType, unit: string|null, securityContext: SecurityContext,
-    sourceSpan: ParseSourceSpan, flags: BindingFlags, i18nMeta: i18n.I18nMeta|undefined): void {
+    sourceSpan: ParseSourceSpan, flags: BindingFlags, templateKind: ir.TemplateKind|null,
+    i18nMeta: i18n.I18nMeta|undefined): void {
  if (value instanceof e.ASTWithSource) {
    value = value.ast;
  }
@@ -926,7 +930,8 @@ function ingestBinding(
  const kind: ir.BindingKind = BINDING_KINDS.get(type)!;
  view.update.push(ir.createBindingOp(
      xref, kind, name, expression, unit, securityContext, !!(flags & BindingFlags.TextValue),
-      !!(flags & BindingFlags.IsStructuralTemplateAttribute), i18nMeta ?? null, sourceSpan));
+      !!(flags & BindingFlags.IsStructuralTemplateAttribute), templateKind, i18nMeta ?? null,
+      sourceSpan));
 }

 /**
@@ -1026,7 +1031,8 @@ function ingestControlFlowInsertionPoint(
    for (const attr of root.attributes) {
      ingestBinding(
          unit, xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
-          SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, attr.i18n);
+          SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, ir.TemplateKind.Block,
+          attr.i18n);
    }

    const tagName = root instanceof t.Element ? root.name : root.tagName;
@@ -26,11 +26,11 @@ export function extractAttributes(job: CompilationJob): void {
        case ir.OpKind.Property:
          if (!op.isAnimationTrigger) {
            let bindingKind: ir.BindingKind;
-            if (op.i18nMessage !== null) {
+            if (op.i18nMessage !== null && op.templateKind === null) {
              // If the binding has an i18n context, it is an i18n attribute, and should have that
              // kind in the consts array.
              bindingKind = ir.BindingKind.I18n;
-            } else if (op.isStructuralTemplate) {
+            } else if (op.isStructuralTemplateAttribute) {
              // TODO: How do i18n attributes on templates work?!
              bindingKind = ir.BindingKind.Template;
            } else {
@@ -119,7 +119,8 @@ function extractAttributeOp(

  if (extractable) {
    const extractedAttributeOp = ir.createExtractedAttributeOp(
-        op.target, op.isStructuralTemplate ? ir.BindingKind.Template : ir.BindingKind.Attribute,
+        op.target,
+        op.isStructuralTemplateAttribute ? ir.BindingKind.Template : ir.BindingKind.Attribute,
        op.name, op.expression, op.i18nContext, op.i18nMessage);
    if (unit.job.kind === CompilationJobKind.Host) {
      // This attribute will apply to the enclosing host binding compilation unit, so order doesn't
@@ -48,7 +48,8 @@ export function specializeBindings(job: CompilationJob): void {
                op,
                ir.createAttributeOp(
                    op.target, op.name, op.expression, op.securityContext, op.isTextAttribute,
-                    op.isStructuralTemplate, op.i18nMessage, op.sourceSpan));
+                    op.isStructuralTemplateAttribute, op.templateKind, op.i18nMessage,
+                    op.sourceSpan));
          }
          break;
        case ir.BindingKind.Property:
@@ -64,8 +65,8 @@ export function specializeBindings(job: CompilationJob): void {
                op,
                ir.createPropertyOp(
                    op.target, op.name, op.expression, op.bindingKind === ir.BindingKind.Animation,
-                    op.securityContext, op.isStructuralTemplate, op.i18nContext, op.i18nMessage,
-                    op.sourceSpan));
+                    op.securityContext, op.isStructuralTemplateAttribute, op.templateKind,
+                    op.i18nContext, op.i18nMessage, op.sourceSpan));
          }

          break;
@@ -105,6 +105,7 @@ class ElementAttributes {
      return;
    }
    this.known.add(name);
+    // TODO: Can this be its own phase
    if (name === 'ngProjectAs') {
      if (value === null || !(value instanceof o.LiteralExpr) || (value.value == null) ||
          (typeof value.value?.toString() !== 'string')) {