Elgato_dark/angular
1
0
Fork 0
mirror of https://github.com/angular/angular synced 2026-05-24 09:28:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

refactor(core): Improve the efficiency of the Typed Forms migration. (#45288)

Browse source 
Consider a file that imports `FormControl` and then never uses it. In that case, we don't need to add the import for `UntypedFormControl`.

By examining constructor calls *first*, we can identify these cases and skip over them.

This will reduce the memory footprint of the migration when run in tsunami, hopefully making OOM errors less likely.

PR Close #45288
This commit is contained in:
Dylan Hunn 2022-03-07 17:47:56 -08:00 committed by Andrew Kushnir
parent 6eaaefd22e
commit 6cec8aa728
2 changed files with 45 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
packages/core/schematics/migrations/typed-forms/util.ts
View file

@ -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[] {

26
packages/core/schematics/test/typed_forms_spec.ts
View file

@ -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));
});