Skip to content

Commit

Permalink
fix(migrations): Add ngForTemplate support to control flow migration (#…
Browse files Browse the repository at this point in the history 
…53076)

This adds code to cover the rare use case of an ngFor with a template param.
fixes: #53068

PR Close #53076
  • Loading branch information
@thePunderWoman @pkozlowski-opensource
thePunderWoman authored and pkozlowski-opensource committed Nov 28, 2023
1 parent 00cb333 commit aab7fb8
Show file tree
Hide file tree
Showing 5 changed files with 257 additions and 15 deletions.
25 changes: 20 additions & 5 deletions packages/core/schematics/ng-generate/control-flow-migration/fors.ts
View file
Expand Up @@ -95,6 +95,7 @@ function migrateStandardNgFor(etm: ElementToMigrate, tmpl: string, offset: numbe
const loopVar = condition.split(' of ')[0];
let trackBy = loopVar;
let aliasedIndex: string|null = null;
let tmplPlaceholder = '';
for (let i = 1; i < parts.length; i++) {
const part = parts[i].trim();

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

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

const {start, middle, end} = getMainBlock(etm, tmpl, offset);
const startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {${lbString}${start}`;

const endBlock = `${end}${lbString}}`;
const forBlock = startBlock + middle + endBlock;
let startBlock = `@for (${condition}; track ${trackBy}${aliasStr}) {${lbString}`;
let endBlock = `${lbString}}`;
let forBlock = '';

if (tmplPlaceholder !== '') {
startBlock = startBlock + tmplPlaceholder;
forBlock = startBlock + endBlock;
} else {
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
startBlock += start;
endBlock = end + endBlock;
forBlock = startBlock + middle + endBlock;
}

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

Expand Down
4 changes: 2 additions & 2 deletions packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
View file
Expand Up @@ -68,8 +68,8 @@ export function migrateIf(template: string):
}

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

if (etm.thenAttr !== undefined || etm.elseAttr !== undefined) {
// bound if then / if then else
Expand Down
36 changes: 33 additions & 3 deletions packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file
Expand Up @@ -116,8 +116,19 @@ export class ElementToMigrate {

getCondition(): string {
const chunks = this.attr.value.split(';');
let condition = chunks[0];

// checks for case of no usage of `;` in if else / if then else
const elseIx = condition.indexOf(' else ');
const thenIx = condition.indexOf(' then ');
if (thenIx > -1) {
condition = condition.slice(0, thenIx);
} else if (elseIx > -1) {
condition = condition.slice(0, elseIx);
}

let letVar = chunks.find(c => c.search(/\s*let\s/) > -1);
return chunks[0] + (letVar ? ';' + letVar : '');
return condition + (letVar ? ';' + letVar : '');
}

getTemplateName(targetStr: string, secondStr?: string): string {
Expand Down Expand Up @@ -150,16 +161,35 @@ export class ElementToMigrate {
*/
export class Template {
el: Element;
name: string;
count: number = 0;
contents: string = '';
children: string = '';
i18n: Attribute|null = null;
attributes: Attribute[];

constructor(el: Element, i18n: Attribute|null) {
constructor(el: Element, name: string, i18n: Attribute|null) {
this.el = el;
this.name = name;
this.attributes = el.attrs;
this.i18n = i18n;
}

get isNgTemplateOutlet() {
return this.attributes.find(attr => attr.name === '*ngTemplateOutlet') !== undefined;
}

get outletContext() {
const letVar = this.attributes.find(attr => attr.name.startsWith('let-'));
return letVar ? `; context: {$implicit: ${letVar.name.split('-')[1]}}` : '';
}

generateTemplateOutlet() {
const attr = this.attributes.find(attr => attr.name === '*ngTemplateOutlet');
const outletValue = attr?.value ?? this.name.slice(1);
return `<ng-container *ngTemplateOutlet="${outletValue}${this.outletContext}"></ng-container>`;
}

generateContents(tmpl: string) {
this.contents = tmpl.slice(this.el.sourceSpan.start.offset, this.el.sourceSpan.end.offset + 1);
this.children = '';
Expand Down Expand Up @@ -307,7 +337,7 @@ export class TemplateCollector extends RecursiveVisitor {
}
if (templateAttr !== null) {
this.elements.push(new ElementToMigrate(el, templateAttr));
this.templates.set(templateAttr.name, new Template(el, i18n));
this.templates.set(templateAttr.name, new Template(el, templateAttr.name, i18n));
}
}
super.visitElement(el, null);
Expand Down
21 changes: 16 additions & 5 deletions packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file
Expand Up @@ -269,7 +269,7 @@ export function reduceNestingOffset(
* Replaces structural directive control flow instances with block control flow equivalents.
* Returns null if the migration failed (e.g. there was a syntax error).
*/
export function countTemplateUsage(template: string): Map<string, Template> {
export function getTemplates(template: string): Map<string, Template> {
const parsed = parseTemplate(template);
if (parsed !== null) {
const visitor = new TemplateCollector();
Expand Down Expand Up @@ -299,22 +299,33 @@ function wrapIntoI18nContainer(i18nAttr: Attribute, content: string) {
*/
export function processNgTemplates(template: string): string {
// count usage
const templates = countTemplateUsage(template);
const templates = getTemplates(template);

// swap placeholders and remove
for (const [name, t] of templates) {
const replaceRegex = new RegExp(`${name}\\|`, 'g');
const matches = [...template.matchAll(replaceRegex)];
const forRegex = new RegExp(`${name}\\#`, 'g');
const forMatches = [...template.matchAll(forRegex)];
const matches = [...forMatches, ...template.matchAll(replaceRegex)];
let safeToRemove = true;
if (matches.length > 0) {
if (t.i18n !== null) {
const container = wrapIntoI18nContainer(t.i18n, t.children);
template = template.replace(replaceRegex, container);
} else if (t.children.trim() === '' && t.isNgTemplateOutlet) {
template = template.replace(replaceRegex, t.generateTemplateOutlet());
} else if (forMatches.length > 0) {
if (t.count === 2) {
template = template.replace(forRegex, t.children);
} else {
template = template.replace(forRegex, t.generateTemplateOutlet());
safeToRemove = false;
}
} else {
template = template.replace(replaceRegex, t.children);
}

// the +1 accounts for the t.count's counting of the original template
if (t.count === matches.length + 1) {
if (t.count === matches.length + 1 && safeToRemove) {
template = template.replace(t.contents, '');
}
}
Expand Down
186 changes: 186 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
View file
Expand Up @@ -654,6 +654,75 @@ describe('control flow migration', () => {
].join('\n'));
});

it('should migrate an if else case with no semicolon before else', 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', [
`<div>`,
`<span *ngIf="show else elseBlock">Content here</span>`,
`<ng-template #elseBlock>Else Content</ng-template>`,
`</div>`,
].join('\n'));

