fix(compiler): ensure that legacy ids are rendered for ICUs (#33318)

When computing i18n messages for templates there are two passes.
This is because messages must be computed before any whitespace
is removed. Then on a second pass, the messages must be recreated
but reusing the message ids from the first pass.

Previously ICUs were losing their legacy ids that had been computed
via the first pass. This commit fixes that by keeping track of the
message from the first pass (`previousMessage`) for ICU placeholder
nodes.

// FW-1637

PR Close #33318

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -2179,6 +2179,23 @@ runInEachFileSystem(os => {
       expect(jsContents).not.toContain(':Some text');
     });
 
+    it('should also render legacy id for ICUs when normal messages are using legacy ids', () => {
+      env.tsconfig({i18nInFormat: 'xliff'});
+      env.write(`test.ts`, `
+     import {Component} from '@angular/core';
+     @Component({
+       selector: 'test',
+       template: '<div i18n="@@custom">Some text {age, plural, 10 {ten} other {other}}</div>'
+     })
+     class FooCmp {}`);
+      env.driveMain();
+      const jsContents = env.getContents('test.js');
+      expect(jsContents)
+          .toContain(
+              ':@@720ba589d043a0497ac721ff972f41db0c919efb:{VAR_PLURAL, plural, 10 {ten} other {other}}');
+      expect(jsContents).toContain(':@@custom:Some text');
+    });
+
     it('@Component\'s `interpolation` should override default interpolation config', () => {
       env.write(`test.ts`, `
       import {Component} from '@angular/core';

packages/compiler/src/i18n/i18n_ast.ts

@@ -93,6 +93,8 @@ export class Placeholder implements Node {
 }
 
 export class IcuPlaceholder implements Node {
+  /** Used to capture a message computed from a previous processing pass (see `setI18nRefs()`). */
+  previousMessage?: Message;
   constructor(public value: Icu, public name: string, public sourceSpan: ParseSourceSpan) {}
 
   visit(visitor: Visitor, context?: any): any { return visitor.visitIcuPlaceholder(this, context); }

packages/compiler/src/i18n/i18n_parser.ts

@@ -21,8 +21,8 @@ const _expParser = new ExpressionParser(new ExpressionLexer());
 export type VisitNodeFn = (html: html.Node, i18n: i18n.Node) => i18n.Node;
 
 export interface I18nMessageFactory {
-  (nodes: html.Node[], meaning: string, description: string, customId: string,
-   visitNodeFn?: VisitNodeFn): i18n.Message;
+  (nodes: html.Node[], meaning: string|undefined, description: string|undefined,
+   customId: string|undefined, visitNodeFn?: VisitNodeFn): i18n.Message;
 }
 
 /**
@@ -54,7 +54,7 @@ class _I18nVisitor implements html.Visitor {
       private _interpolationConfig: InterpolationConfig) {}
 
   public toI18nMessage(
-      nodes: html.Node[], meaning: string, description: string, customId: string,
+      nodes: html.Node[], meaning = '', description = '', customId = '',
       visitNodeFn: VisitNodeFn|undefined): i18n.Message {
     const context: I18nMessageVisitorContext = {
       isIcu: nodes.length == 1 && nodes[0] instanceof html.Expansion,

packages/compiler/src/render3/view/i18n/meta.ts

@@ -23,10 +23,20 @@ export type I18nMeta = {
   meaning?: string
 };
 
-function setI18nRefs(html: html.Node & {i18n?: i18n.I18nMeta}, i18n: i18n.Node): i18n.Node {
-  html.i18n = i18n;
-  return i18n;
-}
+
+const setI18nRefs: VisitNodeFn = (htmlNode, i18nNode) => {
+  if (htmlNode instanceof html.NodeWithI18n) {
+    if (i18nNode instanceof i18n.IcuPlaceholder && htmlNode.i18n instanceof i18n.Message) {
+      // This html node represents an ICU but this is a second processing pass, and the legacy id
+      // was computed in the previous pass and stored in the `i18n` property as a message.
+      // We are about to wipe out that property so capture the previous message to be reused when
+      // generating the message for this ICU later. See `_generateI18nMessage()`.
+      i18nNode.previousMessage = htmlNode.i18n;
+    }
+    htmlNode.i18n = i18nNode;
+  }
+  return i18nNode;
+};
 
 /**
  * This visitor walks over HTML parse tree and converts information stored in
@@ -44,28 +54,10 @@ export class I18nMetaVisitor implements html.Visitor {
   private _generateI18nMessage(
       nodes: html.Node[], meta: string|i18n.I18nMeta = '',
       visitNodeFn?: VisitNodeFn): i18n.Message {
-    const parsed: I18nMeta =
-        typeof meta === 'string' ? parseI18nMeta(meta) : metaFromI18nMessage(meta as i18n.Message);
-    const message = this._createI18nMessage(
-        nodes, parsed.meaning || '', parsed.description || '', parsed.customId || '', visitNodeFn);
-    if (!message.id) {
-      // generate (or restore) message id if not specified in template
-      message.id = typeof meta !== 'string' && (meta as i18n.Message).id || decimalDigest(message);
-    }
-
-    if (this.i18nLegacyMessageIdFormat === 'xlf' || this.i18nLegacyMessageIdFormat === 'xliff') {
-      message.legacyId = computeDigest(message);
-    } else if (
-        this.i18nLegacyMessageIdFormat === 'xlf2' || this.i18nLegacyMessageIdFormat === 'xliff2' ||
-        this.i18nLegacyMessageIdFormat === 'xmb') {
-      message.legacyId = computeDecimalDigest(message);
-    } else if (typeof meta !== 'string') {
-      // This occurs if we are doing the 2nd pass after whitespace removal
-      // In that case we want to reuse the legacy message generated in the 1st pass
-      // See `parseTemplate()` in `packages/compiler/src/render3/view/template.ts`
-      message.legacyId = (meta as i18n.Message).legacyId;
-    }
-
+    const {meaning, description, customId} = this._parseMetadata(meta);
+    const message = this._createI18nMessage(nodes, meaning, description, customId, visitNodeFn);
+    this._setMessageId(message, meta);
+    this._setLegacyId(message, meta);
     return message;
   }
 
@@ -141,6 +133,57 @@ export class I18nMetaVisitor implements html.Visitor {
   visitAttribute(attribute: html.Attribute): any { return attribute; }
   visitComment(comment: html.Comment): any { return comment; }
   visitExpansionCase(expansionCase: html.ExpansionCase): any { return expansionCase; }
+
+  /**
+   * Parse the general form `meta` passed into extract the explicit metadata needed to create a
+   * `Message`.
+   *
+   * There are three possibilities for the `meta` variable
+   * 1) a string from an `i18n` template attribute: parse it to extract the metadata values.
+   * 2) a `Message` from a previous processing pass: reuse the metadata values in the message.
+   * 4) other: ignore this and just process the message metadata as normal
+   *
+   * @param meta the bucket that holds information about the message
+   * @returns the parsed metadata.
+   */
+  private _parseMetadata(meta: string|i18n.I18nMeta): I18nMeta {
+    return typeof meta === 'string' ? parseI18nMeta(meta) :
+                                      meta instanceof i18n.Message ? metaFromI18nMessage(meta) : {};
+  }
+
+  /**
+   * Generate (or restore) message id if not specified already.
+   */
+  private _setMessageId(message: i18n.Message, meta: string|i18n.I18nMeta): void {
+    if (!message.id) {
+      message.id = meta instanceof i18n.Message && meta.id || decimalDigest(message);
+    }
+  }
+
+  /**
+   * Update the `message` with a `legacyId` if necessary.
+   *
+   * @param message the message whose legacy id should be set
+   * @param meta information about the message being processed
+   */
+  private _setLegacyId(message: i18n.Message, meta: string|i18n.I18nMeta): void {
+    if (this.i18nLegacyMessageIdFormat === 'xlf' || this.i18nLegacyMessageIdFormat === 'xliff') {
+      message.legacyId = computeDigest(message);
+    } else if (
+        this.i18nLegacyMessageIdFormat === 'xlf2' || this.i18nLegacyMessageIdFormat === 'xliff2' ||
+        this.i18nLegacyMessageIdFormat === 'xmb') {
+      message.legacyId = computeDecimalDigest(message);
+    } else if (typeof meta !== 'string') {
+      // This occurs if we are doing the 2nd pass after whitespace removal (see `parseTemplate()` in
+      // `packages/compiler/src/render3/view/template.ts`).
+      // In that case we want to reuse the legacy message generated in the 1st pass (see
+      // `setI18nRefs()`).
+      const previousMessage = meta instanceof i18n.Message ?
+          meta :
+          meta instanceof i18n.IcuPlaceholder ? meta.previousMessage : undefined;
+      message.legacyId = previousMessage && previousMessage.legacyId;
+    }
+  }
 }
 
 export function metaFromI18nMessage(message: i18n.Message, id: string | null = null): I18nMeta {

packages/compiler/test/render3/view/i18n_spec.ts

@@ -21,7 +21,7 @@ import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util';
 import {parseR3 as parse} from './util';
 
 const expressionParser = new Parser(new Lexer());
-const i18nOf = (element: t.Node & {i18n?: i18n.AST}) => element.i18n !;
+const i18nOf = (element: t.Node & {i18n?: i18n.I18nMeta}) => element.i18n !;
 
 describe('I18nContext', () => {
   it('should support i18n content collection', () => {