fix(ivy): support re-order embedded templates (#24805)

PR Close #24805

packages/core/src/render3/i18n.ts

@@ -76,10 +76,11 @@ export function i18nMapping(
     expressions?: (PlaceholderMap | null)[] | null, templateRoots?: string[] | null,
     lastChildIndex?: number | null): I18nInstruction[][] {
   const translationParts = translation.split(i18nTagRegex);
-  const instructions: I18nInstruction[][] = [];
+  const nbTemplates = templateRoots ? templateRoots.length + 1 : 1;
+  const instructions: I18nInstruction[][] = (new Array(nbTemplates)).fill(undefined);
 
   generateMappingInstructions(
-      0, translationParts, instructions, elements, expressions, templateRoots, lastChildIndex);
+      0, 0, translationParts, instructions, elements, expressions, templateRoots, lastChildIndex);
 
   return instructions;
 }
@@ -90,7 +91,9 @@ export function i18nMapping(
  *
  * See `i18nMapping()` for more details.
  *
- * @param index The current index in `translationParts`.
+ * @param tmplIndex The order of appearance of the template.
+ * 0 for the root template, following indexes match the order in `templateRoots`.
+ * @param partIndex The current index in `translationParts`.
  * @param translationParts The translation string split into an array of placeholders and text
  * elements.
  * @param instructions The current list of instructions to update.
@@ -102,13 +105,14 @@ export function i18nMapping(
  * generating the instructions for their parent template.
  * @param lastChildIndex The index of the last child of the i18n node. Used when the i18n block is
  * an ng-container.
+ *
  * @returns the current index in `translationParts`
  */
 function generateMappingInstructions(
-    index: number, translationParts: string[], instructions: I18nInstruction[][],
+    tmplIndex: number, partIndex: number, translationParts: string[],
-    elements: (PlaceholderMap | null)[] | null, expressions?: (PlaceholderMap | null)[] | null,
+    instructions: I18nInstruction[][], elements: (PlaceholderMap | null)[] | null,
-    templateRoots?: string[] | null, lastChildIndex?: number | null): number {
+    expressions?: (PlaceholderMap | null)[] | null, templateRoots?: string[] | null,
-  const tmplIndex = instructions.length;
+    lastChildIndex?: number | null): number {
   const tmplInstructions: I18nInstruction[] = [];
   const phVisited: string[] = [];
   let openedTagCount = 0;
@@ -118,20 +122,20 @@ function generateMappingInstructions(
   let currentExpressions: PlaceholderMap|null =
       expressions && expressions[tmplIndex] ? expressions[tmplIndex] : null;
 
-  instructions.push(tmplInstructions);
+  instructions[tmplIndex] = tmplInstructions;
 
-  for (; index < translationParts.length; index++) {
+  for (; partIndex < translationParts.length; partIndex++) {
-    const value = translationParts[index];
+    // The value can either be text or the name of a placeholder (element/template root/expression)
+    const value = translationParts[partIndex];
 
     // Odd indexes are placeholders
-    if (index & 1) {
+    if (partIndex & 1) {
       let phIndex;
       if (currentElements && currentElements[value] !== undefined) {
         phIndex = currentElements[value];
-        // The placeholder represents a DOM element
+        // The placeholder represents a DOM element, add an instruction to move it
-        // Add an instruction to move the element
+        let templateRootIndex = templateRoots ? templateRoots.indexOf(value) : -1;
-        const isTemplateRoot = templateRoots && templateRoots[tmplIndex] === value;
+        if (templateRootIndex !== -1 && (templateRootIndex + 1) !== tmplIndex) {
-        if (isTemplateRoot) {
           // This is a template root, it has no closing tag, not treating it as an element
           tmplInstructions.push(phIndex | I18nInstructions.TemplateRoot);
         } else {
@@ -141,8 +145,7 @@ function generateMappingInstructions(
         phVisited.push(value);
       } else if (currentExpressions && currentExpressions[value] !== undefined) {
         phIndex = currentExpressions[value];
-        // The placeholder represents an expression
+        // The placeholder represents an expression, add an instruction to move it
-        // Add an instruction to move the expression
         tmplInstructions.push(phIndex | I18nInstructions.Expression);
         phVisited.push(value);
       } else {
@@ -163,11 +166,13 @@ function generateMappingInstructions(
         maxIndex = phIndex;
       }
 
-      if (templateRoots && templateRoots.indexOf(value) !== -1 &&
+      if (templateRoots) {
-          templateRoots.indexOf(value) >= tmplIndex) {
+        const newTmplIndex = templateRoots.indexOf(value) + 1;
-        index = generateMappingInstructions(
+        if (newTmplIndex !== 0 && newTmplIndex !== tmplIndex) {
-            index, translationParts, instructions, elements, expressions, templateRoots,
+          partIndex = generateMappingInstructions(
-            lastChildIndex);
+              newTmplIndex, partIndex, translationParts, instructions, elements, expressions,
+              templateRoots, lastChildIndex);
+        }
       }
 
     } else if (value) {
@@ -237,7 +242,7 @@ function generateMappingInstructions(
     }
   }
 
-  return index;
+  return partIndex;
 }
 
 function appendI18nNode(node: LNode, parentNode: LNode, previousNode: LNode) {

packages/core/test/render3/i18n_spec.ts

@@ -720,6 +720,116 @@ describe('Runtime i18n', () => {
               '<ul><li>valeur: one!</li><li>valeur: two!</li><li>valeur bis: one!</li><li>valeur bis: two!</li></ul>');
     });
 
+    it('should support changing the order of multiple template roots in the same template', () => {
+      const MSG_DIV_SECTION_1 =
+          `{$START_LI_1}valeur bis: {$EXP_2}!{$END_LI_1}{$START_LI_0}valeur: {$EXP_1}!{$END_LI_0}`;
+      // The indexes are based on each template function
+      let i18n_1: I18nInstruction[][];
+      class MyApp {
+        items: string[] = ['1', '2'];
+
+        static ngComponentDef = defineComponent({
+          type: MyApp,
+          factory: () => new MyApp(),
+          selectors: [['my-app']],
+          // Initial template:
+          // <ul i18n>
+          //   <li *ngFor="let item of items">value: {{item}}</li>
+          //   <li *ngFor="let item of items">value bis: {{item}}</li>
+          // </ul>
+
+          // Translated to:
+          // <ul i18n>
+          //   <li *ngFor="let item of items">valeur bis: {{item}}!</li>
+          //   <li *ngFor="let item of items">valeur: {{item}}!</li>
+          // </ul>
+          template: (rf: RenderFlags, myApp: MyApp) => {
+            if (rf & RenderFlags.Create) {
+              if (!i18n_1) {
+                i18n_1 = i18nMapping(
+                    MSG_DIV_SECTION_1,
+                    [{'START_LI_0': 1, 'START_LI_1': 2}, {'START_LI_0': 0}, {'START_LI_1': 0}],
+                    [null, {'EXP_1': 1}, {'EXP_2': 1}], ['START_LI_0', 'START_LI_1']);
+              }
+
+              elementStart(0, 'ul');
+              {
+                // Start of translated section 1
+                container(1, liTemplate, null, ['ngForOf', '']);     // START_LI_0
+                container(2, liTemplateBis, null, ['ngForOf', '']);  // START_LI_1
+                // End of translated section 1
+              }
+              elementEnd();
+              i18nApply(1, i18n_1[0]);
+            }
+            if (rf & RenderFlags.Update) {
+              elementProperty(1, 'ngForOf', bind(myApp.items));
+              elementProperty(2, 'ngForOf', bind(myApp.items));
+            }
+
+            function liTemplate(rf1: RenderFlags, row: NgForOfContext<string>) {
+              if (rf1 & RenderFlags.Create) {
+                // This is a container so the whole template is a translated section
+                // Start of translated section 2
+                elementStart(0, 'li');  // START_LI_0
+                { text(1); }            // EXP_1
+                elementEnd();
+                // End of translated section 2
+                i18nApply(0, i18n_1[1]);
+              }
+              if (rf1 & RenderFlags.Update) {
+                textBinding(1, bind(row.$implicit));
+              }
+            }
+
+            function liTemplateBis(rf1: RenderFlags, row: NgForOfContext<string>) {
+              if (rf1 & RenderFlags.Create) {
+                // This is a container so the whole template is a translated section
+                // Start of translated section 3
+                elementStart(0, 'li');  // START_LI_1
+                { text(1); }            // EXP_2
+                elementEnd();
+                // End of translated section 3
+                i18nApply(0, i18n_1[2]);
+              }
+              if (rf1 & RenderFlags.Update) {
+                textBinding(1, bind(row.$implicit));
+              }
+            }
+          },
+          directives: () => [NgForOf]
+        });
+      }
+
+      const fixture = new ComponentFixture(MyApp);
+      expect(fixture.html)
+          .toEqual(
+              '<ul><li>valeur bis: 1!</li><li>valeur bis: 2!</li><li>valeur: 1!</li><li>valeur: 2!</li></ul>');
+
+      // Change detection cycle, no model changes
+      fixture.update();
+      expect(fixture.html)
+          .toEqual(
+              '<ul><li>valeur bis: 1!</li><li>valeur bis: 2!</li><li>valeur: 1!</li><li>valeur: 2!</li></ul>');
+
+      // Remove the last item
+      fixture.component.items.length = 1;
+      fixture.update();
+      expect(fixture.html).toEqual('<ul><li>valeur bis: 1!</li><li>valeur: 1!</li></ul>');
+
+      // Change an item
+      fixture.component.items[0] = 'one';
+      fixture.update();
+      expect(fixture.html).toEqual('<ul><li>valeur bis: one!</li><li>valeur: one!</li></ul>');
+
+      // Add an item
+      fixture.component.items.push('two');
+      fixture.update();
+      expect(fixture.html)
+          .toEqual(
+              '<ul><li>valeur bis: one!</li><li>valeur bis: two!</li><li>valeur: one!</li><li>valeur: two!</li></ul>');
+    });
+
     it('should support nested embedded templates', () => {
       const MSG_DIV_SECTION_1 = `{$START_LI}{$START_SPAN}valeur: {$EXP_1}!{$END_SPAN}{$END_LI}`;
       // The indexes are based on each template function
@@ -831,7 +941,7 @@ describe('Runtime i18n', () => {
               '<ul><li><span>valeur: one!</span><span>valeur: two!</span></li><li><span>valeur: one!</span><span>valeur: two!</span></li></ul>');
     });
 
-    it('should be able to move template directives around', () => {
+    it('should be able to move template roots around', () => {
       const MSG_DIV_SECTION_1 =
           `{$START_LI_0}début{$END_LI_0}{$START_LI_1}valeur: {$EXP_1}{$END_LI_1}fin`;
       // The indexes are based on each template function
@@ -928,7 +1038,7 @@ describe('Runtime i18n', () => {
           .toEqual('<ul><li>début</li><li>valeur: one</li><li>valeur: two</li>fin</ul>');
     });
 
-    it('should be able to remove containers', () => {
+    it('should be able to remove template roots', () => {
       const MSG_DIV_SECTION_1 = `loop`;
       // The indexes are based on each template function
       let i18n_1: I18nInstruction[][];