diff --git a/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility.md b/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility.md deleted file mode 100644 index d1651ff7061..00000000000 --- a/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility.md +++ /dev/null @@ -1,85 +0,0 @@ -# Possible reasons why an `@Input` is not migrated - -The signal input migration may skip migration of `@Input()` declarations if the automated refactoring isn't possible or safe. -This document explains some of the reasons, or better known "migration incompatibilities". - -### `REASON:Accessor` -The input is declared using a getter/setter. -This is non-trivial to migrate without knowing the intent. - -### `REASON:WriteAssignment` -Parts of your application write to this input class field. -This blocks the automated migration because signal inputs cannot be modified programmatically. - -### `REASON:OverriddenByDerivedClass` -The input is part of a class that is extended by another class which overrides the field. -Migrating the input would then cause type conflicts and break the application build. - -```ts -@Component() -class MyComp { - @Input() myInput = true; -} - -class Derived extends MyComp { - override myInput = false; // <-- this wouldn't be a signal input and break! -} -``` - -### `REASON:RedeclaredViaDerivedClassInputsArray` -The input is part of a class that is extended by another class which overrides the field via the `inputs` array of `@Component` or `@Directive`. -Migrating the input would cause a mismatch because fields declared via `inputs` cannot be signal inputs. - -```ts -@Component() -class MyComp { - @Input() myInput = true; -} - -@Component({ - inputs: ['myInput: aliasedName'] -}) -class Derived extends MyComp {} -``` - -### `REASON:TypeConflictWithBaseClass` -The input is overriding a field from a superclass, but the superclass field is not an Angular `@Input` and is not migrated. -This results in a type conflict and would break the build. - -```ts -interface Parent { - disabled: boolean; -} - -@Component() -class MyComp implements Parent { - @Input() disabled = true; -} -``` - -### `REASON:ParentIsIncompatible` -The input is overriding a field from a superclass, but the superclass field could not be migrated. -This means that migrating the input would break the build. - -### `REASON:SpyOnThatOverwritesField` -The input can be migrated, but a Jasmine `spyOn` call for the input field was discovered. -`spyOn` calls are incompatible with signal inputs because they attempt to override the value of the field. -Signal inputs cannot be changed programmatically though— so this breaks. - -### `REASON:PotentiallyNarrowedInTemplateButNoSupportYet` -The input is part of an `@if` or `*ngIf`, or template input in general. -This indicates that the input value type may be narrowed. - -The migration skips migrating such inputs because support for narrowed signals is not available yet. - -### `REASON:RequiredInputButNoGoodExplicitTypeExtractable` -The input is required, but cannot be safely migrated because no good type could be detected. - -A required input with initial value doesn't make sense. -The type is inferred with `@Input` via the initial value, but this isn't possible with `input.required`. - -```ts -class MyComp { - @Input({required: true}) bla = someComplexInitialValue(); -} -``` diff --git a/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility_human.ts b/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility_human.ts new file mode 100644 index 00000000000..136bf8a1b92 --- /dev/null +++ b/packages/core/schematics/migrations/signal-migration/src/input_detection/incompatibility_human.ts @@ -0,0 +1,106 @@ +/** + * @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 {ClassIncompatibilityReason, InputIncompatibilityReason} from './incompatibility'; + +/** + * Gets human-readable message information for the given input incompatibility. + * This text will be used by the language service, or CLI-based migration. + */ +export function getMessageForInputIncompatibility(reason: InputIncompatibilityReason): { + short: string; + extra: string; +} { + switch (reason) { + case InputIncompatibilityReason.Accessor: + return { + short: 'Accessor inputs cannot be migrated as they are too complex.', + extra: + 'The migration potentially requires usage of `effect` or `computed`, but ' + + 'the intent is unclear. The migration cannot safely migrate.', + }; + case InputIncompatibilityReason.OverriddenByDerivedClass: + return { + short: 'The input cannot be migrated because the field is overridden by a subclass.', + extra: 'The field in the subclass is not an input, so migrating would break your build.', + }; + case InputIncompatibilityReason.ParentIsIncompatible: + return { + short: 'This input is inherited from a superclass, but the parent cannot be migrated.', + extra: 'Migrating this input would cause your build to fail.', + }; + case InputIncompatibilityReason.PotentiallyNarrowedInTemplateButNoSupportYet: + return { + short: + 'This input is used in a control flow expression (e.g. `@if` or `*ngIf`) and ' + + 'migrating would break narrowing currently.', + extra: `In the future, Angular intends to support narrowing of signals.`, + }; + case InputIncompatibilityReason.RedeclaredViaDerivedClassInputsArray: + return { + short: 'The input is overridden by a subclass that cannot be migrated.', + extra: + 'The subclass re-declares this input via the `inputs` array in @Directive/@Component. ' + + 'Migrating this input would break your build because the subclass input cannot be migrated.', + }; + case InputIncompatibilityReason.RequiredInputButNoGoodExplicitTypeExtractable: + return { + short: `Input is required, but the migration cannot determine a good type for the input.`, + extra: 'Consider adding an explicit type to make the migration possible.', + }; + case InputIncompatibilityReason.SkippedViaConfigFilter: + return { + short: `This input is not part of the current migration scope.`, + extra: 'Skipped via migration config.', + }; + case InputIncompatibilityReason.SpyOnThatOverwritesField: + return { + short: 'A jasmine `spyOn` call spies on this input. This breaks with signal inputs.', + extra: `Migration cannot safely migrate as "spyOn" writes to the input. Signal inputs are readonly.`, + }; + case InputIncompatibilityReason.TypeConflictWithBaseClass: + return { + short: + 'This input overrides a field from a superclass, while the superclass field is not migrated.', + extra: 'Migrating the input would break your build because of a type conflict then.', + }; + case InputIncompatibilityReason.WriteAssignment: + return { + short: 'Your application code writes to the input. This prevents migration.', + extra: 'Signal inputs are readonly, so migrating would break your build.', + }; + } +} + +/** + * Gets human-readable message information for the given input class incompatibility. + * This text will be used by the language service, or CLI-based migration. + */ +export function getMessageForClassIncompatibility(reason: ClassIncompatibilityReason): { + short: string; + extra: string; +} { + switch (reason) { + case ClassIncompatibilityReason.InputOwningClassReferencedInClassProperty: + return { + short: 'Class of this input is referenced in the signature of another class.', + extra: + 'The other class is likely typed to expect a non-migrated field, so ' + + 'migration is skipped to not break your build.', + }; + case ClassIncompatibilityReason.ClassManuallyInstantiated: + return { + short: + 'Class of this input is manually instantiated (`new Cmp()`). ' + + 'This is discouraged and prevents migration', + extra: + 'Signal inputs require a DI injection context. Manually instantiating ' + + 'breaks this requirement in some cases, so the migration is skipped.', + }; + } +} diff --git a/packages/language-service/src/refactorings/convert_to_signal_input.ts b/packages/language-service/src/refactorings/convert_to_signal_input.ts index 7f6b2039955..a21b2b8f1d1 100644 --- a/packages/language-service/src/refactorings/convert_to_signal_input.ts +++ b/packages/language-service/src/refactorings/convert_to_signal_input.ts @@ -11,29 +11,29 @@ import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; import {getFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system'; import {MetaKind} from '@angular/compiler-cli/src/ngtsc/metadata'; import { - ClassIncompatibilityReason, - InputIncompatibilityReason, -} from '@angular/core/schematics/migrations/signal-migration/src/input_detection/incompatibility'; + getMessageForClassIncompatibility, + getMessageForInputIncompatibility, +} from '@angular/core/schematics/migrations/signal-migration/src/input_detection/incompatibility_human'; import {ApplyRefactoringProgressFn} from '@angular/language-service/api'; import ts from 'typescript'; -import {isInputContainerNode} from '../../../core/schematics/migrations/signal-migration/src/input_detection/input_node'; -import {SignalInputMigration} from '../../../core/schematics/migrations/signal-migration/src/migration'; -import {groupReplacementsByFile} from '../../../core/schematics/utils/tsurge/helpers/group_replacements'; +import {isInputContainerNode} from '@angular/core/schematics/migrations/signal-migration/src/input_detection/input_node'; +import {SignalInputMigration} from '@angular/core/schematics/migrations/signal-migration/src/migration'; +import {MigrationConfig} from '@angular/core/schematics/migrations/signal-migration/src/migration_config'; +import {groupReplacementsByFile} from '@angular/core/schematics/utils/tsurge/helpers/group_replacements'; +import {isTypeScriptFile} from '../utils'; import {findTightestNode, getParentClassDeclaration} from '../utils/ts_utils'; import type {ActiveRefactoring} from './refactoring'; -import {isTypeScriptFile} from '../utils'; /** - * Language service refactoring action that can convert `@Input()` + * Base language service refactoring action that can convert `@Input()` * declarations to signal inputs. * * The user can click on an `@Input` property declaration in e.g. the VSCode * extension and ask for the input to be migrated. All references, imports and * the declaration are updated automatically. */ -export class ConvertToSignalInputRefactoring implements ActiveRefactoring { - static id = 'convert-to-signal-input'; - static description = '(experimental fixer): Convert @Input() to a signal input'; +abstract class BaseConvertToSignalInputRefactoring implements ActiveRefactoring { + abstract config: MigrationConfig; static isApplicable( compiler: NgCompiler, @@ -106,6 +106,7 @@ export class ConvertToSignalInputRefactoring implements ActiveRefactoring { const fs = getFileSystem(); const migration = new SignalInputMigration({ + ...this.config, upgradeAnalysisPhaseToAvoidBatch: true, reportProgressFn: reportProgress, shouldMigrateInput: (input) => input.descriptor.node === containingProp, @@ -145,17 +146,29 @@ export class ConvertToSignalInputRefactoring implements ActiveRefactoring { const {container, descriptor} = targetInput; const memberIncompatibility = container.memberIncompatibility.get(descriptor.key); const classIncompatibility = container.incompatible; + const aggressiveModeRecommendation = !this.config.bestEffortMode + ? `\n—— Consider using the "(forcibly, ignoring errors)" action to forcibly convert.` + : ''; + if (memberIncompatibility !== undefined) { + const {short, extra} = getMessageForInputIncompatibility(memberIncompatibility.reason); + return { + edits: [], + notApplicableReason: `${short}\n${extra}${aggressiveModeRecommendation}`, + }; + } + if (classIncompatibility !== null) { + const {short, extra} = getMessageForClassIncompatibility(classIncompatibility); + return { + edits: [], + notApplicableReason: `${short}\n${extra}${aggressiveModeRecommendation}`, + }; + } return { edits: [], - // TODO: Output a better human-readable message here. For now this is better than a noop. - notApplicableReason: `Input cannot be migrated: ${ - memberIncompatibility !== undefined - ? InputIncompatibilityReason[memberIncompatibility.reason] - : classIncompatibility !== null - ? ClassIncompatibilityReason[classIncompatibility] - : 'unknown' - }`, + notApplicableReason: + 'Input cannot be migrated, but no reason was found. ' + + 'Consider reporting a bug to the Angular team.', }; } @@ -184,6 +197,17 @@ export class ConvertToSignalInputRefactoring implements ActiveRefactoring { } } +export class ConvertToSignalInputRefactoring extends BaseConvertToSignalInputRefactoring { + static id = 'convert-to-signal-input-safe-mode'; + static description = 'Convert this @Input() to a signal input (safe)'; + override config: MigrationConfig = {}; +} +export class ConvertToSignalInputBestEffortRefactoring extends BaseConvertToSignalInputRefactoring { + static id = 'convert-to-signal-input-best-effort-mode'; + static description = 'Convert @Input() to a signal input (forcibly, ignoring errors)'; + override config: MigrationConfig = {bestEffortMode: true}; +} + function findParentPropertyDeclaration(node: ts.Node): ts.PropertyDeclaration | null { while (!ts.isPropertyDeclaration(node) && !ts.isSourceFile(node)) { node = node.parent; diff --git a/packages/language-service/src/refactorings/refactoring.ts b/packages/language-service/src/refactorings/refactoring.ts index ed0b60cbefa..327e2219a0d 100644 --- a/packages/language-service/src/refactorings/refactoring.ts +++ b/packages/language-service/src/refactorings/refactoring.ts @@ -10,7 +10,10 @@ import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; import ts from 'typescript'; import {ApplyRefactoringProgressFn} from '@angular/language-service/api'; import {CompilerOptions} from '@angular/compiler-cli'; -import {ConvertToSignalInputRefactoring} from './convert_to_signal_input'; +import { + ConvertToSignalInputBestEffortRefactoring, + ConvertToSignalInputRefactoring, +} from './convert_to_signal_input'; /** * Interface exposing static metadata for a {@link Refactoring}, @@ -60,4 +63,7 @@ export interface ActiveRefactoring { ): Promise; } -export const allRefactorings: Refactoring[] = [ConvertToSignalInputRefactoring]; +export const allRefactorings: Refactoring[] = [ + ConvertToSignalInputRefactoring, + ConvertToSignalInputBestEffortRefactoring, +]; diff --git a/packages/language-service/test/signal_input_refactoring_action_spec.ts b/packages/language-service/test/signal_input_refactoring_action_spec.ts index 458f7bd68c0..2a7168e6cdb 100644 --- a/packages/language-service/test/signal_input_refactoring_action_spec.ts +++ b/packages/language-service/test/signal_input_refactoring_action_spec.ts @@ -34,8 +34,9 @@ describe('Signal input refactoring action', () => { appFile.moveCursorToText('bl¦a'); const refactorings = project.getRefactoringsAtPosition('app.ts', appFile.cursor); - expect(refactorings.length).toBe(1); - expect(refactorings[0].name).toBe('convert-to-signal-input'); + expect(refactorings.length).toBe(2); + expect(refactorings[0].name).toBe('convert-to-signal-input-safe-mode'); + expect(refactorings[1].name).toBe('convert-to-signal-input-best-effort-mode'); }); it('should not support refactoring a non-Angular property', () => { @@ -95,8 +96,9 @@ describe('Signal input refactoring action', () => { appFile.moveCursorToText('bl¦a'); const refactorings = project.getRefactoringsAtPosition('app.ts', appFile.cursor); - expect(refactorings.length).toBe(1); - expect(refactorings[0].name).toBe('convert-to-signal-input'); + expect(refactorings.length).toBe(2); + expect(refactorings[0].name).toBe('convert-to-signal-input-safe-mode'); + expect(refactorings[1].name).toBe('convert-to-signal-input-best-effort-mode'); const edits = await project.applyRefactoring( 'app.ts',