Skip to content

fix(migrations): correctly detect then/else keywords in control f… #69918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
surajy93 wants to merge 1 commit into
base: main
Choose a base branch
from
+128 −2
Open

fix(migrations): correctly detect then/else keywords in control f… #69918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions packages/core/schematics/ng-generate/control-flow-migration/ifs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,15 @@ export function migrateIf(template: string): {
}

function migrateNgIf(etm: ElementToMigrate, tmpl: string, offset: number): Result {
const matchThen = etm.attr.value.match(/[^\w\d];?\s*then/gm);
const matchElse = etm.attr.value.match(/[^\w\d];?\s*else/gm);
// The negative lookahead (?![\w$]) ensures `then`/`else` are matched only as
// whole keywords. Without it, an else/then template reference name that merely
// *starts* with `then`/`else` (e.g. `else thenBlock`) is misidentified as the
// `then` keyword, since `[^\w$];?\s*then` also matches the `then` prefix of
// `thenBlock`. `$` is included alongside `\w` (which already covers digits)
// since it is a valid identifier character in JS/template reference names
// (e.g. `#then$`), but is not part of `\w`.
const matchThen = etm.attr.value.match(/[^\w$];?\s*then(?![\w$])/gm);
const matchElse = etm.attr.value.match(/[^\w$];?\s*else(?![\w$])/gm);

if (etm.thenAttr !== undefined || etm.elseAttr !== undefined) {
// bound if then / if then else
Expand Down
119 changes: 119 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,125 @@ describe('control flow migration (ng update)', () => {
);
});

it('should migrate an if else case where the else template reference name starts with `then`', async () => {
writeFile(
'/comp.ts',
`
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';

@Component({
templateUrl: './comp.html'
})
class Comp {
data: any;
}
`,
);

writeFile(
'/comp.html',
[
`<ng-container *ngIf="data.teamMember; else thenBlock">`,
` <h2>Hello team member!</h2>`,
`</ng-container>`,
`<ng-template #thenBlock>`,
` <h2>No team member</h2>`,
`</ng-template>`,
].join('\n'),
);

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

expect(content).toBe(
[
`@if (data.teamMember) {`,
` <h2>Hello team member!</h2>`,
`} @else {`,
` <h2>No team member</h2>`,
`}\n`,
].join('\n'),
);
});

it('should migrate an if then case where the then template reference name starts with `else`', async () => {
writeFile(
'/comp.ts',
`
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';

@Component({
templateUrl: './comp.html'
})
class Comp {
data: any;
}
`,
);

writeFile(
'/comp.html',
[
`<ng-container *ngIf="data.teamMember; then elseyBlock"></ng-container>`,
`<ng-template #elseyBlock>`,
` <h2>Then content</h2>`,
`</ng-template>`,
].join('\n'),
);

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

expect(content).toBe(
[`@if (data.teamMember) {`, ` <h2>Then content</h2>`, `}\n`].join('\n'),
);
});

it('should migrate an if then else case where template reference names start with `then`/`else`', async () => {
writeFile(
'/comp.ts',
`
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';

@Component({
templateUrl: './comp.html'
})
class Comp {
data: any;
}
`,
);

writeFile(
'/comp.html',
[
`<ng-container *ngIf="data.teamMember; then thenyBlock; else elseyBlock"></ng-container>`,
`<ng-template #thenyBlock>`,
` <h2>Then content</h2>`,
`</ng-template>`,
`<ng-template #elseyBlock>`,
` <h2>Else content</h2>`,
`</ng-template>`,
].join('\n'),
);

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

expect(content).toBe(
[
`@if (data.teamMember) {`,
` <h2>Then content</h2>`,
`} @else {`,
` <h2>Else content</h2>`,
`}\n`,
].join('\n'),
);
});

it('should migrate an if case on a container', async () => {
writeFile(
'/comp.ts',
Expand Down
Loading