Skip to content

Commit

Permalink
fix(migrations): account for variables in imports initializer (#55081)
Browse files Browse the repository at this point in the history
Fixes that the control flow migration was throwing an error if the `imports` of a component are initialized to an identifier.

Fixes #55080.

PR Close #55081
  • Loading branch information
crisbeto authored and dylhunn committed Mar 28, 2024
1 parent ee76001 commit 2f9d94b
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,16 @@ function updateImportClause(clause: ts.ImportClause, removeCommonModule: boolean
function updateClassImports(
propAssignment: ts.PropertyAssignment, removeCommonModule: boolean): string|null {
const printer = ts.createPrinter();
const importList = propAssignment.initializer as ts.ArrayLiteralExpression;
const importList = propAssignment.initializer;

// Can't change non-array literals.
if (!ts.isArrayLiteralExpression(importList)) {
return null;
}

const removals = removeCommonModule ? importWithCommonRemovals : importRemovals;
const elements = importList.elements.filter(el => !removals.includes(el.getText()));
const elements =
importList.elements.filter(el => !ts.isIdentifier(el) || !removals.includes(el.text));
if (elements.length === importList.elements.length) {
// nothing changed
return null;
Expand Down
65 changes: 65 additions & 0 deletions packages/core/schematics/test/control_flow_migration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5442,6 +5442,71 @@ describe('control flow migration', () => {

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

it('should not modify `imports` initialized to a variable reference', async () => {
writeFile('/comp.ts', [
`import {Component} from '@angular/core';`,
`import {CommonModule} from '@angular/common';\n`,
`const IMPORTS = [CommonModule];\n`,
`@Component({`,
` imports: IMPORTS,`,
` template: '<span *ngIf="show">Content here</span>',`,
`})`,
`class Comp {`,
` show = false;`,
`}`,
].join('\n'));

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

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

it('should handle spread elements in the `imports` array', async () => {
writeFile('/comp.ts', [
`import {Component} from '@angular/core';`,
`import {CommonModule} from '@angular/common';\n`,
`const BEFORE = [];\n`,
`const AFTER = [];\n`,
`@Component({`,
` imports: [...BEFORE, CommonModule, ...AFTER],`,
` template: '<span *ngIf="show">Content here</span>',`,
`})`,
`class Comp {`,
` show = false;`,
`}`,
].join('\n'));

await runMigration();
const actual = tree.readContent('/comp.ts');
const expected = [
`import {Component} from '@angular/core';\n\n`,
`const BEFORE = [];\n`,
`const AFTER = [];\n`,
`@Component({`,
` imports: [...BEFORE, ...AFTER],`,
` template: '@if (show) {<span>Content here</span>}',`,
`})`,
`class Comp {`,
` show = false;`,
`}`,
].join('\n');

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

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

0 comments on commit 2f9d94b

Please sign in to comment.