From 3ebaeccb466119ee43eeaa486f5e132c85e9caa2 Mon Sep 17 00:00:00 2001 From: aparziale Date: Wed, 24 Sep 2025 00:22:25 +0200 Subject: [PATCH] fix(migrations): handle reused templates in control flow migration (#63996) 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 --- .../migrations/control-flow-migration/util.ts | 130 ++++++++++++------ .../test/control_flow_migration_spec.ts | 38 +++++ 2 files changed, 128 insertions(+), 40 deletions(-) diff --git a/packages/core/schematics/migrations/control-flow-migration/util.ts b/packages/core/schematics/migrations/control-flow-migration/util.ts index 4c3fd969adc..aa53113e7dd 100644 --- 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 { return new Map(); } -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, @@ -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 { const names: Array = []; @@ -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 index a57f43f91ac..99feb55728a 100644 --- 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) {
Some greek characters: θδ!
}', ); }); + + 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: \` +
+

TEST

+
+
+

TEST

+
+ + + +

Test

+
Test
+ + \` + }) + class Comp { + } + `, + ); + + await runMigration(); + const content = tree.readContent('/comp.ts'); + + expect(content.replace(/\s+/g, ' ')).toContain( + ``, + ); + expect(content.replace(/\s+/g, ' ')).toContain(``); + }); }); describe('formatting', () => {