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

refactor(migrations): use import manager in signal input migration (#57318)

Browse source 
Instead of fiddling manually with the imports, which worked well, but
comes at a cost of complexity— we are now using the canonical import
manager. This simplifies deletion, insertion and updating of imports.

Notably, our import manager is not super great at preserving whitespaces
right now, but we assume a formatter runs over migrated code anyway.

PR Close #57318
This commit is contained in:
Paul Gschwendtner 2024-08-09 13:13:34 +00:00 committed by Andrew Kushnir
parent 62ec4675be
commit fc4c9baf66
8 changed files with 142 additions and 159 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
packages/core/schematics/migrations/signal-migration/src/convert-input/convert_to_signal.ts
View file

@ -12,6 +12,7 @@ import ts from 'typescript';
import {MigrationHost} from '../migration_host';
import {ConvertInputPreparation} from './prepare_and_check';
import {DecoratorInputTransform} from '../../../../../../compiler-cli/src/ngtsc/metadata';
import {ImportManager} from '../../../../../../compiler-cli/src/ngtsc/translator';
const printer = ts.createPrinter({newLine: ts.NewLineKind.LineFeed});
@ -26,6 +27,7 @@ export function convertToSignalInput(
node: ts.PropertyDeclaration,
{resolvedMetadata: metadata, resolvedType, isResolvedTypeCheckable}: ConvertInputPreparation,
checker: ts.TypeChecker,
importManager: ImportManager,
): string {
let initialValue = node.initializer;
let optionsLiteral: ts.ObjectLiteralExpression | null = null;
@ -89,11 +91,11 @@ export function convertToSignalInput(
inputArgs.push(optionsLiteral);
}
const inputFnRef =
metadata.inputDecoratorRef !== null && ts.isPropertyAccessExpression(metadata.inputDecoratorRef)
? ts.factory.createPropertyAccessExpression(metadata.inputDecoratorRef.expression, 'input')
: ts.factory.createIdentifier('input');
const inputFnRef = importManager.addImport({
exportModuleSpecifier: '@angular/core',
exportSymbolName: 'input',
requestedFile: node.getSourceFile(),
});
const inputInitializerFn = metadata.required
? ts.factory.createPropertyAccessExpression(inputFnRef, 'required')
: inputFnRef;

3
packages/core/schematics/migrations/signal-migration/src/input_detection/input_decorator.ts
View file

@ -34,7 +34,6 @@ import {InputNode, isInputContainerNode} from '../input_detection/input_node';
/** Metadata extracted of an input declaration (in `.ts` or `.d.ts` files). */
export interface ExtractedInput extends InputMapping {
inSourceFile: boolean;
inputDecoratorRef: DecoratorIdentifier | null;
}
/** Attempts to extract metadata of a potential TypeScript `@Input()` declaration. */
@ -87,7 +86,6 @@ function extractDtsInput(node: ts.Node, metadataReader: DtsMetadataReader): Extr
? null
: {
...inputMapping,
inputDecoratorRef: null,
inSourceFile: false,
};
}
@ -152,7 +150,6 @@ function extractSourceCodeInput(
isSignal: false,
inSourceFile: true,
transform: transformResult,
inputDecoratorRef: inputDecorator.identifier,
};
}

59
packages/core/schematics/migrations/signal-migration/src/passes/10_apply_import_manager.ts Normal file
View file

@ -0,0 +1,59 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import ts from 'typescript';
import {ImportManager} from '../../../../../../compiler-cli/src/ngtsc/translator';
import {Replacement} from '../replacement';
import {MigrationResult} from '../result';
/**
* Phase that applies all changes recorded by the import manager in
* previous migrate phases.
*/
export function pass10_applyImportManager(
importManager: ImportManager,
result: MigrationResult,
sourceFiles: readonly ts.SourceFile[],
) {
const {newImports, updatedImports, deletedImports} = importManager.finalize();
const printer = ts.createPrinter({});
const pathToFile = new Map<string, ts.SourceFile>(sourceFiles.map((s) => [s.fileName, s]));
// Capture new imports
newImports.forEach((newImports, fileName) => {
newImports.forEach((newImport) => {
const printedImport = printer.printNode(
ts.EmitHint.Unspecified,
newImport,
pathToFile.get(fileName)!,
);
result.addReplacement(fileName, new Replacement(0, 0, `${printedImport}\n`));
});
});
// Capture updated imports
for (const [oldBindings, newBindings] of updatedImports.entries()) {
const printedBindings = printer.printNode(
ts.EmitHint.Unspecified,
newBindings,
oldBindings.getSourceFile(),
);
result.addReplacement(
oldBindings.getSourceFile().fileName,
new Replacement(oldBindings.getStart(), oldBindings.getEnd(), printedBindings),
);
}
// Update removed imports
for (const removedImport of deletedImports) {
result.addReplacement(
removedImport.getSourceFile().fileName,
new Replacement(removedImport.getStart(), removedImport.getEnd(), ''),
);
}
}

