Skip to content

Commit

Permalink
fix(migrations): handle nested classes in control flow migration (#52309
Browse files Browse the repository at this point in the history
)

Fixes that the control flow migration was only processing top-level classes. Nested classes could come up during unit tests.

PR Close #52309
  • Loading branch information
crisbeto authored and dylhunn committed Oct 24, 2023
1 parent 54bc384 commit c9b1ddf
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 7 deletions.
Expand Up @@ -18,11 +18,7 @@ import {AnalyzedFile, boundngif, CaseCollector, ElementCollector, ElementToMigra
* @param analyzedFiles Map in which to store the results.
*/
export function analyze(sourceFile: ts.SourceFile, analyzedFiles: Map<string, AnalyzedFile>) {
for (const node of sourceFile.statements) {
if (!ts.isClassDeclaration(node)) {
continue;
}

forEachClass(sourceFile, node => {
// Note: we have a utility to resolve the Angular decorators from a class declaration already.
// We don't use it here, because it requires access to the type checker which makes it more
// time-consuming to run internally.
Expand All @@ -38,7 +34,7 @@ export function analyze(sourceFile: ts.SourceFile, analyzedFiles: Map<string, An
null;

if (!metadata) {
continue;
return;
}

for (const prop of metadata.properties) {
Expand All @@ -64,7 +60,7 @@ export function analyze(sourceFile: ts.SourceFile, analyzedFiles: Map<string, An
break;
}
}
}
});
}

/**
Expand Down Expand Up @@ -482,3 +478,13 @@ function getSwitchCaseBlock(
return `${lbString}@case (${etm.attr.value}) {${lbString}${lbSpaces}${
tmpl.slice(elStart, attrStart) + tmpl.slice(valEnd, elEnd)}${lbString}}`;
}

/** Executes a callback on each class declaration in a file. */
function forEachClass(sourceFile: ts.SourceFile, callback: (node: ts.ClassDeclaration) => void) {
sourceFile.forEachChild(function walk(node) {
if (ts.isClassDeclaration(node)) {
callback(node);
}
node.forEachChild(walk);
});
}
124 changes: 124 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Expand Up @@ -570,6 +570,51 @@ describe('control flow migration', () => {
`</div>`,
].join('\n'));
});

it('should migrate a nested class', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';
function foo() {
@Component({
imports: [NgIf],
template: \`<div><span *ngIf="toggle">This should be hidden</span></div>\`
})
class Comp {
toggle = false;
}
}
`);

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

expect(content).toContain(
'template: `<div>@if (toggle) {<span>This should be hidden</span>}</div>`');
});

it('should migrate a nested class', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgIf} from '@angular/common';
function foo() {
@Component({
imports: [NgIf],
template: \`<div><span *ngIf="toggle">This should be hidden</span></div>\`
})
class Comp {
toggle = false;
}
}
`);

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

expect(content).toContain(
'template: `<div>@if (toggle) {<span>This should be hidden</span>}</div>`');
});
});

describe('ngFor', () => {
Expand Down Expand Up @@ -870,6 +915,59 @@ describe('control flow migration', () => {
expect(content).toContain(
'template: `@for (item of items; track item) {<ng-container [bindMe]="stuff"><p>{{item.text}}</p></ng-container>}`');
});

it('should migrate a nested class', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgFor} from '@angular/common';
interface Item {
id: number;
text: string;
}
function foo() {
@Component({
imports: [NgFor],
template: \`<ul><li *ngFor="let item of items">{{item.text}}</li></ul>\`
})
class Comp {
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
}
}
`);

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

expect(content).toContain(
'template: `<ul>@for (item of items; track item) {<li>{{item.text}}</li>}</ul>`');
});

it('should migrate a nested class', async () => {
writeFile('/comp.ts', `
import {Component} from '@angular/core';
import {NgFor} from '@angular/common';
interface Item {
id: number;
text: string;
}
function foo() {
@Component({
imports: [NgFor],
template: \`<ul><li *ngFor="let item of items">{{item.text}}</li></ul>\`
})
class Comp {
items: Item[] = [{id: 1, text: 'blah'},{id: 2, text: 'stuff'}];
}
}
`);

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

expect(content).toContain(
'template: `<ul>@for (item of items; track item) {<li>{{item.text}}</li>}</ul>`');
});
});

describe('ngSwitch', () => {
Expand Down Expand Up @@ -1131,6 +1229,32 @@ describe('control flow migration', () => {
expect(content).toContain(
'template: `<div>@switch (testOpts) { @case (1) { <p>Option 1</p> } @case (2) { <p>Option 2</p> } @default { <p>Option 3</p> }}</div>');
});

it('should migrate a nested class', async () => {
writeFile(
'/comp.ts',
`
import {Component} from '@angular/core';
import {ngSwitch, ngSwitchCase} from '@angular/common';
function foo() {
@Component({
template: \`<div [ngSwitch]="testOpts">` +
`<p *ngSwitchCase="1">Option 1</p>` +
`<p *ngSwitchCase="2">Option 2</p>` +
`</div>\`
})
class Comp {
testOpts = "1";
}
}
`);

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

expect(content).toContain(
'template: `<div>@switch (testOpts) { @case (1) { <p>Option 1</p> } @case (2) { <p>Option 2</p> }}</div>`');
});
});

describe('nested structures', () => {
Expand Down

0 comments on commit c9b1ddf

Please sign in to comment.