diff --git a/packages/core/schematics/ng-generate/control-flow-migration/identifier-lookup.ts b/packages/core/schematics/ng-generate/control-flow-migration/identifier-lookup.ts index b3f84bd2af9..89602be4f78 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/identifier-lookup.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/identifier-lookup.ts @@ -9,10 +9,10 @@ import ts from 'typescript'; export function lookupIdentifiersInSourceFile( - sourceFile: ts.SourceFile, name: string): Set { + sourceFile: ts.SourceFile, names: string[]): Set { const results = new Set(); const visit = (node: ts.Node): void => { - if (ts.isIdentifier(node) && node.text === name) { + if (ts.isIdentifier(node) && names.includes(node.text)) { results.add(node); } ts.forEachChild(node, visit); diff --git a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts index 1d2d1790a8d..15636b62218 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/migration.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/migration.ts @@ -82,6 +82,7 @@ export function migrateTemplate( // and in that case we can't safely remove the common module import. componentFile.verifyCanRemoveImports(); } + file.verifyCanRemoveImports(); errors = [ ...ifResult.errors, diff --git a/packages/core/schematics/ng-generate/control-flow-migration/types.ts b/packages/core/schematics/ng-generate/control-flow-migration/types.ts index d7ab008e234..658abf0d5f9 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/types.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/types.ts @@ -23,6 +23,13 @@ export const endMarker = '✢'; export const startI18nMarker = '⚈'; export const endI18nMarker = '⚉'; +export const importRemovals = [ + 'NgIf', 'NgIfElse', 'NgIfThenElse', 'NgFor', 'NgForOf', 'NgForTrackBy', 'NgSwitch', + 'NgSwitchCase', 'NgSwitchDefault' +]; + +export const importWithCommonRemovals = [...importRemovals, 'CommonModule']; + function allFormsOf(selector: string): string[] { return [ selector, @@ -308,7 +315,7 @@ export class AnalyzedFile { // skip this check entirely if (this.removeCommonModule) { const importDeclaration = this.importRanges.find(r => r.type === 'importDeclaration'); - const instances = lookupIdentifiersInSourceFile(this.sourceFile, 'CommonModule'); + const instances = lookupIdentifiersInSourceFile(this.sourceFile, importWithCommonRemovals); let foundImportDeclaration = false; let count = 0; for (let range of this.importRanges) { 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 71550111bb1..d5ee9717bc7 100644 --- a/packages/core/schematics/ng-generate/control-flow-migration/util.ts +++ b/packages/core/schematics/ng-generate/control-flow-migration/util.ts @@ -10,13 +10,8 @@ import {Attribute, Element, HtmlParser, Node, ParseTreeResult, visitAll} from '@ import {dirname, join} from 'path'; import ts from 'typescript'; -import {AnalyzedFile, CommonCollector, ElementCollector, ElementToMigrate, endI18nMarker, endMarker, i18nCollector, ParseResult, startI18nMarker, startMarker, Template, TemplateCollector} from './types'; +import {AnalyzedFile, CommonCollector, ElementCollector, ElementToMigrate, endI18nMarker, endMarker, i18nCollector, importRemovals, importWithCommonRemovals, ParseResult, startI18nMarker, startMarker, Template, TemplateCollector} from './types'; -const importRemovals = [ - 'NgIf', 'NgIfElse', 'NgIfThenElse', 'NgFor', 'NgForOf', 'NgForTrackBy', 'NgSwitch', - 'NgSwitchCase', 'NgSwitchDefault' -]; -const importWithCommonRemovals = [...importRemovals, 'CommonModule']; const startMarkerRegex = new RegExp(startMarker, 'gm'); const endMarkerRegex = new RegExp(endMarker, 'gm'); const startI18nMarkerRegex = new RegExp(startI18nMarker, 'gm'); diff --git a/packages/core/schematics/test/control_flow_migration_spec.ts b/packages/core/schematics/test/control_flow_migration_spec.ts index 069b8a224d4..e55a691d9bb 100644 --- a/packages/core/schematics/test/control_flow_migration_spec.ts +++ b/packages/core/schematics/test/control_flow_migration_spec.ts @@ -5087,6 +5087,68 @@ describe('control flow migration', () => { expect(actual).toBe(expected); }); + + it('should not remove imports when mismatch in counts', async () => { + writeFile('/comp.ts', [ + `import {CommonModule} from '@angular/common';`, + `import {Component, NgModule, Pipe, PipeTransform} from '@angular/core';`, + `@Component({`, + ` selector: 'description',`, + ` template: \`{{getDescription()}}\`,`, + `})`, + `export class DescriptionController {`, + ` getDescription(): string {`, + ` return 'stuff';`, + ` }`, + `}`, + ``, + `@Pipe({name: 'description'})`, + `export class DescriptionPipe implements PipeTransform {`, + ` transform(nameString?: string): string {`, + ` return nameString ?? '';`, + ` }`, + `}`, + `@NgModule({`, + ` declarations: [DescriptionController, DescriptionPipe],`, + ` imports: [CommonModule],`, + ` providers: [],`, + ` exports: [DescriptionController, DescriptionPipe],`, + `})`, + `export class DescriptionModule {}`, + ].join('\n')); + + await runMigration(); + const actual = tree.readContent('/comp.ts'); + const expected = [ + `import {CommonModule} from '@angular/common';`, + `import {Component, NgModule, Pipe, PipeTransform} from '@angular/core';`, + `@Component({`, + ` selector: 'description',`, + ` template: \`{{getDescription()}}\`,`, + `})`, + `export class DescriptionController {`, + ` getDescription(): string {`, + ` return 'stuff';`, + ` }`, + `}`, + ``, + `@Pipe({name: 'description'})`, + `export class DescriptionPipe implements PipeTransform {`, + ` transform(nameString?: string): string {`, + ` return nameString ?? '';`, + ` }`, + `}`, + `@NgModule({`, + ` declarations: [DescriptionController, DescriptionPipe],`, + ` imports: [CommonModule],`, + ` providers: [],`, + ` exports: [DescriptionController, DescriptionPipe],`, + `})`, + `export class DescriptionModule {}`, + ].join('\n'); + + expect(actual).toBe(expected); + }); }); describe('no migration needed', () => {