Skip to content

Commit aab7fb8

Browse files
thePunderWomanpkozlowski-opensource
thePunderWoman
authored and
pkozlowski-opensource
committed
fix(migrations): Add ngForTemplate support to control flow migration (#53076)
This adds code to cover the rare use case of an ngFor with a template param. fixes: #53068 PR Close #53076
1 parent 00cb333 commit aab7fb8

File tree

5 files changed

+257
-15
lines changed

5 files changed

+257
-15
lines changed

‎packages/core/schematics/ng-generate/control-flow-migration/fors.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe
9595
const loopVar = condition.split(' of ')[0];
9696
let trackBy = loopVar;
9797
let aliasedIndex: string|null = null;
98+
let tmplPlaceholder = '';
9899
for (let i = 1; i < parts.length; i++) {
99100
const part = parts[i].trim();
100101

@@ -103,6 +104,12 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe
103104
const trackByFn = part.replace('trackBy:', '').trim();
104105
trackBy = `${trackByFn}($index, ${loopVar})`;
105106
}
107+
// template
108+
if (part.startsWith('template:')) {
109+
// this generates a special template placeholder just for this use case
110+
// which has a # at the end instead of the standard | in other placeholders
111+
tmplPlaceholder = `#${part.split(':')[1].trim()}#`;
112+
}
106113
// aliases
107114
// declared with `let myIndex = index`
108115
if (part.match(aliasWithEqualRegexp)) {
@@ -136,11 +143,19 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe
136143

137144
const aliasStr = (aliases.length > 0) ? `;${aliases.join(';')}` : '';
138145

139-
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
140-
const startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {${lbString}${start}`;
141-
142-
const endBlock = `${end}${lbString}}`;
143-
const forBlock = startBlock + middle + endBlock;
146+
let startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {${lbString}`;
147+
let endBlock = `${lbString}}`;
148+
let forBlock = '';
149+
150+
if (tmplPlaceholder !== '') {
151+
startBlock = startBlock + tmplPlaceholder;
152+
forBlock = startBlock + endBlock;
153+
} else {
154+
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
155+
startBlock += start;
156+
endBlock = end + endBlock;
157+
forBlock = startBlock + middle + endBlock;
158+
}
144159

145160
const updatedTmpl = tmpl.slice(0, etm.start(offset)) + forBlock + tmpl.slice(etm.end(offset));
146161

‎packages/core/schematics/ng-generate/control-flow-migration/ifs.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ export function migrateIf(template: string):
6868
}
6969

7070
function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Result {
71-
const matchThen = etm.attr.value.match(/;\s*then/gm);
72-
const matchElse = etm.attr.value.match(/;\s*else/gm);
71+
const matchThen = etm.attr.value.match(/;?\s*then/gm);
72+
const matchElse = etm.attr.value.match(/;?\s*else/gm);
7373

7474
if (etm.thenAttr !== undefined || etm.elseAttr !== undefined) {
7575
// bound if then / if then else

‎packages/core/schematics/ng-generate/control-flow-migration/types.ts‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,19 @@ export class ElementToMigrate {
116116

117117
getCondition(): string {
118118
const chunks = this.attr.value.split(';');
119+
let condition = chunks[0];
120+
121+
// checks for case of no usage of `;` in if else / if then else
122+
const elseIx = condition.indexOf(' else ');
123+
const thenIx = condition.indexOf(' then ');
124+
if (thenIx > -1) {
125+
condition = condition.slice(0, thenIx);
126+
} else if (elseIx > -1) {
127+
condition = condition.slice(0, elseIx);
128+
}
129+
119130
let letVar = chunks.find(c => c.search(/\s*let\s/) > -1);
120-
return chunks[0] + (letVar ? ';' + letVar : '');
131+
return condition + (letVar ? ';' + letVar : '');
121132
}
122133

123134
getTemplateName(targetStr: string, secondStr?: string): string {
@@ -150,16 +161,35 @@ export class ElementToMigrate {
150161
*/
151162
export class Template {
152163
el: Element;
164+
name: string;
153165
count: number = 0;
154166
contents: string = '';
155167
children: string = '';
156168
i18n: Attribute|null = null;
169+
attributes: Attribute[];
157170

158-
constructor(el: Element, i18n: Attribute|null) {
171+
constructor(el: Element, name: string, i18n: Attribute|null) {
159172
this.el = el;
173+
this.name = name;
174+
this.attributes = el.attrs;
160175
this.i18n = i18n;
161176
}
162177

178+
get isNgTemplateOutlet() {
179+
return this.attributes.find(attr => attr.name === '*ngTemplateOutlet') !== undefined;
180+
}
181+
182+
get outletContext() {
183+
const letVar = this.attributes.find(attr => attr.name.startsWith('let-'));
184+
return letVar ? `; context: {$implicit: ${letVar.name.split('-')[1]}}` : '';
185+
}
186+
187+
generateTemplateOutlet() {
188+
const attr = this.attributes.find(attr => attr.name === '*ngTemplateOutlet');
189+
const outletValue = attr?.value ?? this.name.slice(1);
190+
return `<ng-container *ngTemplateOutlet="${outletValue}${this.outletContext}"></ng-container>`;
191+
}
192+
163193
generateContents(tmpl: string) {
164194
this.contents = tmpl.slice(this.el.sourceSpan.start.offset, this.el.sourceSpan.end.offset + 1);
165195
this.children = '';
@@ -307,7 +337,7 @@ export class TemplateCollector extends RecursiveVisitor {
307337
}
308338
if (templateAttr !== null) {
309339
this.elements.push(new ElementToMigrate(el, templateAttr));
310-
this.templates.set(templateAttr.name, new Template(el, i18n));
340+
this.templates.set(templateAttr.name, new Template(el, templateAttr.name, i18n));
311341
}
312342
}
313343
super.visitElement(el, null);

‎packages/core/schematics/ng-generate/control-flow-migration/util.ts‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ export function reduceNestingOffset(
269269
* Replaces structural directive control flow instances with block control flow equivalents.
270270
* Returns null if the migration failed (e.g. there was a syntax error).
271271
*/
272-
export function countTemplateUsage(template: string): Map<string, Template> {
272+
export function getTemplates(template: string): Map<string, Template> {
273273
const parsed = parseTemplate(template);
274274
if (parsed !== null) {
275275
const visitor = new TemplateCollector();
@@ -299,22 +299,33 @@ function wrapIntoI18nContainer(i18nAttr: Attribute, content: string) {
299299
*/
300300
export function processNgTemplates(template: string): string {
301301
// count usage
302-
const templates = countTemplateUsage(template);
302+
const templates = getTemplates(template);
303303

304304
// swap placeholders and remove
305305
for (const [name, t] of templates) {
306306
const replaceRegex = new RegExp(`${name}\\|`, 'g');
307-
const matches = [...template.matchAll(replaceRegex)];
307+
const forRegex = new RegExp(`${name}\\#`, 'g');
308+
const forMatches = [...template.matchAll(forRegex)];
309+
const matches = [...forMatches, ...template.matchAll(replaceRegex)];
310+
let safeToRemove = true;
308311
if (matches.length > 0) {
309312
if (t.i18n !== null) {
310313
const container = wrapIntoI18nContainer(t.i18n, t.children);
311314
template = template.replace(replaceRegex, container);
315+
} else if (t.children.trim() === '' && t.isNgTemplateOutlet) {
316+
template = template.replace(replaceRegex, t.generateTemplateOutlet());
317+
} else if (forMatches.length > 0) {
318+
if (t.count === 2) {
319+
template = template.replace(forRegex, t.children);
320+
} else {
321+
template = template.replace(forRegex, t.generateTemplateOutlet());
322+
safeToRemove = false;
323+
}
312324
} else {
313325
template = template.replace(replaceRegex, t.children);
314326
}
315-
316327
// the +1 accounts for the t.count's counting of the original template
317-
if (t.count === matches.length + 1) {
328+
if (t.count === matches.length + 1 && safeToRemove) {
318329
template = template.replace(t.contents, '');
319330
}
320331
}

‎packages/core/schematics/test/control_flow_migration_spec.ts‎

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,75 @@ describe('control flow migration', () => {
654654
].join('\n'));
655655
});
656656

657+
it('should migrate an if else case with no semicolon before else', async () => {
658+
writeFile('/comp.ts', `
659+
import {Component} from '@angular/core';
660+
import {NgIf} from '@angular/common';
661+
662+
@Component({
663+
templateUrl: './comp.html'
664+
})
665+
class Comp {
666+
show = false;
667+
}
668+
`);
669+
670+
writeFile('/comp.html', [
671+
`<div>`,
672+
`<span *ngIf="show else elseBlock">Content here</span>`,
673+
`<ng-template #elseBlock>Else Content</ng-template>`,
674+
`</div>`,
675+
].join('\n'));
676+
677+
await runMigration();
678+
const content = tree.readContent('/comp.html');
679+
680+
expect(content).toBe([
681+
`<div>`,
682+
` @if (show) {`,
683+
` <span>Content here</span>`,
684+
` } @else {`,
685+
` Else Content`,
686+
` }`,
687+
`</div>`,
688+
].join('\n'));
689+
});
690+
691+
it('should migrate an if then else case with no semicolons', async () => {
692+
writeFile('/comp.ts', `
693+
import {Component} from '@angular/core';
694+
import {NgIf} from '@angular/common';
695+
696+
@Component({
697+
templateUrl: './comp.html'
698+
})
699+
class Comp {
700+
show = false;
701+
}
702+
`);
703+
704+
writeFile('/comp.html', [
705+
`<div>`,
706+
`<span *ngIf="show then thenTmpl else elseTmpl"></span>`,
707+
`<ng-template #thenTmpl><span>Content here</span></ng-template>`,
708+
`<ng-template #elseTmpl>Else Content</ng-template>`,
709+
`</div>`,
710+
].join('\n'));
711+
712+
await runMigration();
713+
const content = tree.readContent('/comp.html');
714+
715+
expect(content).toBe([
716+
`<div>`,
717+
` @if (show) {`,
718+
` <span>Content here</span>`,
719+
` } @else {`,
720+
` Else Content`,
721+
` }`,
722+
`</div>`,
723+
].join('\n'));
724+
});
725+
657726
it('should migrate an if else case with no space after ;', async () => {
658727
writeFile('/comp.ts', `
659728
import {Component} from '@angular/core';
@@ -1703,6 +1772,87 @@ describe('control flow migration', () => {
17031772
expect(content).toContain(
17041773
'template: `<ul>@for (itm of \'1,2,3\'; track itm) {<li>{{itm}}</li>}</ul>`');
17051774
});
1775+
1776+
it('should migrate ngForTemplate', async () => {
1777+
writeFile('/comp.ts', `
1778+
import {Component} from '@angular/core';
1779+
import {NgFor} from '@angular/common';
1780+
interface Item {
1781+
id: number;
1782+
text: string;
1783+
}
1784+
1785+
@Component({
1786+
imports: [NgFor],
1787+
templateUrl: 'comp.html',
1788+
})
1789+
class Comp {
1790+
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
1791+
}
1792+
`);
1793+
1794+
writeFile('/comp.html', [
1795+
`<ng-container *ngFor="let item of things; template: itemTemplate"></ng-container>`,
1796+
`<ng-container *ngFor="let item of items; template: itemTemplate"></ng-container>`,
1797+
`<ng-template #itemTemplate let-item>`,
1798+
` <p>Stuff</p>`,
1799+
`</ng-template>`,
1800+
].join('\n'));
1801+
1802+
await runMigration();
1803+
const actual = tree.readContent('/comp.html');
1804+
1805+
const expected = [
1806+
`@for (item of things; track item) {`,
1807+
` <ng-container *ngTemplateOutlet="itemTemplate; context: {$implicit: item}"></ng-container>`,
1808+
`}`,
1809+
`@for (item of items; track item) {`,
1810+
` <ng-container *ngTemplateOutlet="itemTemplate; context: {$implicit: item}"></ng-container>`,
1811+
`}`,
1812+
`<ng-template #itemTemplate let-item>`,
1813+
` <p>Stuff</p>`,
1814+
`</ng-template>`,
1815+
].join('\n');
1816+
1817+
expect(actual).toBe(expected);
1818+
});
1819+
1820+
it('should migrate ngForTemplate when template is only used once', async () => {
1821+
writeFile('/comp.ts', `
1822+
import {Component} from '@angular/core';
1823+
import {NgFor} from '@angular/common';
1824+
interface Item {
1825+
id: number;
1826+
text: string;
1827+
}
1828+
1829+
@Component({
1830+
imports: [NgFor],
1831+
templateUrl: 'comp.html',
1832+
})
1833+
class Comp {
1834+
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
1835+
}
1836+
`);
1837+
1838+
writeFile('/comp.html', [
1839+
`<ng-container *ngFor="let item of items; template: itemTemplate"></ng-container>`,
1840+
`<ng-template #itemTemplate let-item>`,
1841+
` <p>Stuff</p>`,
1842+
`</ng-template>`,
1843+
].join('\n'));
1844+
1845+
await runMigration();
1846+
const actual = tree.readContent('/comp.html');
1847+
1848+
const expected = [
1849+
`@for (item of items; track item) {`,
1850+
` <p>Stuff</p>`,
1851+
`}\n`,
1852+
].join('\n');
1853+
1854+
expect(actual).toBe(expected);
1855+
});
17061856
});
17071857

17081858
describe('ngForOf', () => {
@@ -3155,6 +3305,42 @@ describe('control flow migration', () => {
31553305
expect(content).toBe(result);
31563306
});
31573307

3308+
it('should migrate template with ngTemplateOutlet', async () => {
3309+
writeFile('/comp.ts', `
3310+
import {Component} from '@angular/core';
3311+
import {NgIf} from '@angular/common';
3312+
3313+
@Component({
3314+
selector: 'declare-comp',
3315+
templateUrl: 'comp.html',
3316+
})
3317+
class DeclareComp {}
3318+
`);
3319+
3320+
writeFile('/comp.html', [
3321+
`<ng-template [ngIf]="!thing" [ngIfElse]="elseTmpl">`,
3322+
` {{ this.value(option) }}`,
3323+
`</ng-template>`,
3324+
`<ng-template`,
3325+
` #elseTmpl`,
3326+
` *ngTemplateOutlet="thing; context: {option: option}">`,
3327+
`</ng-template>`,
3328+
].join('\n'));
3329+
3330+
await runMigration();
3331+
const content = tree.readContent('/comp.html');
3332+
3333+
const result = [
3334+
`@if (!thing) {`,
3335+
` {{ this.value(option) }}`,
3336+
`} @else {`,
3337+
` <ng-container *ngTemplateOutlet="thing; context: {option: option}"></ng-container>`,
3338+
`}\n`,
3339+
].join('\n');
3340+
3341+
expect(content).toBe(result);
3342+
});
3343+
31583344
it('should move templates after they were migrated to new syntax', async () => {
31593345
writeFile('/comp.ts', `
31603346
import {Component} from '@angular/core';

0 commit comments

Comments
 (0)