Skip to content

Commit 42805e3

Browse files
thePunderWomanthePunderWoman
committed
fix(migrations): Add support for bound versions of NgIfElse and NgIfThenElse (#52869)
This ensures the bound version of NgIfElse and NgIfThenElse work properly with the migration. fixes: #52842 PR Close #52869
1 parent 26e980d commit 42805e3

File tree

4 files changed

+192
-15
lines changed

4 files changed

+192
-15
lines changed

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

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,14 @@ function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Resul
6868
const matchThen = etm.attr.value.match(/;\s*then/gm);
6969
const matchElse = etm.attr.value.match(/;\s*else/gm);
7070

71-
if (matchThen && matchThen.length > 0) {
72-
return buildIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
73-
} else if (matchElse && matchElse.length > 0) {
71+
if (etm.thenAttr !== undefined || etm.elseAttr !== undefined) {
72+
// bound if then / if then else
73+
return buildBoundIfElseBlock(etm, tmpl, offset);
74+
} else if (matchThen && matchThen.length > 0) {
75+
return buildStandardIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
76+
} else if ((matchElse && matchElse.length > 0)) {
7477
// just else
75-
return buildIfElseBlock(etm, tmpl, matchElse[0], offset);
78+
return buildStandardIfElseBlock(etm, tmpl, matchElse![0], offset);
7679
}
7780

7881
return buildIfBlock(etm, tmpl, offset);
@@ -100,15 +103,32 @@ function buildIfBlock(etm: ElementToMigrate, tmpl: string, offset: number): Resu
100103
return {tmpl: updatedTmpl, offsets: {pre, post}};
101104
}
102105

103-
function buildIfElseBlock(
106+
function buildStandardIfElseBlock(
104107
etm: ElementToMigrate, tmpl: string, elseString: string, offset: number): Result {
105108
// includes the mandatory semicolon before as
106-
const lbString = etm.hasLineBreaks ? '\n' : '';
107109
const condition = etm.getCondition(elseString).replace(' as ', '; as ');
110+
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
111+
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
112+
}
113+
114+
function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result {
115+
// includes the mandatory semicolon before as
116+
const condition = etm.attr.value.replace(' as ', '; as ');
117+
const elsePlaceholder = `#${etm.elseAttr!.value}|`;
118+
if (etm.thenAttr !== undefined) {
119+
const thenPlaceholder = `#${etm.thenAttr!.value}|`;
120+
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
121+
}
122+
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
123+
}
124+
125+
function buildIfElseBlock(
126+
etm: ElementToMigrate, tmpl: string, condition: string, elsePlaceholder: string,
127+
offset: number): Result {
128+
const lbString = etm.hasLineBreaks ? '\n' : '';
108129

109130
const originals = getOriginals(etm, tmpl, offset);
110131

111-
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
112132
const {start, middle, end} = getMainBlock(etm, tmpl, offset);
113133
const startBlock = `@if (${condition}) {${lbString}${start}`;
114134

@@ -127,20 +147,26 @@ function buildIfElseBlock(
127147
return {tmpl: updatedTmpl, offsets: {pre, post}};
128148
}
129149

130-
function buildIfThenElseBlock(
150+
function buildStandardIfThenElseBlock(
131151
etm: ElementToMigrate, tmpl: string, thenString: string, elseString: string,
132152
offset: number): Result {
153+
// includes the mandatory semicolon before as
133154
const condition = etm.getCondition(thenString).replace(' as ', '; as ');
155+
const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
156+
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
157+
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
158+
}
159+
160+
function buildIfThenElseBlock(
161+
etm: ElementToMigrate, tmpl: string, condition: string, thenPlaceholder: string,
162+
elsePlaceholder: string, offset: number): Result {
134163
const lbString = etm.hasLineBreaks ? '\n' : '';
135164

136165
const originals = getOriginals(etm, tmpl, offset);
137166

138167
const startBlock = `@if (${condition}) {${lbString}`;
139168
const elseBlock = `${lbString}} @else {${lbString}`;
140169

141-
const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
142-
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
143-
144170
const postBlock = thenPlaceholder + elseBlock + elsePlaceholder + `${lbString}}`;
145171
const ifThenElseBlock = startBlock + postBlock;
146172

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {Attribute, Element, RecursiveVisitor, Text} from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
export const ngtemplate = 'ng-template';
13+
export const boundngifelse = '[ngIfElse]';
14+
export const boundngifthenelse = '[ngIfThenElse]';
1315

1416
function allFormsOf(selector: string): string[] {
1517
return [
@@ -84,12 +86,18 @@ export type MigrateError = {
8486
export class ElementToMigrate {
8587
el: Element;
8688
attr: Attribute;
89+
elseAttr: Attribute|undefined;
90+
thenAttr: Attribute|undefined;
8791
nestCount = 0;
8892
hasLineBreaks = false;
8993

90-
constructor(el: Element, attr: Attribute) {
94+
constructor(
95+
el: Element, attr: Attribute, elseAttr: Attribute|undefined = undefined,
96+
thenAttr: Attribute|undefined = undefined) {
9197
this.el = el;
9298
this.attr = attr;
99+
this.elseAttr = elseAttr;
100+
this.thenAttr = thenAttr;
93101
}
94102

95103
getCondition(targetStr: string): string {
@@ -226,7 +234,9 @@ export class ElementCollector extends RecursiveVisitor {
226234
if (el.attrs.length > 0) {
227235
for (const attr of el.attrs) {
228236
if (this._attributes.includes(attr.name)) {
229-
this.elements.push(new ElementToMigrate(el, attr));
237+
const elseAttr = el.attrs.find(x => x.name === boundngifelse);
238+
const thenAttr = el.attrs.find(x => x.name === boundngifthenelse);
239+
this.elements.push(new ElementToMigrate(el, attr, elseAttr, thenAttr));
230240
}
231241
}
232242
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,13 +360,16 @@ export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number
360360
{start: string, middle: string, end: string} {
361361
const i18nAttr = etm.el.attrs.find(x => x.name === 'i18n');
362362
if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
363-
etm.el.attrs.length === 1) {
363+
(etm.el.attrs.length === 1 || (etm.el.attrs.length === 2 && etm.elseAttr !== undefined) ||
364+
(etm.el.attrs.length === 3 && etm.elseAttr !== undefined && etm.thenAttr !== undefined))) {
364365
// this is the case where we're migrating and there's no need to keep the ng-container
365366
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
366367
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
367368
const middle = tmpl.slice(childStart, childEnd);
368369
return {start: '', middle, end: ''};
369-
} else if (etm.el.name === 'ng-template' && etm.el.attrs.length === 2 && i18nAttr !== undefined) {
370+
} else if (
371+
etm.el.name === 'ng-template' && i18nAttr !== undefined &&
372+
(etm.el.attrs.length === 2 || (etm.el.attrs.length === 3 && etm.elseAttr !== undefined))) {
370373
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
371374
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
372375
const middle = wrapIntoI18nContainer(i18nAttr, tmpl.slice(childStart, childEnd));

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

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,75 @@ describe('control flow migration', () => {
328328
].join('\n'));
329329
});
330330

331+
it('should migrate an if else case as bindings', async () => {
332+
writeFile('/comp.ts', `
333+
import {Component} from '@angular/core';
334+
import {NgIf,NgIfElse} from '@angular/common';
335+
336+
@Component({
337+
templateUrl: './comp.html'
338+
})
339+
class Comp {
340+
show = false;
341+
}
342+
`);
343+
344+
writeFile('/comp.html', [
345+
`<div>`,
346+
`<ng-template [ngIf]="show" [ngIfElse]="tmplName"><span>Content here</span></ng-template>`,
347+
`</div>`,
348+
`<ng-template #tmplName><p>Stuff</p></ng-template>`,
349+
].join('\n'));
350+
351+
await runMigration();
352+
const content = tree.readContent('/comp.html');
353+
354+
expect(content).toBe([
355+
`<div>`,
356+
`@if (show) {`,
357+
`<span>Content here</span>`,
358+
`} @else {`,
359+
`<p>Stuff</p>`,
360+
`}`,
361+
`</div>\n`,
362+
].join('\n'));
363+
});
364+
365+
it('should migrate an if then else case as bindings', async () => {
366+
writeFile('/comp.ts', `
367+
import {Component} from '@angular/core';
368+
import {NgIf,NgIfElse} from '@angular/common';
369+
370+
@Component({
371+
templateUrl: './comp.html'
372+
})
373+
class Comp {
374+
show = false;
375+
}
376+
`);
377+
378+
writeFile('/comp.html', [
379+
`<div>`,
380+
`<ng-template [ngIf]="show" [ngIfElse]="elseTmpl" [ngIfThenElse]="thenTmpl">Ignore Me</ng-template>`,
381+
`</div>`,
382+
`<ng-template #elseTmpl><p>Stuff</p></ng-template>`,
383+
`<ng-template #thenTmpl><span>Content here</span></ng-template>`,
384+
].join('\n'));
385+
386+
await runMigration();
387+
const content = tree.readContent('/comp.html');
388+
389+
expect(content).toBe([
390+
`<div>`,
391+
`@if (show) {`,
392+
`<span>Content here</span>`,
393+
`} @else {`,
394+
`<p>Stuff</p>`,
395+
`}`,
396+
`</div>\n`,
397+
].join('\n'));
398+
});
399+
331400
it('should migrate an if case on a container', async () => {
332401
writeFile('/comp.ts', `
333402
import {Component} from '@angular/core';
@@ -483,6 +552,75 @@ describe('control flow migration', () => {
483552
].join('\n'));
484553
});
485554

555+
it('should migrate a bound NgIfThenElse case with ng-templates with i18n', async () => {
556+
writeFile('/comp.ts', `
557+
import {Component} from '@angular/core';
558+
import {NgIf} from '@angular/common';
559+
560+
@Component({
561+
templateUrl: './comp.html'
562+
})
563+
class Comp {
564+
show = false;
565+
}
566+
`);
567+
568+
writeFile('/comp.html', [
569+
`<div>`,
570+
`<ng-template [ngIf]="show" [ngIfThenElse]="thenTmpl" [ngIfElse]="elseTmpl">ignored</ng-template>`,
571+
`</div>`,
572+
`<ng-template #thenTmpl i18n="@@something"><span>Content here</span></ng-template>`,
573+
`<ng-template #elseTmpl i18n="@@somethingElse"><p>other</p></ng-template>`,
574+
].join('\n'));
575+
576+
await runMigration();
577+
const content = tree.readContent('/comp.html');
578+
579+
expect(content).toBe([
580+
`<div>`,
581+
`@if (show) {`,
582+
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
583+
`} @else {`,
584+
`<ng-container i18n="@@somethingElse"><p>other</p></ng-container>`,
585+
`}`,
586+
`</div>\n`,
587+
].join('\n'));
588+
});
589+
590+
it('should migrate a bound NgIfElse case with ng-templates with i18n', async () => {
591+
writeFile('/comp.ts', `
592+
import {Component} from '@angular/core';
593+
import {NgIf} from '@angular/common';
594+
595+
@Component({
596+
templateUrl: './comp.html'
597+
})
598+
class Comp {
599+
show = false;
600+
}
601+
`);
602+
603+
writeFile('/comp.html', [
604+
`<div>`,
605+
`<ng-template [ngIf]="show" [ngIfElse]="elseTmpl" i18n="@@something"><span>Content here</span></ng-template>`,
606+
`</div>`,
607+
`<ng-template #elseTmpl i18n="@@somethingElse"><p>other</p></ng-template>`,
608+
].join('\n'));
609+
610+
await runMigration();
611+
const content = tree.readContent('/comp.html');
612+
613+
expect(content).toBe([
614+
`<div>`,
615+
`@if (show) {`,
616+
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
617+
`} @else {`,
618+
`<ng-container i18n="@@somethingElse"><p>other</p></ng-container>`,
619+
`}`,
620+
`</div>\n`,
621+
].join('\n'));
622+
});
623+
486624
it('should migrate an if else case', async () => {
487625
writeFile('/comp.ts', `
488626
import {Component} from '@angular/core';

0 commit comments

Comments
 (0)