refactor(compiler): Additional fixes for pipes in i18n (#51876)

Adds a new test to verify pipe behavior in i18n blocks, and makes
serveral fixes to allow the test to pass.

PR Close #51876

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

@@ -15,6 +15,20 @@
         }
       ],
       "skipForTemplatePipeline": true
+    },
+    {
+      "description": "should support i18n message with multiple pipes",
+      "inputFiles": [
+        "multiple_pipes.ts"
+      ],
+      "expectations": [
+        {
+          "extraChecks": [
+            "verifyPlaceholdersIntegrity",
+            "verifyUniqueConsts"
+          ]
+        }
+      ]
     }
   ]
 }

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/multiple_pipes.js

@@ -0,0 +1,31 @@
+consts: function() {
+  __i18nMsg__('{$interpolation} and {$interpolation_1}', [['interpolation', String.raw`\uFFFD0\uFFFD`], ['interpolation_1', String.raw`\uFFFD1\uFFFD`]], {original_code: {'interpolation': '{{ valueA | pipeA }}', 'interpolation_1': '{{ valueB | pipeB }}'}}, {})
+  __i18nMsg__('{$startTagSpan}{$interpolation}{$closeTagSpan} and {$interpolation_1} {$startTagSpan}and {$interpolation_2}{$closeTagSpan}', [['closeTagSpan', String.raw`[\uFFFD/#6\uFFFD|\uFFFD/#9\uFFFD]`], ['interpolation', String.raw`\uFFFD0\uFFFD`], ['interpolation_1', String.raw`\uFFFD1\uFFFD`], ['interpolation_2', String.raw`\uFFFD2\uFFFD`], ['startTagSpan', String.raw`[\uFFFD#6\uFFFD|\uFFFD#9\uFFFD]`]], {original_code: {'closeTagSpan': '</span>', 'interpolation': '{{ valueA | pipeA }}', 'interpolation_1': '{{ valueB | pipeB }}', 'interpolation_2': '{{ valueC | pipeC }}', 'startTagSpan': '<span>'}}, {})
+  …
+},
+template: function MyComponent_Template(rf, ctx) {
+  if (rf & 1) {
+    $r3$.ɵɵelementStart(0, "div");
+    $r3$.ɵɵi18n(1, 0);
+    $r3$.ɵɵpipe(2, "pipeA");
+    $r3$.ɵɵpipe(3, "pipeB");
+    $r3$.ɵɵelementEnd();
+    $r3$.ɵɵelementStart(4, "div");
+    $r3$.ɵɵi18nStart(5, 1);
+    $r3$.ɵɵelement(6, "span");
+    $r3$.ɵɵpipe(7, "pipeA");
+    $r3$.ɵɵpipe(8, "pipeB");
+    $r3$.ɵɵelement(9, "span");
+    $r3$.ɵɵpipe(10, "pipeC");
+    $r3$.ɵɵi18nEnd();
+    $r3$.ɵɵelementEnd();
+  }
+  if (rf & 2) {
+    $r3$.ɵɵadvance(3);
+    $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(2, …, ctx.valueA))($r3$.ɵɵpipeBind1(3, …, ctx.valueB));
+    $r3$.ɵɵi18nApply(1);
+    $r3$.ɵɵadvance(7);
+    $r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(7, …, ctx.valueA))($r3$.ɵɵpipeBind1(8, …, ctx.valueB))($r3$.ɵɵpipeBind1(10, …, ctx.valueC));
+    $r3$.ɵɵi18nApply(5);
+  }
+}

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/multiple_pipes.ts

@@ -0,0 +1,39 @@
+import {Component, NgModule, Pipe, PipeTransform} from '@angular/core';
+
+@Component({
+  selector: 'my-component',
+  template: `
+  <div i18n>{{ valueA | pipeA }} and {{ valueB | pipeB }}</div>
+  <div i18n><span>{{ valueA | pipeA }}</span> and {{ valueB | pipeB }} <span>and {{ valueC | pipeC }}</span></div>
+`,
+})
+export class MyComponent {
+  valueA: 0;
+  valueB: 0;
+  valueC: 0;
+}
+
+@Pipe({name: 'pipeA'})
+export class PipeA implements PipeTransform {
+  transform() {
+    return null;
+  }
+}
+
+@Pipe({name: 'pipeB'})
+export class PipeB implements PipeTransform {
+  transform() {
+    return null;
+  }
+}
+
+@Pipe({name: 'pipeC'})
+export class PipeC implements PipeTransform {
+  transform() {
+    return null;
+  }
+}
+
+@NgModule({declarations: [MyComponent, PipeA, PipeB, PipeC]})
+export class MyModule {
+}

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

