Skip to content

Commit c993e9a

Browse files
crisbetodylhunn
authored andcommitted
fix(migrations): handle nested classes in control flow migration (angular#52309)
Fixes that the control flow migration was only processing top-level classes. Nested classes could come up during unit tests. PR Close angular#52309
1 parent 9e76468 commit c993e9a

File tree

2 files changed

+137
-7
lines changed

2 files changed

+137
-7
lines changed

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,7 @@ import {AnalyzedFile, boundngif, CaseCollector, ElementCollector, ElementToMigra
1818
* @param analyzedFiles Map in which to store the results.
1919
*/
2020
export function analyze(sourceFile: ts.SourceFile, analyzedFiles: Map<string, AnalyzedFile>) {
21-
for (const node of sourceFile.statements) {
22-
if (!ts.isClassDeclaration(node)) {
23-
continue;
24-
}
25-
21+
forEachClass(sourceFile, node => {
2622
// Note: we have a utility to resolve the Angular decorators from a class declaration already.
2723
// We don't use it here, because it requires access to the type checker which makes it more
2824
// time-consuming to run internally.
@@ -38,7 +34,7 @@ export function analyze(sourceFile: ts.SourceFile, analyzedFiles: Map<string, An
3834
null;
3935

4036
if (!metadata) {
41-
continue;
37+
return;
4238
}
4339

4440
for (const prop of metadata.properties) {
@@ -64,7 +60,7 @@ export function analyze(sourceFile: ts.SourceFile, analyzedFiles: Map<string, An
6460
break;
6561
}
6662
}
67-
}
63+
});
6864
}
6965

7066
/**
@@ -482,3 +478,13 @@ function getSwitchCaseBlock(
482478
return `${lbString}@case (${etm.attr.value}) {${lbString}${lbSpaces}${
483479
tmpl.slice(elStart, attrStart) + tmpl.slice(valEnd, elEnd)}${lbString}}`;
484480
}
481+
482+
/** Executes a callback on each class declaration in a file. */
483+
function forEachClass(sourceFile: ts.SourceFile, callback: (node: ts.ClassDeclaration) => void) {
484+
sourceFile.forEachChild(function walk(node) {
485+
if (ts.isClassDeclaration(node)) {
486+
callback(node);
487+
}
488+
node.forEachChild(walk);
489+
});
490+
}

packages/core/schematics/test/control_flow_migration_spec.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,51 @@ describe('control flow migration', () => {
570570
`</div>`,
571571
].join('\n'));
572572
});
573+
574+
it('should migrate a nested class', async () => {
575+
writeFile('/comp.ts', `
576+
import {Component} from '@angular/core';
577+
import {NgIf} from '@angular/common';
578+
579+
function foo() {
580+
@Component({
581+
imports: [NgIf],
582+
template: \`<div><span *ngIf="toggle">This should be hidden</span></div>\`
583+
})
584+
class Comp {
585+
toggle = false;
586+
}
587+
}
588+
`);
589+
590+
await runMigration();
591+
const content = tree.readContent('/comp.ts');
592+
593+
expect(content).toContain(
594+
'template: `<div>@if (toggle) {<span>This should be hidden</span>}</div>`');
595+
});
596+
597+
it('should migrate a nested class', async () => {
598+
writeFile('/comp.ts', `
599+
import {Component} from '@angular/core';
600+
import {NgIf} from '@angular/common';
601+
function foo() {
602+
@Component({
603+
imports: [NgIf],
604+
template: \`<div><span *ngIf="toggle">This should be hidden</span></div>\`
605+
})
606+
class Comp {
607+
toggle = false;
608+
}
609+
}
610+
`);
611+
612+
await runMigration();
613+
const content = tree.readContent('/comp.ts');
614+
615+
expect(content).toContain(
616+
'template: `<div>@if (toggle) {<span>This should be hidden</span>}</div>`');
617+
});
573618
});
574619

575620
describe('ngFor', () => {
@@ -870,6 +915,59 @@ describe('control flow migration', () => {
870915
expect(content).toContain(
871916
'template: `@for (item of items; track item) {<ng-container [bindMe]="stuff"><p>{{item.text}}</p></ng-container>}`');
872917
});
918+
919+
it('should migrate a nested class', async () => {
920+
writeFile('/comp.ts', `
921+
import {Component} from '@angular/core';
922+
import {NgFor} from '@angular/common';
923+
interface Item {
924+
id: number;
925+
text: string;
926+
}
927+
928+
function foo() {
929+
@Component({
930+
imports: [NgFor],
931+
template: \`<ul><li *ngFor="let item of items">{{item.text}}</li></ul>\`
932+
})
933+
class Comp {
934+
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
935+
}
936+
}
937+
`);
938+
939+
await runMigration();
940+
const content = tree.readContent('/comp.ts');
941+
942+
expect(content).toContain(
943+
'template: `<ul>@for (item of items; track item) {<li>{{item.text}}</li>}</ul>`');
944+
});
945+
946+
it('should migrate a nested class', async () => {
947+
writeFile('/comp.ts', `
948+
import {Component} from '@angular/core';
949+
import {NgFor} from '@angular/common';
950+
interface Item {
951+
id: number;
952+
text: string;
953+
}
954+
function foo() {
955+
@Component({
956+
imports: [NgFor],
957+
template: \`<ul><li *ngFor="let item of items">{{item.text}}</li></ul>\`
958+
})
959+
class Comp {
960+
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
961+
}
962+
}
963+
`);
964+
965+
await runMigration();
966+
const content = tree.readContent('/comp.ts');
967+
968+
expect(content).toContain(
969+
'template: `<ul>@for (item of items; track item) {<li>{{item.text}}</li>}</ul>`');
970+
});
873971
});
874972

875973
describe('ngSwitch', () => {
@@ -1131,6 +1229,32 @@ describe('control flow migration', () => {
11311229
expect(content).toContain(
11321230
'template: `<div>@switch (testOpts) { @case (1) { <p>Option 1</p> } @case (2) { <p>Option 2</p> } @default { <p>Option 3</p> }}</div>');
11331231
});
1232+
1233+
it('should migrate a nested class', async () => {
1234+
writeFile(
1235+
'/comp.ts',
1236+
`
1237+
import {Component} from '@angular/core';
1238+
import {ngSwitch, ngSwitchCase} from '@angular/common';
1239+
function foo() {
1240+
@Component({
1241+
template: \`<div [ngSwitch]="testOpts">` +
1242+
`<p *ngSwitchCase="1">Option 1</p>` +
1243+
`<p *ngSwitchCase="2">Option 2</p>` +
1244+
`</div>\`
1245+
})
1246+
class Comp {
1247+
testOpts = "1";
1248+
}
1249+
}
1250+
`);
1251+
1252+
await runMigration();
1253+
const content = tree.readContent('/comp.ts');
1254+
1255+
expect(content).toContain(
1256+
'template: `<div>@switch (testOpts) { @case (1) { <p>Option 1</p> } @case (2) { <p>Option 2</p> }}</div>`');
1257+
});
11341258
});
11351259

11361260
describe('nested structures', () => {

0 commit comments

Comments
 (0)