mirror of
https://github.com/angular/angular
synced 2026-05-24 09:28:37 +00:00
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
This commit is contained in:
parent
3c14d0523c
commit
3ebaeccb46
2 changed files with 128 additions and 40 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue