From 12ffb25e94889b6af070e3932b5fb7608890dedf Mon Sep 17 00:00:00 2001
From: Miles Malerba <mmalerba@google.com>
Date: Tue, 7 Nov 2023 21:46:56 -0800
Subject: [PATCH] refactor(compiler): Fix some issues with i18n expressions in
 ICUs (#52698)

We were previously counting the i18n expression index and deciding when
to apply i18n expressions based on the i18n context. These should be
done based on the i18n block instead.

PR Close #52698
---
 .../icu_logic/TEST_CASES.json                 |  3 +--
 .../src/phases/apply_i18n_expressions.ts      | 19 +++++++++++++++----
 .../resolve_i18n_expression_placeholders.ts   |  6 +++---
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json
index 83fde4ec32e..c4e48e6e789 100644
--- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json
+++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json
@@ -118,8 +118,7 @@
             "verifyUniqueConsts"
           ]
         }
-      ],
-      "skipForTemplatePipeline": true
+      ]
     },
     {
       "description": "should handle multiple icus that share same placeholder",
diff --git a/packages/compiler/src/template/pipeline/src/phases/apply_i18n_expressions.ts b/packages/compiler/src/template/pipeline/src/phases/apply_i18n_expressions.ts
index 23e4700080c..6528585129a 100644
--- a/packages/compiler/src/template/pipeline/src/phases/apply_i18n_expressions.ts
+++ b/packages/compiler/src/template/pipeline/src/phases/apply_i18n_expressions.ts
@@ -13,10 +13,19 @@ import {CompilationJob} from '../compilation';
  * Adds apply operations after i18n expressions.
  */
 export function applyI18nExpressions(job: CompilationJob): void {
+  const i18nContexts = new Map<ir.XrefId, ir.I18nContextOp>();
+  for (const unit of job.units) {
+    for (const op of unit.create) {
+      if (op.kind === ir.OpKind.I18nContext) {
+        i18nContexts.set(op.xref, op);
+      }
+    }
+  }
+
   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)) {
+      if (op.kind === ir.OpKind.I18nExpression && needsApplication(i18nContexts, op)) {
         // TODO: what should be the source span for the apply op?
         ir.OpList.insertAfter<ir.UpdateOp>(ir.createI18nApplyOp(op.target, op.handle, null!), op);
       }
@@ -27,13 +36,15 @@ export function applyI18nExpressions(job: CompilationJob): void {
 /**
  * Checks whether the given expression op needs to be followed with an apply op.
  */
-function needsApplication(op: ir.I18nExpressionOp) {
+function needsApplication(i18nContexts: Map<ir.XrefId, ir.I18nContextOp>, 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 context, we need to apply.
-  if (op.next.context !== op.context) {
+  // If the next op is an expression targeting a different i18n block, we need to apply.
+  const context = i18nContexts.get(op.context)!;
+  const nextContext = i18nContexts.get(op.next.context)!;
+  if (context.i18nBlock !== nextContext.i18nBlock) {
     return true;
   }
   return false;
diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts
index f92673a283c..b6bc1151275 100644
--- a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts
+++ b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts
@@ -29,14 +29,14 @@ export function resolveI18nExpressionPlaceholders(job: ComponentCompilationJob)
     }
   }
 
-  // Keep track of the next available expression index per i18n context.
+  // Keep track of the next available expression index per i18n block.
   const expressionIndices = new Map<ir.XrefId, number>();
 
   for (const unit of job.units) {
     for (const op of unit.update) {
       if (op.kind === ir.OpKind.I18nExpression) {
-        const index = expressionIndices.get(op.context) || 0;
         const i18nContext = i18nContexts.get(op.context)!;
+        const index = expressionIndices.get(i18nContext.i18nBlock) || 0;
         const subTemplateIndex = subTemplateIndicies.get(i18nContext.i18nBlock)!;
         // Add the expression index in the appropriate params map.
         const params = op.resolutionTime === ir.I18nParamResolutionTime.Creation ?
@@ -50,7 +50,7 @@ export function resolveI18nExpressionPlaceholders(job: ComponentCompilationJob)
         });
         params.set(op.i18nPlaceholder, values);
 
-        expressionIndices.set(op.context, index + 1);
+        expressionIndices.set(i18nContext.i18nBlock, index + 1);
       }
     }
   }