@@ -542,6 +542,11 @@ export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
    */
   expression: o.Expression;
 
+  /**
+   * The i18n placeholder associated with this expression.
+   */
+  i18nPlaceholder: i18n.Placeholder;
+
   sourceSpan: ParseSourceSpan;
 }
 
@@ -549,11 +554,13 @@ export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
  * Create an i18n expression op.
  */
 export function createI18nExpressionOp(
-    target: XrefId, expression: o.Expression, sourceSpan: ParseSourceSpan): I18nExpressionOp {
+    target: XrefId, expression: o.Expression, i18nPlaceholder: i18n.Placeholder,
+    sourceSpan: ParseSourceSpan): I18nExpressionOp {
   return {
     kind: OpKind.I18nExpression,
     target,
     expression,
+    i18nPlaceholder,
     sourceSpan,
     ...NEW_OP,
     ...TRAIT_CONSUMES_VARS,
@@ -577,24 +584,16 @@ export interface I18nApplyOp extends Op<UpdateOp>, UsesSlotIndexTrait {
    */
   slot: number|null;
 
-  /**
-   * The i18n placeholders associated with this operation.
-   */
-  i18nPlaceholders: i18n.Placeholder[];
-
   sourceSpan: ParseSourceSpan;
 }
 
 /**
  * Creates an op to apply i18n expression ops.
  */
-export function createI18nApplyOp(
-    target: XrefId, i18nPlaceholders: i18n.Placeholder[],
-    sourceSpan: ParseSourceSpan): I18nApplyOp {
+export function createI18nApplyOp(target: XrefId, sourceSpan: ParseSourceSpan): I18nApplyOp {
   return {
     kind: OpKind.I18nApply,
     target,
-    i18nPlaceholders,
     sourceSpan,
     ...NEW_OP,
     ...TRAIT_USES_SLOT_INDEX,

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

@@ -14,6 +14,7 @@ import {CompilationJob, CompilationJobKind as Kind, type ComponentCompilationJob
 
 import {phaseAlignPipeVariadicVarOffset} from './phases/align_pipe_variadic_var_offset';
 import {phaseFindAnyCasts} from './phases/any_cast';
+import {phaseApplyI18nExpressions} from './phases/apply_i18n_expressions';
 import {phaseAttributeExtraction} from './phases/attribute_extraction';
 import {phaseBindingSpecialization} from './phases/binding_specialization';
 import {phaseChaining} from './phases/chaining';
@@ -81,6 +82,7 @@ const phases: Phase[] = [
   {kind: Kind.Tmpl, fn: phaseNoListenersOnTemplates},
   {kind: Kind.Tmpl, fn: phasePipeCreation},
   {kind: Kind.Tmpl, fn: phaseI18nTextExtraction},
+  {kind: Kind.Tmpl, fn: phaseApplyI18nExpressions},
   {kind: Kind.Tmpl, fn: phasePipeVariadic},
   {kind: Kind.Both, fn: phasePureLiteralStructures},
   {kind: Kind.Tmpl, fn: phaseGenerateProjectionDef},

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

@@ -0,0 +1,41 @@
+/**
+ * @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 {CompilationJob} from '../compilation';
+
+
+/**
+ * Adds apply operations after i18n expressions.
+ */
+export function phaseApplyI18nExpressions(job: CompilationJob): void {
+  for (const unit of job.units) {
+    for (const op of unit.update) {
+      // Only add apply after expressions that are not followed by more expressions.
+      if (op.kind === ir.OpKind.I18nExpression && needsApplication(op)) {
+        // TODO: what should be the source span for the apply op?
+        ir.OpList.insertAfter<ir.UpdateOp>(ir.createI18nApplyOp(op.target, null!), op);
+      }
+    }
+  }
+}
+
+/**
+ * Checks whether the given expression op needs to be followed with an apply op.
+ */
+function needsApplication(op: ir.I18nExpressionOp) {
+  // If the next op is not another expression, we need to apply.
+  if (op.next?.kind !== ir.OpKind.I18nExpression) {
+    return true;
+  }
+  // If the next op is an expression targeting a different i18n block, we need to apply.
+  if (op.next.target !== op.target) {
+    return true;
+  }
+  return false;
+}

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

@@ -12,14 +12,20 @@ import * as ir from '../../ir';
 import {CompilationJob} from '../compilation';
 
 const CHAINABLE = new Set([
-  R3.elementStart,
-  R3.elementEnd,
-  R3.element,
-  R3.property,
-  R3.hostProperty,
-  R3.syntheticHostProperty,
-  R3.styleProp,
   R3.attribute,
+  R3.classProp,
+  R3.element,
+  R3.elementContainer,
+  R3.elementContainerEnd,
+  R3.elementContainerStart,
+  R3.elementEnd,
+  R3.elementStart,
+  R3.hostProperty,
+  R3.i18nExp,
+  R3.listener,
+  R3.listener,
+  R3.property,
+  R3.styleProp,
   R3.stylePropInterpolate1,
   R3.stylePropInterpolate2,
   R3.stylePropInterpolate3,
@@ -29,13 +35,8 @@ const CHAINABLE = new Set([
   R3.stylePropInterpolate7,
   R3.stylePropInterpolate8,
   R3.stylePropInterpolateV,
-  R3.classProp,
-  R3.listener,
-  R3.elementContainerStart,
-  R3.elementContainerEnd,
-  R3.elementContainer,
-  R3.listener,
   R3.syntheticHostListener,
+  R3.syntheticHostProperty,
   R3.templateCreate,
 ]);
 

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

@@ -46,12 +46,14 @@ export function phaseI18nTextExtraction(job: CompilationJob): void {
 
           const i18nBlockId = textNodes.get(op.target)!;
           const ops: ir.UpdateOp[] = [];
-          for (const expr of op.interpolation.expressions) {
-            ops.push(
-                ir.createI18nExpressionOp(i18nBlockId, expr, expr.sourceSpan ?? op.sourceSpan));
+          for (let i = 0; i < op.interpolation.expressions.length; i++) {
+            const expr = op.interpolation.expressions[i];
+            const placeholder = op.i18nPlaceholders[i];
+            ops.push(ir.createI18nExpressionOp(
+                i18nBlockId, expr, placeholder, expr.sourceSpan ?? op.sourceSpan));
           }
           if (ops.length > 0) {
-            ops.push(ir.createI18nApplyOp(i18nBlockId, op.i18nPlaceholders, op.sourceSpan));
+            // ops.push(ir.createI18nApplyOp(i18nBlockId, op.i18nPlaceholders, op.sourceSpan));
           }
           ir.OpList.replaceWithMany(op as ir.UpdateOp, ops);
           break;

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

@@ -15,89 +15,69 @@ import {CompilationJob} from '../compilation';
  */
 const ESCAPE = '\uFFFD';
 
-/**
- * Represents a placeholder value mapping on an I18nStartOp.
- */
-interface PlaceholderValue {
-  op: ir.I18nStartOp;
-
-  placeholder: string;
-
-  value: o.Expression;
-}
-
 /**
  * Resolve the placeholders in i18n messages.
  */
 export function phaseResolveI18nPlaceholders(job: CompilationJob) {
   for (const unit of job.units) {
     const i18nOps = new Map<ir.XrefId, ir.I18nOp|ir.I18nStartOp>();
-    let currentOp: ir.I18nStartOp|null = null;
-    let startTags: PlaceholderValue[] = [];
-    let closeTags: PlaceholderValue[] = [];
+    let startTagSlots = new Map<string, number[]>();
+    let closeTagSlots = new Map<string, number[]>();
+    let currentI18nOp: ir.I18nStartOp|null = null;
 
-    // Fill in values for tag name placeholders.
+    // Record slots for tag name placeholders.
     for (const op of unit.create) {
       switch (op.kind) {
         case ir.OpKind.I18nStart:
-          i18nOps.set(op.xref, op);
-          currentOp = op;
-          break;
         case ir.OpKind.I18n:
+          // Initialize collected slots for a new i18n block.
           i18nOps.set(op.xref, op);
+          currentI18nOp = op.kind === ir.OpKind.I18nStart ? op : null;
+          startTagSlots = new Map();
+          closeTagSlots = new Map();
           break;
         case ir.OpKind.I18nEnd:
-          currentOp = null;
+          // Add values for tag placeholders.
+          if (currentI18nOp === null) {
+            throw Error('Missing corresponding i18n start op for i18n end op');
+          }
+          for (const [placeholder, slots] of startTagSlots) {
+            currentI18nOp.params.set(placeholder, serializeSlots(slots, true));
+          }
+          for (const [placeholder, slots] of closeTagSlots) {
+            currentI18nOp.params.set(placeholder, serializeSlots(slots, false));
+          }
+          currentI18nOp = null;
           break;
         case ir.OpKind.Element:
         case ir.OpKind.ElementStart:
+          // Record slots for tag placeholders.
           if (op.i18nPlaceholder != undefined) {
-            if (currentOp === null) {
+            if (currentI18nOp === null) {
               throw Error('i18n tag placeholder should only occur inside an i18n block');
             }
-            // In order to add the keys in the same order as TemplateDefinitionBuilder, we
-            // separately track the start and close tag placeholders.
-            // TODO: when TemplateDefinitionBuilder compatibility is not required, we can just add
-            // both keys directly to the map here.
-            startTags.push({
-              op: currentOp,
-              placeholder: op.i18nPlaceholder.startName,
-              value: o.literal(`${ESCAPE}#${op.slot}${ESCAPE}`)
-            });
-            closeTags.push({
-              op: currentOp,
-              placeholder: op.i18nPlaceholder.closeName,
-              value: o.literal(`${ESCAPE}/#${op.slot}${ESCAPE}`)
-            });
+            if (!op.slot) {
+              throw Error('Slots should be allocated before i18n placeholder resolution');
+            }
+            const {startName, closeName} = op.i18nPlaceholder;
+            addTagSlot(startTagSlots, startName, op.slot);
+            addTagSlot(closeTagSlots, closeName, op.slot);
           }
           break;
       }
     }
 
-    // Add the start tags in the order we encountered them, to match TemplateDefinitionBuilder.
-    for (const {op, placeholder, value} of startTags) {
-      op.params.set(placeholder, value);
-    }
-    // Add the close tags in reverse order that we encountered the start tags, to match
-    // TemplateDefinitionBuilder.
-    for (let i = closeTags.length - 1; i >= 0; i--) {
-      const {op, placeholder, value} = closeTags[i];
-      op.params.set(placeholder, value);
-    }
-
     // Fill in values for each of the expression placeholders applied in i18nApply operations.
     const i18nBlockPlaceholderIndices = new Map<ir.XrefId, number>();
     for (const op of unit.update) {
-      if (op.kind === ir.OpKind.I18nApply) {
-        for (const placeholder of op.i18nPlaceholders) {
-          const i18nOp = i18nOps.get(op.target);
-          let index = i18nBlockPlaceholderIndices.get(op.target) || 0;
-          if (!i18nOp) {
-            throw Error('Cannot find corresponding i18nStart for i18nApply');
-          }
-          i18nOp.params.set(placeholder.name, o.literal(`${ESCAPE}${index++}${ESCAPE}`));
-          i18nBlockPlaceholderIndices.set(op.target, index);
+      if (op.kind === ir.OpKind.I18nExpression) {
+        const i18nOp = i18nOps.get(op.target);
+        let index = i18nBlockPlaceholderIndices.get(op.target) || 0;
+        if (!i18nOp) {
+          throw Error('Cannot find corresponding i18nStart for i18nExpr');
         }
+        i18nOp.params.set(op.i18nPlaceholder.name, o.literal(`${ESCAPE}${index++}${ESCAPE}`));
+        i18nBlockPlaceholderIndices.set(op.target, index);
       }
     }
 
@@ -111,3 +91,23 @@ export function phaseResolveI18nPlaceholders(job: CompilationJob) {
     }
   }
 }
+
+/**
+ * Updates the given slots map with the specified slot.
+ */
+function addTagSlot(tagSlots: Map<string, number[]>, placeholder: string, slot: number) {
+  const slots = tagSlots.get(placeholder) || [];
+  slots.push(slot);
+  tagSlots.set(placeholder, slots);
+}
+
+/**
+ * Serializes a list of slots to a string literal expression.
+ */
+function serializeSlots(slots: number[], start: boolean): o.Expression {
+  const slotStrings = slots.map(slot => `${ESCAPE}${start ? '' : '/'}#${slot}${ESCAPE}`);
+  if (slotStrings.length === 1) {
+    return o.literal(slotStrings[0]);
+  }
+  return o.literal(`[${slotStrings.join('|')}]`);
+}