diff --git a/packages/core/schematics/migrations/typed-forms/util.ts b/packages/core/schematics/migrations/typed-forms/util.ts index 5188a18e483..bf3d99777fd 100644 --- a/packages/core/schematics/migrations/typed-forms/util.ts +++ b/packages/core/schematics/migrations/typed-forms/util.ts @@ -29,6 +29,27 @@ export function migrateFile( // If no relevant classes are imported, we can exit early. if (imports.length === 0) return; + // For each control class, migrate all of its uses. + for (let i = imports.length; i >= 0; i--) { + const imp = imports[i]; + const usages = getUsages(sourceFile, typeChecker, imp); + if (usages.length === 0) { + // Since there are no usages of this class we need to migrate it, we should completely + // skip it for the subsequent migration steps. + imports.splice(i, 1); + } + for (const usage of usages) { + const newName = getUntypedVersionOfImportOrName(usage.importName); + if (newName === null) { + // This should never happen. + console.error( + `Typed forms migration error: unknown replacement for usage ${usage.node.getText()}`); + continue; + } + rewrite(usage.node.getStart(), usage.node.getWidth(), newName); + } + } + // For each imported control class, insert the corresponding uptyped import. for (const imp of imports) { const untypedClass = getUntypedVersionOfImportOrName(imp.getText()); @@ -45,21 +66,6 @@ export function migrateFile( } rewrite(imp.getEnd(), 0, `, ${untypedClass}`); } - - // For each control class, migrate all of its uses. - for (const imp of imports) { - const usages = getUsages(sourceFile, typeChecker, imp); - for (const usage of usages) { - const newName = getUntypedVersionOfImportOrName(usage.importName); - if (newName === null) { - // This should never happen. - console.error( - `Typed forms migration error: unknown replacement for usage ${usage.node.getText()}`); - continue; - } - rewrite(usage.node.getStart(), usage.node.getWidth(), newName); - } - } } function getImports(sourceFile: ts.SourceFile): ts.ImportSpecifier[] { diff --git a/packages/core/schematics/test/typed_forms_spec.ts b/packages/core/schematics/test/typed_forms_spec.ts index ca74a836cfe..4b9a2239c61 100644 --- a/packages/core/schematics/test/typed_forms_spec.ts +++ b/packages/core/schematics/test/typed_forms_spec.ts @@ -61,8 +61,8 @@ describe('Typed Forms migration', () => { shx.rm('-r', tmpDirPath); }); - describe('should rename', () => { - it('imports and constructor calls', async () => { + describe('should', () => { + it('rename imports and constructor calls', async () => { writeFile('/index.ts', ` import { Component } from '@angular/core'; import { AbstractControl, FormArray, FormBuilder, FormControl as FC, FormGroup, UntypedFormGroup } from '@angular/forms'; @@ -76,6 +76,8 @@ describe('Typed Forms migration', () => { private fb = new FormBuilder(); + private someSet = new Set([1]); + build() { const c = this.fb.control(42); const g = this.fb.group({one: this.fb.control('')}); @@ -98,7 +100,27 @@ describe('Typed Forms migration', () => { `const fc2 = new UntypedFormControl(0);`, // Except UntypedFormGroup, which is already migrated. `private _ungroup = new UntypedFormGroup({});`, + // Unrelated constructors should not be changed. + `private someSet = new Set([1]);`, + ]; + cases.forEach(t => expect(tree.readContent('/index.ts')).toContain(t)); + }); + it('skip adding imports that would be unused', async () => { + writeFile('/index.ts', ` + import { Component } from '@angular/core'; + import { FormControl, FormGroup } from '@angular/forms'; + + @Component({template: ''}) + export class MyComponent { + private _group!: FormGroup; + private _control = new FormControl(42); + } + `); + await runMigration(); + const cases = [ + // Because FormGroup is never directly constructed, the import should not be added. + `import { FormControl, UntypedFormControl, FormGroup } from '@angular/forms';`, ]; cases.forEach(t => expect(tree.readContent('/index.ts')).toContain(t)); });