fix(migrations): handle nested ng-template replacement safely in CF migration (#53368)

thePunderWoman · dylhunn · commit 5104a89b3035 · 2023-12-05T17:19:37.000-08:00
When there are ng-templates nested inside other ng-templates, the replacement and removal of the templates gets disrupted. Re-processing the templates in the file along the way resolves this issue. fixes: #53362 PR Close #53368
diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts
@@ -284,7 +284,7 @@ export function getTemplates(template: string): Map<string, Template> {
     // count usages of each ng-template
     for (let [key, tmpl] of visitor.templates) {
       const escapeKey = escapeRegExp(key.slice(1));
-      const regex = new RegExp(`[^a-zA-Z0-9-<\']${escapeKey}\\W`, 'gm');
+      const regex = new RegExp(`[^a-zA-Z0-9-<(\']${escapeKey}\\W`, 'gm');
       const matches = template.match(regex);
       tmpl.count = matches?.length ?? 0;
       tmpl.generateContents(template);
@@ -295,6 +295,15 @@ export function getTemplates(template: string): Map<string, Template> {
   return new Map<string, Template>();
 }
 
+export function updateTemplates(
+    template: string, templates: Map<string, Template>): Map<string, Template> {
+  const updatedTemplates = getTemplates(template);
+  for (let [key, tmpl] of updatedTemplates) {
+    templates.set(key, tmpl);
+  }
+  return templates;
+}
+
 function wrapIntoI18nContainer(i18nAttr: Attribute, content: string) {
   const {start, middle, end} = generatei18nContainer(i18nAttr, content);
   return `${start}${middle}${end}`;
@@ -341,6 +350,9 @@ export function processNgTemplates(template: string): {migrated: string, err: Er
         if (t.count === matches.length + 1 && safeToRemove) {
           template = template.replace(t.contents, '');
         }
+        // templates may have changed structure from nested replaced templates
+        // so we need to reprocess them before the next loop.
+        updateTemplates(template, templates);
       }
     }
     return {migrated: template, err: undefined};
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -3970,6 +3970,48 @@ describe('control flow migration', () => {
         `}`,
       ].join('\n'));
     });
+
+    it('should migrate nested template usage correctly', async () => {
+      writeFile('/comp.ts', `
+        import {Component} from '@angular/core';
+        import {NgIf} from '@angular/common';
+
+        @Component({
+          templateUrl: './comp.html'
+        })
+        class Comp {
+          show = false;
+        }
+      `);
+
+      writeFile('/comp.html', [
+        `<ng-container *ngIf="!(condition$ | async); else template">`,
+        `  Hello!`,
+        `</ng-container>`,
+        `<ng-template #bar>Bar</ng-template>`,
+        `<ng-template #foo>Foo</ng-template>`,
+        `<ng-template #template>`,
+        `  <ng-container`,
+        `    *ngIf="(foo$ | async) === true; then foo; else bar"`,
+        `  ></ng-container>`,
+        `</ng-template>`,
+      ].join('\n'));
+
+      await runMigration();
+      const content = tree.readContent('/comp.html');
+
+      expect(content).toBe([
+        `@if (!(condition$ | async)) {`,
+        `  Hello!`,
+        `} @else {`,
+        `  @if ((foo$ | async) === true) {`,
+        `    Foo`,
+        `  } @else {`,
+        `    Bar`,
+        `  }`,
+        `}\n`,
+      ].join('\n'));
+    });
   });
 
   describe('formatting', () => {
