Skip to content

Commit

Permalink
fix(migrations): CF Migration add support for ngIf with just a then (#…
Browse files Browse the repository at this point in the history
…53297)

Prior to this fix, the expectation that anytime then was used, else would always be present. That is not a valid assumption.

fixes: #53287

PR Close #53297
  • Loading branch information
jessicajaniuk authored and dylhunn committed Dec 1, 2023
1 parent 45064f1 commit 58a96e0
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 1 deletion.
Expand Up @@ -74,8 +74,12 @@ function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Resul
if (etm.thenAttr !== undefined || etm.elseAttr !== undefined) {
// bound if then / if then else
return buildBoundIfElseBlock(etm, tmpl, offset);
} else if (matchThen && matchThen.length > 0) {
} else if (matchThen && matchThen.length > 0 && matchElse && matchElse.length > 0) {
// then else
return buildStandardIfThenElseBlock(etm, tmpl, matchThen[0], matchElse![0], offset);
} else if (matchThen && matchThen.length > 0) {
// just then
return buildStandardIfThenBlock(etm, tmpl, matchThen[0], offset);
} else if ((matchElse && matchElse.length > 0)) {
// just else
return buildStandardIfElseBlock(etm, tmpl, matchElse![0], offset);
Expand Down Expand Up @@ -189,6 +193,17 @@ function buildStandardIfThenElseBlock(
return buildIfThenElseBlock(etm, tmpl, condition, thenPlaceholder, elsePlaceholder, offset);
}

function buildStandardIfThenBlock(
etm: ElementToMigrate, tmpl: string, thenString: string, offset: number): Result {
// includes the mandatory semicolon before as
const condition = etm.getCondition()
.replace(' as ', '; as ')
// replace 'let' with 'as' whatever spaces are between ; and 'let'
.replace(/;\s*let/g, '; as');
const thenPlaceholder = `#${etm.getTemplateName(thenString)}|`;
return buildIfThenBlock(etm, tmpl, condition, thenPlaceholder, offset);
}

function buildIfThenElseBlock(
etm: ElementToMigrate, tmpl: string, condition: string, thenPlaceholder: string,
elsePlaceholder: string, offset: number): Result {
Expand All @@ -214,3 +229,28 @@ function buildIfThenElseBlock(

return {tmpl: updatedTmpl, offsets: {pre, post}};
}

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

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

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

const postBlock = thenPlaceholder + `${lbString}}`;
const ifThenBlock = startBlock + postBlock;

const tmplStart = tmpl.slice(0, etm.start(offset));
const tmplEnd = tmpl.slice(etm.end(offset));

const updatedTmpl = tmplStart + ifThenBlock + tmplEnd;

// We ignore the contents of the element on if then else.
// If there's anything there, we need to account for the length in the offset.
const pre = originals.start.length + originals.childLength - startBlock.length;
const post = originals.end.length - postBlock.length;

return {tmpl: updatedTmpl, offsets: {pre, post}};
}
32 changes: 32 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Expand Up @@ -487,6 +487,38 @@ describe('control flow migration', () => {
].join('\n'));
});

it('should migrate an if case on an empty container', 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', [
`<ng-container`,
` *ngIf="true; then template"`,
`></ng-container>`,
`<ng-template #template>`,
` Hello!`,
`</ng-template> `,
].join('\n'));

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

expect(content).toBe([
`@if (true) {`,
` Hello!`,
`}\n`,
].join('\n'));
});

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

0 comments on commit 58a96e0

Please sign in to comment.