await runMigration();
const content = tree.readContent('/comp.html');

expect(content).toBe([
`<div>`,
` @if (show) {`,
` <span>Content here</span>`,
` } @else {`,
` Else Content`,
` }`,
`</div>`,
].join('\n'));
});

it('should migrate an if then else case with no semicolons', 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', [
`<div>`,
`<span *ngIf="show then thenTmpl else elseTmpl"></span>`,
`<ng-template #thenTmpl><span>Content here</span></ng-template>`,
`<ng-template #elseTmpl>Else Content</ng-template>`,
`</div>`,
].join('\n'));

await runMigration();
const content = tree.readContent('/comp.html');

expect(content).toBe([
`<div>`,
` @if (show) {`,
` <span>Content here</span>`,
` } @else {`,
` Else Content`,
` }`,
`</div>`,
].join('\n'));
});

it('should migrate an if else case with no space after ;', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
Expand Down Expand Up @@ -1703,6 +1772,87 @@ describe('control flow migration', () => {
expect(content).toContain(
'template: `<ul>@for (itm of \'1,2,3\'; track itm) {<li>{{itm}}</li>}</ul>`');
});

it('should migrate ngForTemplate', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgFor} from '@angular/common';
interface Item {
id: number;
text: string;
}
@Component({
imports: [NgFor],
templateUrl: 'comp.html',
})
class Comp {
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
}
`);

writeFile('/comp.html', [
`<ng-container *ngFor="let item of things; template: itemTemplate"></ng-container>`,
`<ng-container *ngFor="let item of items; template: itemTemplate"></ng-container>`,
`<ng-template #itemTemplate let-item>`,
` <p>Stuff</p>`,
`</ng-template>`,
].join('\n'));

await runMigration();
const actual = tree.readContent('/comp.html');

const expected = [
`@for (item of things; track item) {`,
` <ng-container *ngTemplateOutlet="itemTemplate; context: {$implicit: item}"></ng-container>`,
`}`,
`@for (item of items; track item) {`,
` <ng-container *ngTemplateOutlet="itemTemplate; context: {$implicit: item}"></ng-container>`,
`}`,
`<ng-template #itemTemplate let-item>`,
` <p>Stuff</p>`,
`</ng-template>`,
].join('\n');

expect(actual).toBe(expected);
});

it('should migrate ngForTemplate when template is only used once', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgFor} from '@angular/common';
interface Item {
id: number;
text: string;
}
@Component({
imports: [NgFor],
templateUrl: 'comp.html',
})
class Comp {
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
}
`);

writeFile('/comp.html', [
`<ng-container *ngFor="let item of items; template: itemTemplate"></ng-container>`,
`<ng-template #itemTemplate let-item>`,
` <p>Stuff</p>`,
`</ng-template>`,
].join('\n'));

await runMigration();
const actual = tree.readContent('/comp.html');

const expected = [
`@for (item of items; track item) {`,
` <p>Stuff</p>`,
`}\n`,
].join('\n');

expect(actual).toBe(expected);
});
});

describe('ngForOf', () => {
Expand Down Expand Up @@ -3155,6 +3305,42 @@ describe('control flow migration', () => {
expect(content).toBe(result);
});

it('should migrate template with ngTemplateOutlet', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';
@Component({
selector: 'declare-comp',
templateUrl: 'comp.html',
})
class DeclareComp {}
`);

writeFile('/comp.html', [
`<ng-template [ngIf]="!thing" [ngIfElse]="elseTmpl">`,
` {{ this.value(option) }}`,
`</ng-template>`,
`<ng-template`,
` #elseTmpl`,
` *ngTemplateOutlet="thing; context: {option: option}">`,
`</ng-template>`,
].join('\n'));

await runMigration();
const content = tree.readContent('/comp.html');

const result = [
`@if (!thing) {`,
` {{ this.value(option) }}`,
`} @else {`,
` <ng-container *ngTemplateOutlet="thing; context: {option: option}"></ng-container>`,
`}\n`,
].join('\n');

expect(content).toBe(result);
});

it('should move templates after they were migrated to new syntax', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
Expand Down

0 comments on commit aab7fb8

Please sign in to comment.