fix(ivy): proper i18n postprocessing in case of nested templates (#28209)

Prior to this change the postprocess step relied on the order of placeholders combined in one group (e.g. [�#1�|�*1:1�]). The order is not guaranteed in case we have nested templates (since we use BFS to process templates) and some tags are represented using same placeholders. This change performs postprocessing more accurate by keeping track of currently active template and searching for matching placeholder.

PR Close #28209

packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts

@@ -1682,6 +1682,31 @@ describe('i18n support in the view compiler', () => {
       verify(input, output);
     });
 
+    it('should support ICU-only templates', () => {
+      const input = `
+        {age, select, 10 {ten} 20 {twenty} other {other}}
+      `;
+
+      const output = String.raw `
+        const $MSG_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$ = goog.getMsg("{VAR_SELECT, select, 10 {ten} 20 {twenty} other {other}}");
+        const $I18N_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$ = $r3$.ɵi18nPostprocess($MSG_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$, { "VAR_SELECT": "\uFFFD0\uFFFD" });
+        …
+        consts: 1,
+        vars: 1,
+        template: function MyComponent_Template(rf, ctx) {
+          if (rf & 1) {
+            $r3$.ɵi18n(0, $I18N_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$);
+          }
+          if (rf & 2) {
+            $r3$.ɵi18nExp($r3$.ɵbind(ctx.age));
+            $r3$.ɵi18nApply(0);
+          }
+        }
+      `;
+
+      verify(input, output);
+    });
+
     it('should generate i18n instructions for icus generated outside of i18n blocks', () => {
       const input = `
         <div>{gender, select, male {male} female {female} other {other}}</div>

packages/core/src/render3/i18n.ts

@@ -25,16 +25,24 @@ import {NO_CHANGE} from './tokens';
 import {addAllToArray, getNativeByIndex, getNativeByTNode, getTNode, isLContainer, renderStringify} from './util';
 
 const MARKER = `<EFBFBD>`;
-const ICU_BLOCK_REGEX = /^\s*(<28>\d+:?\d*<2A>)\s*,\s*(select|plural)\s*,/;
+const ICU_BLOCK_REGEXP = /^\s*(<28>\d+:?\d*<2A>)\s*,\s*(select|plural)\s*,/;
 const SUBTEMPLATE_REGEXP = /<2F>\/?\*(\d+:\d+)<29>/gi;
 const PH_REGEXP = /<2F>(\/?[#*]\d+):?\d*<2A>/gi;
 const BINDING_REGEXP = /<2F>(\d+):?\d*<2A>/gi;
 const ICU_REGEXP = /({\s*<2A>\d+:?\d*<2A>\s*,\s*\S{6}\s*,[\s\S]*})/gi;
 
-// i18nPostproocess regexps
+// i18nPostprocess consts
-const PP_PLACEHOLDERS = /\[(<28>.+?<3F>?)\]/g;
+const ROOT_TEMPLATE_ID = 0;
-const PP_ICU_VARS = /({\s*)(VAR_(PLURAL|SELECT)(_\d+)?)(\s*,)/g;
+const PP_MULTI_VALUE_PLACEHOLDERS_REGEXP = /\[(<28>.+?<3F>?)\]/;
-const PP_ICUS = /<2F>I18N_EXP_(ICU(_\d+)?)<29>/g;
+const PP_PLACEHOLDERS_REGEXP = /\[(<28>.+?<3F>?)\]|(<28>\/?\*\d+:\d+<2B>)/g;
+const PP_ICU_VARS_REGEXP = /({\s*)(VAR_(PLURAL|SELECT)(_\d+)?)(\s*,)/g;
+const PP_ICUS_REGEXP = /<2F>I18N_EXP_(ICU(_\d+)?)<29>/g;
+const PP_CLOSE_TEMPLATE_REGEXP = /\/\*/;
+const PP_TEMPLATE_ID_REGEXP = /\d+\:(\d+)/;
+
+// Parsed placeholder structure used in postprocessing (within `i18nPostprocess` function)
+// Contains the following fields: [templateId, isCloseTemplateTag, placeholder]
+type PostprocessPlaceholder = [number, boolean, string];
 
 interface IcuExpression {
   type: IcuType;
@@ -104,7 +112,7 @@ function extractParts(pattern: string): (string | IcuExpression)[] {
       if (braceStack.length == 0) {
         // End of the block.
         const block = pattern.substring(prevPos, pos);
-        if (ICU_BLOCK_REGEX.test(block)) {
+        if (ICU_BLOCK_REGEXP.test(block)) {
           results.push(parseICUBlock(block));
         } else if (block) {  // Don't push empty strings
           results.push(block);
@@ -142,7 +150,7 @@ function parseICUBlock(pattern: string): IcuExpression {
   const values: (string | IcuExpression)[][] = [];
   let icuType = IcuType.plural;
   let mainBinding = 0;
-  pattern = pattern.replace(ICU_BLOCK_REGEX, function(str: string, binding: string, type: string) {
+  pattern = pattern.replace(ICU_BLOCK_REGEXP, function(str: string, binding: string, type: string) {
     if (type === 'select') {
       icuType = IcuType.select;
     } else {
@@ -505,24 +513,62 @@ function appendI18nNode(tNode: TNode, parentTNode: TNode, previousTNode: TNode |
  */
 export function i18nPostprocess(
     message: string, replacements: {[key: string]: (string | string[])} = {}): string {
-  //
+  /**
-  // Step 1: resolve all multi-value cases (like [<5B>*1:1<><31>#2:1<>|<7C>#4:1<>|<7C>5<EFBFBD>])
+   * Step 1: resolve all multi-value placeholders like [<EFBFBD>#5<EFBFBD>|<EFBFBD>*1:1<EFBFBD><EFBFBD>#2:1<EFBFBD>|<EFBFBD>#4:1<EFBFBD>]
-  //
+   *
-  const matches: {[key: string]: string[]} = {};
+   * Note: due to the way we process nested templates (BFS), multi-value placeholders are typically
-  let result = message.replace(PP_PLACEHOLDERS, (_match, content: string): string => {
+   * grouped by templates, for example: [<EFBFBD>#5<EFBFBD>|<EFBFBD>#6<EFBFBD>|<EFBFBD>#1:1<EFBFBD>|<EFBFBD>#3:2<EFBFBD>] where <EFBFBD>#5<EFBFBD> and <EFBFBD>#6<EFBFBD> belong to root
-    if (!matches[content]) {
+   * template, <EFBFBD>#1:1<EFBFBD> belong to nested template with index 1 and <EFBFBD>#1:2<EFBFBD> - nested template with index
-      matches[content] = content.split('|');
+   * 3. However in real templates the order might be different: i.e. <EFBFBD>#1:1<EFBFBD> and/or <EFBFBD>#3:2<EFBFBD> may go in
-    }
+   * front of <EFBFBD>#6<EFBFBD>. The post processing step restores the right order by keeping track of the
-    if (!matches[content].length) {
+   * template id stack and looks for placeholders that belong to the currently active template.
-      throw new Error(`i18n postprocess: unmatched placeholder - ${content}`);
+   */
-    }
+  let result: string = message;
-    return matches[content].shift() !;
+  if (PP_MULTI_VALUE_PLACEHOLDERS_REGEXP.test(message)) {
-  });
+    const matches: {[key: string]: PostprocessPlaceholder[]} = {};
+    const templateIdsStack: number[] = [ROOT_TEMPLATE_ID];
+    result = result.replace(PP_PLACEHOLDERS_REGEXP, (m: any, phs: string, tmpl: string): string => {
+      const content = phs || tmpl;
+      if (!matches[content]) {
+        const placeholders: PostprocessPlaceholder[] = [];
+        content.split('|').forEach((placeholder: string) => {
+          const match = placeholder.match(PP_TEMPLATE_ID_REGEXP);
+          const templateId = match ? parseInt(match[1], 10) : ROOT_TEMPLATE_ID;
+          const isCloseTemplateTag = PP_CLOSE_TEMPLATE_REGEXP.test(placeholder);
+          placeholders.push([templateId, isCloseTemplateTag, placeholder]);
+        });
+        matches[content] = placeholders;
+      }
+      if (!matches[content].length) {
+        throw new Error(`i18n postprocess: unmatched placeholder - ${content}`);
+      }
+      const currentTemplateId = templateIdsStack[templateIdsStack.length - 1];
+      const placeholders = matches[content];
+      let idx = 0;
+      // find placeholder index that matches current template id
+      for (let i = 0; i < placeholders.length; i++) {
+        if (placeholders[i][0] === currentTemplateId) {
+          idx = i;
+          break;
+        }
+      }
+      // update template id stack based on the current tag extracted
+      const [templateId, isCloseTemplateTag, placeholder] = placeholders[idx];
+      if (isCloseTemplateTag) {
+        templateIdsStack.pop();
+      } else if (currentTemplateId !== templateId) {
+        templateIdsStack.push(templateId);
+      }
+      // remove processed tag from the list
+      placeholders.splice(idx, 1);
+      return placeholder;
+    });
 
-  // verify that we injected all values
+    // verify that we injected all values
-  const hasUnmatchedValues = Object.keys(matches).some(key => !!matches[key].length);
+    const hasUnmatchedValues = Object.keys(matches).some(key => !!matches[key].length);
-  if (hasUnmatchedValues) {
+    if (hasUnmatchedValues) {
-    throw new Error(`i18n postprocess: unmatched values - ${JSON.stringify(matches)}`);
+      throw new Error(`i18n postprocess: unmatched values - ${JSON.stringify(matches)}`);
+    }
   }
 
   // return current result if no replacements specified
@@ -530,18 +576,18 @@ export function i18nPostprocess(
     return result;
   }
 
-  //
+  /**
-  // Step 2: replace all ICU vars (like "VAR_PLURAL")
+   * Step 2: replace all ICU vars (like "VAR_PLURAL")
-  //
+   */
-  result = result.replace(PP_ICU_VARS, (match, start, key, _type, _idx, end): string => {
+  result = result.replace(PP_ICU_VARS_REGEXP, (match, start, key, _type, _idx, end): string => {
     return replacements.hasOwnProperty(key) ? `${start}${replacements[key]}${end}` : match;
   });
 
-  //
+  /**
-  // Step 3: replace all ICU references with corresponding values (like <20>ICU_EXP_ICU_1<5F>)
+   * Step 3: replace all ICU references with corresponding values (like <EFBFBD>ICU_EXP_ICU_1<EFBFBD>) in case
-  // in case multiple ICUs have the same placeholder name
+   * multiple ICUs have the same placeholder name
-  //
+   */
-  result = result.replace(PP_ICUS, (match, key): string => {
+  result = result.replace(PP_ICUS_REGEXP, (match, key): string => {
     if (replacements.hasOwnProperty(key)) {
       const list = replacements[key] as string[];
       if (!list.length) {

packages/core/test/i18n_integration_spec.ts

@@ -46,8 +46,12 @@ const TRANSLATIONS: any = {
       '{$startTagSpan}Mon logo{$tagImg}{$closeTagSpan}',
   '{$startTagNgTemplate} Hello {$closeTagNgTemplate}{$startTagNgContainer} Bye {$closeTagNgContainer}':
       '{$startTagNgTemplate} Bonjour {$closeTagNgTemplate}{$startTagNgContainer} Au revoir {$closeTagNgContainer}',
+  '{$startTagNgTemplate}{$startTagSpan}Hello{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan}Hello{$closeTagSpan}{$closeTagNgContainer}':
+      '{$startTagNgTemplate}{$startTagSpan}Bonjour{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan}Bonjour{$closeTagSpan}{$closeTagNgContainer}',
   '{$startTagNgTemplate}{$startTagSpan}Hello{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan_1}Hello{$closeTagSpan}{$closeTagNgContainer}':
       '{$startTagNgTemplate}{$startTagSpan}Bonjour{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan_1}Bonjour{$closeTagSpan}{$closeTagNgContainer}',
+  '{$startTagSpan} Hello - 1 {$closeTagSpan}{$startTagSpan_1} Hello - 2 {$startTagSpan_1} Hello - 3 {$startTagSpan_1} Hello - 4 {$closeTagSpan}{$closeTagSpan}{$closeTagSpan}{$startTagSpan} Hello - 5 {$closeTagSpan}':
+      '{$startTagSpan} Bonjour - 1 {$closeTagSpan}{$startTagSpan_1} Bonjour - 2 {$startTagSpan_1} Bonjour - 3 {$startTagSpan_1} Bonjour - 4 {$closeTagSpan}{$closeTagSpan}{$closeTagSpan}{$startTagSpan} Bonjour - 5 {$closeTagSpan}',
   '{VAR_SELECT, select, 10 {ten} 20 {twenty} other {other}}':
       '{VAR_SELECT, select, 10 {dix} 20 {vingt} other {autres}}',
   '{VAR_SELECT, select, 1 {one} 2 {two} other {more than two}}':
@@ -283,29 +287,56 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() {
       expect(element.textContent.replace(/\s+/g, ' ').trim()).toBe('Bonjour Au revoir');
     });
 
-    fixmeIvy(
+    it('should be able to act as child elements inside i18n block (text + tags)', () => {
-        'FW-910: Invalid placeholder structure generated when using <ng-template> with content that contains tags')
+      const content = 'Hello';
-        .it('should be able to act as child elements inside i18n block (text + tags)', () => {
+      const template = `
-          const content = 'Hello';
+        <div i18n>
-          const template = `
+          <ng-template tplRef>
-            <div i18n>
+            <span>${content}</span>
-              <ng-template tplRef>
+          </ng-template>
-                <span>${content}</span>
+          <ng-container>
-              </ng-template>
+            <span>${content}</span>
-              <ng-container>
+          </ng-container>
-                <span>${content}</span>
+        </div>
-              </ng-container>
+      `;
-            </div>
+      const fixture = getFixtureWithOverrides({template});
-          `;
-          const fixture = getFixtureWithOverrides({template});
 
-          const element = fixture.nativeElement;
+      const element = fixture.nativeElement;
-          const spans = element.getElementsByTagName('span');
+      const spans = element.getElementsByTagName('span');
-          for (let i = 0; i < spans.length; i++) {
+      for (let i = 0; i < spans.length; i++) {
-            const child = spans[i];
+        expect(spans[i]).toHaveText('Bonjour');
-            expect((child as any).innerHTML).toBe('Bonjour');
+      }
-          }
+    });
-        });
+
+    it('should be able to handle deep nested levels with templates', () => {
+      const content = 'Hello';
+      const template = `
+        <div i18n>
+          <span>
+            ${content} - 1
+          </span>
+          <span *ngIf="visible">
+            ${content} - 2
+            <span *ngIf="visible">
+              ${content} - 3
+              <span *ngIf="visible">
+                ${content} - 4
+              </span>
+            </span>
+          </span>
+          <span>
+            ${content} - 5
+          </span>
+        </div>
+      `;
+      const fixture = getFixtureWithOverrides({template});
+
+      const element = fixture.nativeElement;
+      const spans = element.getElementsByTagName('span');
+      for (let i = 0; i < spans.length; i++) {
+        expect(spans[i].innerHTML).toContain(`Bonjour - ${i + 1}`);
+      }
+    });
 
     it('should handle self-closing tags as content', () => {
       const label = 'My logo';
@@ -340,6 +371,16 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() {
       expect(element).toHaveText('vingt');
     });
 
+    it('should support ICU-only templates', () => {
+      const template = `
+        {age, select, 10 {ten} 20 {twenty} other {other}}
+      `;
+      const fixture = getFixtureWithOverrides({template});
+
+      const element = fixture.nativeElement;
+      expect(element).toHaveText('vingt');
+    });
+
     it('should support ICUs generated outside of i18n blocks', () => {
       const template = `
         <div>{age, select, 10 {ten} 20 {twenty} other {other}}</div>

packages/core/test/render3/i18n_spec.ts

@@ -1777,7 +1777,7 @@ describe('Runtime i18n', () => {
 
   describe('i18nPostprocess', () => {
     it('should handle valid cases', () => {
-      const arr = ['<27>*1:1<><31>#2:1<>', '<27>#4:2<EFBFBD>', '<27>6:4<EFBFBD>', '<27>/#2:1<><31>/*1:1<>'];
+      const arr = ['<27>*1:1<><31>#2:1<>', '<27>#4:1<EFBFBD>', '<27>6:1<EFBFBD>', '<27>/#2:1<><31>/*1:1<>'];
       const str = `[${arr.join('|')}]`;
 
       const cases = [
@@ -1855,6 +1855,57 @@ describe('Runtime i18n', () => {
       });
     });
 
+    it('should handle nested template represented by multi-value placeholders', () => {
+      /**
+       * <div i18n>
+       *   <span>
+       *     Hello - 1
+       *   </span>
+       *   <span *ngIf="visible">
+       *     Hello - 2
+       *     <span *ngIf="visible">
+       *       Hello - 3
+       *       <span *ngIf="visible">
+       *         Hello - 4
+       *       </span>
+       *     </span>
+       *   </span>
+       *   <span>
+       *     Hello - 5
+       *   </span>
+       * </div>
+       */
+      const generated = `
+        [<EFBFBD>#2<EFBFBD>|<EFBFBD>#4<EFBFBD>] Bonjour - 1 [<EFBFBD>/#2<EFBFBD>|<EFBFBD>/#1:3<EFBFBD><EFBFBD>/*2:3<EFBFBD>|<EFBFBD>/#1:2<EFBFBD><EFBFBD>/*2:2<EFBFBD>|<EFBFBD>/#1:1<EFBFBD><EFBFBD>/*3:1<EFBFBD>|<EFBFBD>/#4<EFBFBD>]
+        [<EFBFBD>*3:1<EFBFBD><EFBFBD>#1:1<EFBFBD>|<EFBFBD>*2:2<EFBFBD><EFBFBD>#1:2<EFBFBD>|<EFBFBD>*2:3<EFBFBD><EFBFBD>#1:3<EFBFBD>]
+          Bonjour - 2
+          [<EFBFBD>*3:1<EFBFBD><EFBFBD>#1:1<EFBFBD>|<EFBFBD>*2:2<EFBFBD><EFBFBD>#1:2<EFBFBD>|<EFBFBD>*2:3<EFBFBD><EFBFBD>#1:3<EFBFBD>]
+            Bonjour - 3
+            [<EFBFBD>*3:1<EFBFBD><EFBFBD>#1:1<EFBFBD>|<EFBFBD>*2:2<EFBFBD><EFBFBD>#1:2<EFBFBD>|<EFBFBD>*2:3<EFBFBD><EFBFBD>#1:3<EFBFBD>] Bonjour - 4 [<EFBFBD>/#2<EFBFBD>|<EFBFBD>/#1:3<EFBFBD><EFBFBD>/*2:3<EFBFBD>|<EFBFBD>/#1:2<EFBFBD><EFBFBD>/*2:2<EFBFBD>|<EFBFBD>/#1:1<EFBFBD><EFBFBD>/*3:1<EFBFBD>|<EFBFBD>/#4<EFBFBD>]
+          [<EFBFBD>/#2<EFBFBD>|<EFBFBD>/#1:3<EFBFBD><EFBFBD>/*2:3<EFBFBD>|<EFBFBD>/#1:2<EFBFBD><EFBFBD>/*2:2<EFBFBD>|<EFBFBD>/#1:1<EFBFBD><EFBFBD>/*3:1<EFBFBD>|<EFBFBD>/#4<EFBFBD>]
+        [<EFBFBD>/#2<EFBFBD>|<EFBFBD>/#1:3<EFBFBD><EFBFBD>/*2:3<EFBFBD>|<EFBFBD>/#1:2<EFBFBD><EFBFBD>/*2:2<EFBFBD>|<EFBFBD>/#1:1<EFBFBD><EFBFBD>/*3:1<EFBFBD>|<EFBFBD>/#4<EFBFBD>]
+        [<EFBFBD>#2<EFBFBD>|<EFBFBD>#4<EFBFBD>] Bonjour - 5 [<EFBFBD>/#2<EFBFBD>|<EFBFBD>/#1:3<EFBFBD><EFBFBD>/*2:3<EFBFBD>|<EFBFBD>/#1:2<EFBFBD><EFBFBD>/*2:2<EFBFBD>|<EFBFBD>/#1:1<EFBFBD><EFBFBD>/*3:1<EFBFBD>|<EFBFBD>/#4<EFBFBD>]
+      `;
+      const final = `
+        <EFBFBD>#2<EFBFBD> Bonjour - 1 <EFBFBD>/#2<EFBFBD>
+        <EFBFBD>*3:1<EFBFBD>
+          <EFBFBD>#1:1<EFBFBD>
+            Bonjour - 2
+            <EFBFBD>*2:2<EFBFBD>
+              <EFBFBD>#1:2<EFBFBD>
+                Bonjour - 3
+                <EFBFBD>*2:3<EFBFBD>
+                  <EFBFBD>#1:3<EFBFBD> Bonjour - 4 <EFBFBD>/#1:3<EFBFBD>
+                <EFBFBD>/*2:3<EFBFBD>
+              <EFBFBD>/#1:2<EFBFBD>
+            <EFBFBD>/*2:2<EFBFBD>
+          <EFBFBD>/#1:1<EFBFBD>
+        <EFBFBD>/*3:1<EFBFBD>
+        <EFBFBD>#4<EFBFBD> Bonjour - 5 <EFBFBD>/#4<EFBFBD>
+      `;
+      expect(i18nPostprocess(generated.replace(/\s+/g, ''))).toEqual(final.replace(/\s+/g, ''));
+    });
+
     it('should throw in case we have invalid string', () => {
       const arr = ['<27>*1:1<><31>#2:1<>', '<27>#4:2<>', '<27>6:4<>', '<27>/#2:1<><31>/*1:1<>'];
       const str = `[${arr.join('|')}]`;