From 63688714aeb788a24b030f9f9cdcc55e7fb0d758 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 28 Mar 2024 09:01:11 +0100 Subject: [PATCH] fix(migrations): account for variables in imports initializer (#55081) 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 --- .../control-flow-migration/util.ts | 11 +++- .../test/control_flow_migration_spec.ts | 65 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/packages/core/schematics/ng-generate/control-flow-migration/util.ts b/packages/core/schematics/ng-generate/control-flow-migration/util.ts index b5ebbb9a3ca67..20ace8e87667f 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -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; diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 0bcb141741d59..c475a649b1dfb 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -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: 'Content here',`, + `})`, + `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) {Content here}',`, + `})`, + `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: 'Content here',`, + `})`, + `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) {Content here}',`, + `})`, + `class Comp {`, + ` show = false;`, + `}`, + ].join('\n'); + + expect(actual).toBe(expected); + }); }); describe('no migration needed', () => {