Skip to content

Commit

Permalink
fix(migrations): account for let declarations in control flow migrati…
Browse files Browse the repository at this point in the history
…on (#59861)

Fixes that the control flow migration wasn't accounting for `@let` when determining which symbols are used.

PR Close #59861
  • Loading branch information
crisbeto authored and atscott committed Feb 12, 2025
1 parent 1119f85 commit aa285c5
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Attribute,
Block,
Element,
LetDeclaration,
ParseTreeResult,
RecursiveVisitor,
Text,
Expand Down Expand Up @@ -399,6 +400,13 @@ export class CommonCollector extends RecursiveVisitor {
}
}

override visitLetDeclaration(decl: LetDeclaration): void {
if (this.hasPipes(decl.value)) {
this.count++;
}
super.visitLetDeclaration(decl, null);
}

private hasDirectives(input: string): boolean {
return commonModuleDirectives.has(input);
}
Expand Down
33 changes: 33 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6512,6 +6512,39 @@ describe('control flow migration', () => {

expect(actual).toBe(expected);
});

it('should not remove common module if symbols are used inside @let', async () => {
writeFile(
'/comp.ts',
[
`import {CommonModule} from '@angular/common';`,
`import {Component} from '@angular/core';\n`,
`@Component({`,
` imports: [CommonModule],`,
` template: \`@let foo = 123 | date; <span *ngIf="foo">{{foo}}</span>\``,
`})`,
`class Comp {`,
` toggle = false;`,
`}`,
].join('\n'),
);

await runMigration();
const actual = tree.readContent('/comp.ts');
const expected = [
`import {CommonModule} from '@angular/common';`,
`import {Component} from '@angular/core';\n`,
`@Component({`,
` imports: [CommonModule],`,
` template: \`@let foo = 123 | date; @if (foo) {<span>{{foo}}</span>}\``,
`})`,
`class Comp {`,
` toggle = false;`,
`}`,
].join('\n');

expect(actual).toBe(expected);
});
});

describe('no migration needed', () => {
Expand Down

0 comments on commit aa285c5

Please sign in to comment.