60
packages/core/schematics/migrations/signal-migration/src/passes/6_migrate_input_declarations.ts
View file

@ -13,6 +13,7 @@ import {convertToSignalInput} from '../convert-input/convert_to_signal';
import assert from 'assert';
import {MigrationHost} from '../migration_host';
import {KnownInputs} from '../input_detection/known_inputs';
import {ImportManager} from '../../../../../../compiler-cli/src/ngtsc/translator';
/**
* Phase that migrates `@Input()` declarations to signal inputs and
@ -23,6 +24,7 @@ export function pass6__migrateInputDeclarations(
checker: ts.TypeChecker,
result: MigrationResult,
knownInputs: KnownInputs,
importManager: ImportManager,
) {
let filesWithMigratedInputs = new Set<ts.SourceFile>();
let filesWithIncompatibleInputs = new WeakSet<ts.SourceFile>();
@ -44,65 +46,15 @@ export function pass6__migrateInputDeclarations(
new Replacement(
input.node.getStart(),
input.node.getEnd(),
convertToSignalInput(host, input.node, metadata, checker),
convertToSignalInput(host, input.node, metadata, checker, importManager),
),
);
}
// TODO: Consider using import manager for conflicts. but also in `convert_to_signal` then.
for (const file of filesWithMigratedInputs) {
const specifiers = result.inputDecoratorSpecifiers.get(file);
assert(specifiers !== undefined, `No imports in source file of input: ${file.fileName}`);
const decoratorInputSpecifier = specifiers.find((s) => s.kind === 'decorator-input-import');
assert(decoratorInputSpecifier, 'Expected at least one import to "Input"');
const signalInputSpecifier = specifiers.find((s) => s.kind === 'signal-input-import');
if (filesWithIncompatibleInputs.has(file)) {
// add `input`, but not remove `Input`. Both are needed still.
if (signalInputSpecifier === undefined) {
result.addReplacement(
file.fileName,
new Replacement(
decoratorInputSpecifier.node.getEnd(),
decoratorInputSpecifier.node.getEnd(),
', input',
),
);
}
} else {
// replace `Input` with `input`. All migrated in this file.
if (signalInputSpecifier === undefined) {
result.addReplacement(
file.fileName,
new Replacement(
decoratorInputSpecifier.node.getStart(),
decoratorInputSpecifier.node.getEnd(),
'input',
),
);
} else {
const elements = decoratorInputSpecifier.node.parent.elements;
const indexOfSpecifier = elements.findIndex((c) => c === decoratorInputSpecifier.node);
let start = decoratorInputSpecifier.node.getStart();
let end = decoratorInputSpecifier.node.getEnd();
// Either collapse (the comman between specifiers) with the preceding element,
// or with the following if present.
if (indexOfSpecifier > 0) {
start = elements[indexOfSpecifier - 1].getEnd();
} else {
if (elements.length > indexOfSpecifier + 1) {
end = elements[indexOfSpecifier + 1].getStart();
}
}
// We already have `input` in source code. Remove `Input` completely.
result.addReplacement(file.fileName, new Replacement(start, end, ''));
}
// All inputs were migrated, so we can safely remove the `Input` symbol.
if (!filesWithIncompatibleInputs.has(file)) {
importManager.removeImport(file, 'Input', '@angular/core');
}
}
}

38
packages/core/schematics/migrations/signal-migration/src/passes/9_migrate_ts_type_references.ts
View file

@ -23,10 +23,9 @@ import {ImportManager} from '../../../../../../compiler-cli/src/ngtsc/translator
export function pass9__migrateTypeScriptTypeReferences(
result: MigrationResult,
knownInputs: KnownInputs,
importManager: ImportManager,
) {
const seenTypeNodes = new WeakSet<ts.TypeReferenceNode>();
const fileNamesToFiles = new Map<string, ts.SourceFile>();
const importManager = new ImportManager();
for (const reference of result.references) {
// This pass only deals with TS input class type references.
@ -50,7 +49,6 @@ export function pass9__migrateTypeScriptTypeReferences(
const firstArg = reference.from.node.typeArguments[0];
const sf = firstArg.getSourceFile();
fileNamesToFiles.set(sf.fileName, sf);
const unwrapImportExpr = importManager.addImport({
exportModuleSpecifier: 'google3/javascript/angular2/testing/catalyst',
exportSymbolName: 'UnwrapSignalInputs',
@ -70,39 +68,5 @@ export function pass9__migrateTypeScriptTypeReferences(
new Replacement(firstArg.getEnd(), firstArg.getEnd(), '>'),
);
}
// TODO: Formalize import manager support across phases.
const {newImports, updatedImports} = importManager.finalize();
// Capture new imports
newImports.forEach((newImports, fileName) => {
newImports.forEach((newImport) => {
result.addReplacement(
fileName,
new Replacement(
0,
0,
ts
.createPrinter()
.printNode(ts.EmitHint.Unspecified, newImport, fileNamesToFiles.get(fileName)!) +
'\n',
),
);
});
});
// Capture updated imports
for (const [oldBindings, newBindings] of updatedImports.entries()) {
result.addReplacement(
oldBindings.getSourceFile().fileName,
new Replacement(
oldBindings.getStart(),
oldBindings.getEnd(),
ts
.createPrinter()
.printNode(ts.EmitHint.Unspecified, newBindings, oldBindings.getSourceFile()),
),
);
}
}
}

15
packages/core/schematics/migrations/signal-migration/src/phase_migrate.ts
View file

@ -15,6 +15,8 @@ import {pass7__migrateTemplateReferences} from './passes/7_migrate_template_refe
import {pass8__migrateHostBindings} from './passes/8_migrate_host_bindings';
import {pass9__migrateTypeScriptTypeReferences} from './passes/9_migrate_ts_type_references';
import {MigrationResult} from './result';
import {pass10_applyImportManager} from './passes/10_apply_import_manager';
import {ImportManager} from '../../../../../compiler-cli/src/ngtsc/translator';
/**
* Executes the migration phase.
@ -29,12 +31,19 @@ export function executeMigrationPhase(
host: MigrationHost,
knownInputs: KnownInputs,
result: MigrationResult,
{typeChecker}: AnalysisProgramInfo,
{typeChecker, sourceFiles}: AnalysisProgramInfo,
) {
const importManager = new ImportManager({
// For the purpose of this migration, we always use `input` and don't alias
// it to e.g. `input_1`.
generateUniqueIdentifier: () => null,
});
// Migrate passes.
pass5__migrateTypeScriptReferences(result, typeChecker, knownInputs);
pass6__migrateInputDeclarations(host, typeChecker, result, knownInputs);
pass6__migrateInputDeclarations(host, typeChecker, result, knownInputs, importManager);
pass7__migrateTemplateReferences(host, result, knownInputs);
pass8__migrateHostBindings(result, knownInputs);
pass9__migrateTypeScriptTypeReferences(result, knownInputs);
pass9__migrateTypeScriptTypeReferences(result, knownInputs, importManager);
pass10_applyImportManager(importManager, result, sourceFiles);
}

52
packages/core/schematics/migrations/signal-migration/test/golden.txt
View file

@ -2,7 +2,7 @@
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
@ -27,7 +27,7 @@ it('should work', () => {
// tslint:disable
import {Directive, Input, input} from '@angular/core';
import { Directive, Input, input } from '@angular/core';
class BaseNonAngular {
disabled: string = '';
@ -78,7 +78,7 @@ class BothInputImported {
import { UnwrapSignalInputs } from "google3/javascript/angular2/testing/catalyst";
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
// angular2/testing/catalyst
// ^^ this allows the advisor to even consider this file.
@ -101,7 +101,7 @@ it('should work', () => {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -173,7 +173,7 @@ class DerivedExternal extends Base2 {
// tslint:disable
import {input, Directive, Component} from '@angular/core';
import { Directive, Component, input } from '@angular/core';
// Material form field test case
@ -222,7 +222,7 @@ export class MatFormFieldTest {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class ExistingSignalImport {
signalInput = input<boolean>();
@ -254,7 +254,7 @@ export class WithGetters {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: '',
@ -307,7 +307,7 @@ class HostBindingsShared2 {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
const complex = 'some global variable';
@ -356,7 +356,7 @@ class MyComp {
// tslint:disable
import {Component, Input, input} from '@angular/core';
import { Component, Input, input } from '@angular/core';
interface Vehicle {}
interface Car extends Vehicle {
@ -492,7 +492,7 @@ x.incompatible = null;
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({template: ''})
class IndexAccessInput {
@ -549,7 +549,7 @@ describe('bla', () => {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -564,7 +564,7 @@ export class InlineTmpl {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class MyTestCmp {
someInput = input.required<boolean | string>();
@ -610,7 +610,7 @@ export class ManualInstantiation {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
export class TestCmp {
shared = input<{
@ -632,7 +632,7 @@ export class TestCmp {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
interface Config {
bla?: string;
@ -652,7 +652,7 @@ export class NestedTemplatePropAccess {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({})
export class ObjectExpansion {
@ -684,7 +684,7 @@ function assertValidLoadingInput(dir: AppComponent) {
// tslint:disable
import {Directive, input} from '@angular/core';
import { Directive, input } from '@angular/core';
@Directive()
class OptionalInput {
@ -729,7 +729,7 @@ export const COMPLEX_VAR = {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
import {COMPLEX_VAR} from './required-no-explicit-type-extra';
export const CONST = {field: true};
@ -746,7 +746,7 @@ export class RequiredNoExplicitType {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class Required {
simpleInput = input.required<string>();
@ -755,7 +755,7 @@ class Required {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
export class TestCmp {
shared = input(false);
@ -777,7 +777,7 @@ export class TestCmp {
// tslint:disable
import {input, Directive, Component} from '@angular/core';
import { Directive, Component, input } from '@angular/core';
@Directive()
class SomeDir {
@ -882,7 +882,7 @@ export class MyComp {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -900,7 +900,7 @@ export class TemplateObjectShorthand {
// tslint:disable
import {Component, Input, input} from '@angular/core';
import { Component, Input, input } from '@angular/core';
@Component({
template: `
@ -922,7 +922,7 @@ class TwoWayBinding {
// tslint:disable
import {Input, input} from '@angular/core';
import { Input, input } from '@angular/core';
import {COMPLEX_VAR} from './required-no-explicit-type-extra';
function x(v: string | undefined): string | undefined {
@ -946,7 +946,7 @@ export class TransformFunctions {
// tslint:disable
import {Directive, input} from '@angular/core';
import { Directive, input } from '@angular/core';
// see: button-base Material.
@ -959,7 +959,7 @@ class TransformIncompatibleTypes {
// tslint:disable
import {Input, input} from '@angular/core';
import { Input, input } from '@angular/core';
export class WithSettersAndGetters {
@Input()
@ -1001,7 +1001,7 @@ class WithGettersExternalRef {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class WithJsdoc {
/**

62
packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt
View file

@ -2,7 +2,7 @@
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
@ -27,7 +27,7 @@ it('should work', () => {
// tslint:disable
import {Directive, input} from '@angular/core';
import { Directive, input } from '@angular/core';
class BaseNonAngular {
disabled: string = '';
@ -61,7 +61,7 @@ class Sub3 implements BaseNonAngularInterface {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class BothInputImported {
decoratorInput = input(true);
@ -78,7 +78,7 @@ class BothInputImported {
import { UnwrapSignalInputs } from "google3/javascript/angular2/testing/catalyst";
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
// angular2/testing/catalyst
// ^^ this allows the advisor to even consider this file.
@ -101,7 +101,7 @@ it('should work', () => {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -128,7 +128,7 @@ class Option {
// tslint:disable
import {input, Directive} from '@angular/core';
import { Directive, input } from '@angular/core';
@Directive()
class Base {
@ -148,7 +148,7 @@ export class Base2 {
// tslint:disable
import {input, Directive} from '@angular/core';
import { Directive, input } from '@angular/core';
@Directive()
class Base {
@ -173,7 +173,7 @@ class DerivedExternal extends Base2 {
// tslint:disable
import {input, Directive, Component} from '@angular/core';
import { Directive, Component, input } from '@angular/core';
// Material form field test case
@ -222,7 +222,7 @@ export class MatFormFieldTest {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class ExistingSignalImport {
signalInput = input<boolean>();
@ -254,7 +254,7 @@ export class WithGetters {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: '',
@ -307,7 +307,7 @@ class HostBindingsShared2 {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
const complex = 'some global variable';
@ -356,7 +356,7 @@ class MyComp {
// tslint:disable
import {Component, Input, input} from '@angular/core';
import { Component, Input, input } from '@angular/core';
interface Vehicle {}
interface Car extends Vehicle {
@ -492,7 +492,7 @@ x.incompatible = null;
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({template: ''})
class IndexAccessInput {
@ -549,7 +549,7 @@ describe('bla', () => {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -564,7 +564,7 @@ export class InlineTmpl {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class MyTestCmp {
someInput = input.required<boolean | string>();
@ -610,7 +610,7 @@ export class ManualInstantiation {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
export class TestCmp {
shared = input<{
@ -632,7 +632,7 @@ export class TestCmp {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
interface Config {
bla?: string;
@ -652,7 +652,7 @@ export class NestedTemplatePropAccess {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({})
export class ObjectExpansion {
@ -684,7 +684,7 @@ function assertValidLoadingInput(dir: AppComponent) {
// tslint:disable
import {Directive, input} from '@angular/core';
import { Directive, input } from '@angular/core';
@Directive()
class OptionalInput {
@ -694,7 +694,7 @@ class OptionalInput {
// tslint:disable
import {Component, Directive, QueryList, input} from '@angular/core';
import { Component, Directive, QueryList, input } from '@angular/core';
@Component({
template: `
@ -729,7 +729,7 @@ export const COMPLEX_VAR = {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
import {COMPLEX_VAR} from './required-no-explicit-type-extra';
export const CONST = {field: true};
@ -746,7 +746,7 @@ export class RequiredNoExplicitType {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class Required {
simpleInput = input.required<string>();
@ -755,7 +755,7 @@ class Required {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
export class TestCmp {
shared = input(false);
@ -777,7 +777,7 @@ export class TestCmp {
// tslint:disable
import {input, Directive, Component} from '@angular/core';
import { Directive, Component, input } from '@angular/core';
@Directive()
class SomeDir {
@ -846,7 +846,7 @@ spyOn<MyComp>(new MyComp(), 'myInput').and.returnValue();
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -882,7 +882,7 @@ export class MyComp {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -900,7 +900,7 @@ export class TemplateObjectShorthand {
// tslint:disable
import {Component, input} from '@angular/core';
import { Component, input } from '@angular/core';
@Component({
template: `
@ -922,7 +922,7 @@ class TwoWayBinding {
// tslint:disable
import {Input, input} from '@angular/core';
import { Input, input } from '@angular/core';
import {COMPLEX_VAR} from './required-no-explicit-type-extra';
function x(v: string | undefined): string | undefined {
@ -946,7 +946,7 @@ export class TransformFunctions {
// tslint:disable
import {Directive, input} from '@angular/core';
import { Directive, input } from '@angular/core';
// see: button-base Material.
@ -959,7 +959,7 @@ class TransformIncompatibleTypes {
// tslint:disable
import {Input, input} from '@angular/core';
import { Input, input } from '@angular/core';
export class WithSettersAndGetters {
@Input()
@ -1001,7 +1001,7 @@ class WithGettersExternalRef {
// tslint:disable
import {input} from '@angular/core';
import { input } from '@angular/core';
class WithJsdoc {
/**