fix(migrations): handle reused templates in control flow migration (#63996)

aparzi · thePunderWoman · commit 3ebaeccb4661 · 2025-09-24T16:01:58.000Z
The control flow migration was incorrectly removing `ng-template` elements in scenarios where they were referenced by multiple `*ngIf` directives' `else` clauses and also used independently via `ngTemplateOutlet`. PR Close #63996
diff --git a/packages/core/schematics/migrations/control-flow-migration/util.ts b/packages/core/schematics/migrations/control-flow-migration/util.ts
@@ -33,6 +33,12 @@ const endMarkerRegex = new RegExp(endMarker, 'gm');
 const startI18nMarkerRegex = new RegExp(startI18nMarker, 'gm');
 const endI18nMarkerRegex = new RegExp(endI18nMarker, 'gm');
 const replaceMarkerRegex = new RegExp(`${startMarker}|${endMarker}`, 'gm');
+const PRIORITY_WEIGHT_TEMPLATE_REFERENCE_BY_OUTLET = 2;
+
+interface TemplateUsageResult {
+  isReferencedInTemplateOutlet: boolean;
+  totalCount: number;
+}
 
 /**
  * Analyzes a source file to find file that need to be migrated and the text ranges within them.
@@ -430,39 +436,6 @@ export function getTemplates(template: string): Map<string, Template> {
   return new Map<string, Template>();
 }
 
-function countTemplateUsage(nodes: any[], templateName: string): number {
-  let count = 0;
-  let isReferencedInTemplateOutlet = false;
-
-  for (const node of nodes) {
-    if (node.attrs) {
-      for (const attr of node.attrs) {
-        if (attr.name === '*ngTemplateOutlet' && attr.value === templateName.slice(1)) {
-          isReferencedInTemplateOutlet = true;
-          break;
-        }
-
-        if (attr.name.trim() === templateName) {
-          count++;
-        }
-      }
-    }
-
-    if (node.children) {
-      if (node.name === 'for') {
-        for (const child of node.children) {
-          if (child.value?.includes(templateName.slice(1))) {
-            count++;
-          }
-        }
-      }
-      count += countTemplateUsage(node.children, templateName);
-    }
-  }
-
-  return isReferencedInTemplateOutlet ? count + 2 : count;
-}
-
 export function updateTemplates(
   template: string,
   templates: Map<string, Template>,
@@ -499,9 +472,13 @@ export function processNgTemplates(
     const templates = getTemplates(template);
 
     // swap placeholders and remove
-    for (const [name, t] of templates) {
-      const replaceRegex = new RegExp(getPlaceholder(name.slice(1)), 'g');
-      const forRegex = new RegExp(getPlaceholder(name.slice(1), PlaceholderKind.Alternate), 'g');
+    for (const [nameWithHash, t] of templates) {
+      const name = nameWithHash.slice(1);
+      const replaceRegex = new RegExp(getPlaceholder(name), 'g');
+      const forRegex = new RegExp(
+        getPlaceholder(nameWithHash.slice(1), PlaceholderKind.Alternate),
+        'g',
+      );
       const forMatches = [...template.matchAll(forRegex)];
       const matches = [...forMatches, ...template.matchAll(replaceRegex)];
       let safeToRemove = true;
@@ -526,7 +503,15 @@ export function processNgTemplates(
           (obj, index, self) => index === self.findIndex((t) => t.input === obj.input),
         );
 
-        if ((t.count === dist.length || t.count - matches.length === 1) && safeToRemove) {
+        // Check if template is used by ngTemplateOutlet in addition to control flow
+        const hasTemplateOutletUsage = checkForTemplateOutletUsage(template, nameWithHash.slice(1));
+
+        // Only remove template if it's safe to do so AND not used by ngTemplateOutlet
+        if (
+          (t.count === dist.length || t.count - matches.length === 1) &&
+          safeToRemove &&
+          !hasTemplateOutletUsage
+        ) {
           const refsInComponentFile = getViewChildOrViewChildrenNames(sourceFile);
           if (refsInComponentFile?.length > 0) {
             const templateRefs = getTemplateReferences(template);
@@ -555,6 +540,71 @@ export function processNgTemplates(
   }
 }
 
+function analyzeTemplateUsage(nodes: any[], templateName: string): TemplateUsageResult {
+  let count = 0;
+  let isReferencedInTemplateOutlet = false;
+  const templateNameWithHash = `#${templateName}`;
+
+  function traverseNodes(nodeList: any[]): void {
+    for (const node of nodeList) {
+      if (node.attrs) {
+        for (const attr of node.attrs) {
+          if (
+            (attr.name === '*ngTemplateOutlet' || attr.name === '[ngTemplateOutlet]') &&
+            attr.value === templateName
+          ) {
+            isReferencedInTemplateOutlet = true;
+          }
+
+          if (attr.name.trim() === templateNameWithHash) {
+            count++;
+          }
+        }
+      }
+
+      if (node.children) {
+        if (node.name === 'for') {
+          for (const child of node.children) {
+            if (child.value?.includes(templateName)) {
+              count++;
+            }
+          }
+        }
+
+        traverseNodes(node.children);
+      }
+    }
+  }
+
+  traverseNodes(nodes);
+
+  return {
+    isReferencedInTemplateOutlet,
+    totalCount: isReferencedInTemplateOutlet
+      ? count + PRIORITY_WEIGHT_TEMPLATE_REFERENCE_BY_OUTLET
+      : count,
+  };
+}
+
+/**
+ * Checks if a template is used by ngTemplateOutlet directive
+ */
+function checkForTemplateOutletUsage(template: string, templateName: string): boolean {
+  const parsed = parseTemplate(template);
+  if (parsed.tree === undefined) {
+    return false;
+  }
+
+  const result = analyzeTemplateUsage(parsed.tree.rootNodes, templateName);
+  return result.isReferencedInTemplateOutlet;
+}
+
+function countTemplateUsage(nodes: any[], templateNameWithHash: string): number {
+  const templateName = templateNameWithHash.slice(1);
+  const result = analyzeTemplateUsage(nodes, templateName);
+  return result.totalCount;
+}
+
 function getViewChildOrViewChildrenNames(sourceFile: ts.SourceFile): Array<string> {
   const names: Array<string> = [];
 
@@ -585,12 +635,12 @@ function getTemplateReferences(template: string): string[] {
     return [];
   }
 
-  const references: string[] = [];
+  const templateNameRefWithoutHash: string[] = [];
 
   function visitNodes(nodes: any) {
     for (const node of nodes) {
       if (node?.name === 'ng-template') {
-        references.push(...node.attrs?.map((ref: any) => ref?.name?.slice(1)));
+        templateNameRefWithoutHash.push(...node.attrs?.map((ref: any) => ref?.name?.slice(1)));
       }
       if (node.children) {
         visitNodes(node.children);
@@ -599,7 +649,7 @@ function getTemplateReferences(template: string): string[] {
   }
 
   visitNodes(parsed.tree.rootNodes);
-  return references;
+  return templateNameRefWithoutHash;
 }
 
 function replaceRemainingPlaceholders(template: string): string {
diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts
@@ -5074,6 +5074,44 @@ describe('control flow migration (ng update)', () => {
         '@if (show) {<div>Some greek characters: θδ!</div>}',
       );
     });
+
+    it('should migrate multiple ngIf directives with same else template and preserve template outlet', async () => {
+      writeFile(
+        '/comp.ts',
+        `
+        import {Component} from '@angular/core';
+        import {NgIf} from '@angular/common';
+
+        @Component({
+          imports: [NgIf],
+          template: \`
+            <div *ngIf="1 == 1; else elseTemplate">
+              <h1>TEST</h1>
+            </div>
+            <div *ngIf="1 == 1; else elseTemplate">
+              <h1>TEST</h1>
+            </div>
+
+            <ng-container [ngTemplateOutlet]="elseTemplate"></ng-container>
+            <ng-template #elseTemplate>
+              <h1>Test</h1>
+              <div>Test</div>
+            </ng-template>
+          \`
+        })
+        class Comp {
+        }
+      `,
+      );
+
+      await runMigration();
+      const content = tree.readContent('/comp.ts');
+
+      expect(content.replace(/\s+/g, ' ')).toContain(
+        `<ng-container [ngTemplateOutlet]="elseTemplate"></ng-container>`,
+      );
+      expect(content.replace(/\s+/g, ' ')).toContain(`<ng-template #elseTemplate>`);
+    });
   });
 
   describe('formatting', () => {
