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

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

fixes: angular#52842
  • Loading branch information
@thePunderWoman
thePunderWoman committed Nov 13, 2023
1 parent 9e2c3bb commit 25e6f67
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 14 deletions.
50 changes: 39 additions & 11 deletions packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
View file
Expand Up @@ -13,6 +13,8 @@ import {calculateNesting, getMainBlock, getOriginals, hasLineBreaks, parseTempla

export const ngif = '*ngIf';
export const boundngif = '[ngIf]';
export const boundngifelse = '[ngIfElse]';
export const boundngifthenelse = '[ngIfThenElse]';
export const nakedngif = 'ngIf';

export const ifs = [
Expand Down Expand Up @@ -68,11 +70,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 +105,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 +149,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 @@ -9,6 +9,8 @@
import {Attribute, Element, RecursiveVisitor, Text} from '@angular/compiler';
import ts from 'typescript';

import {boundngifelse, boundngifthenelse} from './ifs';

export const ngtemplate = 'ng-template';

function allFormsOf(selector: string): string[] {
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
3 changes: 2 additions & 1 deletion packages/core/schematics/ng-generate/control-flow-migration/util.ts
View file
Expand Up @@ -360,7 +360,8 @@ 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;
Expand Down
69 changes: 69 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

0 comments on commit 25e6f67

Please sign in to comment.