Skip to content

Commit

Permalink
fix(migrations): Add support for bound versions of NgIfElse and NgIfT…
Browse files Browse the repository at this point in the history 
…henElse (#52869)

This ensures the bound version of NgIfElse and NgIfThenElse work properly with the migration.

fixes: #52842

PR Close #52869
  • Loading branch information
@jessicajaniuk
jessicajaniuk committed Nov 13, 2023
1 parent 8b6fd8d commit d033540
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 15 deletions.
48 changes: 37 additions & 11 deletions packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
View file
Expand Up @@ -68,11 +68,14 @@ function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Resul
const matchThen = etm.attr.value.match(/;\s*then/gm);
const matchElse = etm.attr.value.match(/;\s*else/gm);

if (matchThen && matchThen.length > 0) {
return buildIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
} else if (matchElse && matchElse.length > 0) {
if (etm.thenAttr !== undefined || etm.elseAttr !== undefined) {
// bound if then / if then else
return buildBoundIfElseBlock(etm, tmpl, offset);
} else if (matchThen && matchThen.length > 0) {
return buildStandardIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
} else if ((matchElse && matchElse.length > 0)) {
// just else
return buildIfElseBlock(etm, tmpl, matchElse[0], offset);
return buildStandardIfElseBlock(etm, tmpl, matchElse![0], offset);
}

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

function buildIfElseBlock(
function buildStandardIfElseBlock(
etm: ElementToMigrate, tmpl: string, elseString: string, offset: number): Result {
// includes the mandatory semicolon before as
const lbString = etm.hasLineBreaks ? '\n' : '';
const condition = etm.getCondition(elseString).replace(' as ', '; as ');
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
}

function buildBoundIfElseBlock(etm: ElementToMigrate, tmpl: string, offset: number): Result {
// includes the mandatory semicolon before as
const condition = etm.attr.value.replace(' as ', '; as ');
const elsePlaceholder = `#${etm.elseAttr!.value}|`;
if (etm.thenAttr !== undefined) {
const thenPlaceholder = `#${etm.thenAttr!.value}|`;
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
}
return buildIfElseBlock(etm, tmpl, condition, elsePlaceholder, offset);
}

function buildIfElseBlock(
etm: ElementToMigrate, tmpl: string, condition: string, elsePlaceholder: string,
offset: number): Result {
const lbString = etm.hasLineBreaks ? '\n' : '';

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

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

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

function buildIfThenElseBlock(
function buildStandardIfThenElseBlock(
etm: ElementToMigrate, tmpl: string, thenString: string, elseString: string,
offset: number): Result {
// includes the mandatory semicolon before as
const condition = etm.getCondition(thenString).replace(' as ', '; as ');
const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
}

function buildIfThenElseBlock(
etm: ElementToMigrate, tmpl: string, condition: string, thenPlaceholder: string,
elsePlaceholder: string, offset: number): Result {
const lbString = etm.hasLineBreaks ? '\n' : '';

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

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

const thenPlaceholder = `#${etm.getTemplateName(thenString, elseString)}|`;
const elsePlaceholder = `#${etm.getTemplateName(elseString)}|`;

const postBlock = thenPlaceholder + elseBlock + elsePlaceholder + `${lbString}}`;
const ifThenElseBlock = startBlock + postBlock;

Expand Down
14 changes: 12 additions & 2 deletions packages/core/schematics/ng-generate/control-flow-migration/types.ts
View file
Expand Up @@ -10,6 +10,8 @@ import {Attribute, Element, RecursiveVisitor, Text} from '@angular/compiler';
import ts from 'typescript';

export const ngtemplate = 'ng-template';
export const boundngifelse = '[ngIfElse]';
export const boundngifthenelse = '[ngIfThenElse]';

function allFormsOf(selector: string): string[] {
return [
Expand Down Expand Up @@ -84,12 +86,18 @@ export type MigrateError = {
export class ElementToMigrate {
el: Element;
attr: Attribute;
elseAttr: Attribute|undefined;
thenAttr: Attribute|undefined;
nestCount = 0;
hasLineBreaks = false;

constructor(el: Element, attr: Attribute) {
constructor(
el: Element, attr: Attribute, elseAttr: Attribute|undefined = undefined,
thenAttr: Attribute|undefined = undefined) {
this.el = el;
this.attr = attr;
this.elseAttr = elseAttr;
this.thenAttr = thenAttr;
}

getCondition(targetStr: string): string {
Expand Down Expand Up @@ -226,7 +234,9 @@ export class ElementCollector extends RecursiveVisitor {
if (el.attrs.length > 0) {
for (const attr of el.attrs) {
if (this._attributes.includes(attr.name)) {
this.elements.push(new ElementToMigrate(el, attr));
const elseAttr = el.attrs.find(x => x.name === boundngifelse);
const thenAttr = el.attrs.find(x => x.name === boundngifthenelse);
this.elements.push(new ElementToMigrate(el, attr, elseAttr, thenAttr));
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file
Expand Up @@ -360,13 +360,16 @@ export function getMainBlock(etm: ElementToMigrate, tmpl: string, offset: number
{start: string, middle: string, end: string} {
const i18nAttr = etm.el.attrs.find(x => x.name === 'i18n');
if ((etm.el.name === 'ng-container' || etm.el.name === 'ng-template') &&
etm.el.attrs.length === 1) {
(etm.el.attrs.length === 1 || (etm.el.attrs.length === 2 && etm.elseAttr !== undefined) ||
(etm.el.attrs.length === 3 && etm.elseAttr !== undefined && etm.thenAttr !== undefined))) {
// this is the case where we're migrating and there's no need to keep the ng-container
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
const middle = tmpl.slice(childStart, childEnd);
return {start: '', middle, end: ''};
} else if (etm.el.name === 'ng-template' && etm.el.attrs.length === 2 && i18nAttr !== undefined) {
} else if (
etm.el.name === 'ng-template' && i18nAttr !== undefined &&
(etm.el.attrs.length === 2 || (etm.el.attrs.length === 3 && etm.elseAttr !== undefined))) {
const childStart = etm.el.children[0].sourceSpan.start.offset - offset;
const childEnd = etm.el.children[etm.el.children.length - 1].sourceSpan.end.offset - offset;
const middle = wrapIntoI18nContainer(i18nAttr, tmpl.slice(childStart, childEnd));
Expand Down
138 changes: 138 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
View file
Expand Up @@ -328,6 +328,75 @@ describe('control flow migration', () => {
].join('\n'));
});

it('should migrate an if else case as bindings', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf,NgIfElse} from '@angular/common';
@Component({
templateUrl: './comp.html'
})
class Comp {
show = false;
}
`);

writeFile('/comp.html', [
`<div>`,
`<ng-template [ngIf]="show" [ngIfElse]="tmplName"><span>Content here</span></ng-template>`,
`</div>`,
`<ng-template #tmplName><p>Stuff</p></ng-template>`,
].join('\n'));

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

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

it('should migrate an if then else case as bindings', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf,NgIfElse} from '@angular/common';
@Component({
templateUrl: './comp.html'
})
class Comp {
show = false;
}
`);

writeFile('/comp.html', [
`<div>`,
`<ng-template [ngIf]="show" [ngIfElse]="elseTmpl" [ngIfThenElse]="thenTmpl">Ignore Me</ng-template>`,
`</div>`,
`<ng-template #elseTmpl><p>Stuff</p></ng-template>`,
`<ng-template #thenTmpl><span>Content here</span></ng-template>`,
].join('\n'));

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

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

it('should migrate an if case on a container', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
Expand Down Expand Up @@ -483,6 +552,75 @@ describe('control flow migration', () => {
].join('\n'));
});

it('should migrate a bound NgIfThenElse case with ng-templates with i18n', 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>`,
`<ng-template [ngIf]="show" [ngIfThenElse]="thenTmpl" [ngIfElse]="elseTmpl">ignored</ng-template>`,
`</div>`,
`<ng-template #thenTmpl i18n="@@something"><span>Content here</span></ng-template>`,
`<ng-template #elseTmpl i18n="@@somethingElse"><p>other</p></ng-template>`,
].join('\n'));

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

expect(content).toBe([
`<div>`,
`@if (show) {`,
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
`} @else {`,
`<ng-container i18n="@@somethingElse"><p>other</p></ng-container>`,
`}`,
`</div>\n`,
].join('\n'));
});

it('should migrate a bound NgIfElse case with ng-templates with i18n', 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>`,
`<ng-template [ngIf]="show" [ngIfElse]="elseTmpl" i18n="@@something"><span>Content here</span></ng-template>`,
`</div>`,
`<ng-template #elseTmpl i18n="@@somethingElse"><p>other</p></ng-template>`,
].join('\n'));

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

expect(content).toBe([
`<div>`,
`@if (show) {`,
`<ng-container i18n="@@something"><span>Content here</span></ng-container>`,
`} @else {`,
`<ng-container i18n="@@somethingElse"><p>other</p></ng-container>`,
`}`,
`</div>\n`,
].join('\n'));
});

it('should migrate an if else case', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
Expand Down

0 comments on commit d033540

Please sign in to comment.