From 58a79b6e435f2a46a7ab17ff5538083e05340b6f Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 14 Aug 2024 11:22:53 +0200 Subject: [PATCH] fix(migrations): preserve type when using inject decorator (#57389) Updates the migration so that it passes the type as a generic in the case of `@Inject(SOME_TOKEN) foo: SomeType`. This is done for two reasons: 1. It's a fairly common pattern and it ensures that the code can still be compiled. 2. It avoids leaving behind unused imports. PR Close #57389 --- .../ng-generate/inject-migration/migration.ts | 12 ++++++++++++ .../core/schematics/test/inject_migration_spec.ts | 10 +++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/core/schematics/ng-generate/inject-migration/migration.ts b/packages/core/schematics/ng-generate/inject-migration/migration.ts index c481bd7226b..8e13a252f22 100644 --- a/packages/core/schematics/ng-generate/inject-migration/migration.ts +++ b/packages/core/schematics/ng-generate/inject-migration/migration.ts @@ -526,6 +526,18 @@ function migrateInjectDecorator( injectedType = arrowFn.body.getText(); } } + } else if ( + // Pass the type for cases like `@Inject(FOO_TOKEN) foo: Foo`, because: + // 1. It guarantees that the type stays the same as before. + // 2. Avoids leaving unused imports behind. + // We only do this for type references since the `@Inject` pattern above is fairly common and + // apps don't necessarily type their injection tokens correctly, whereas doing it for literal + // types will add a lot of noise to the generated code. + type && + (ts.isTypeReferenceNode(type) || + (ts.isUnionTypeNode(type) && type.types.some(ts.isTypeReferenceNode))) + ) { + typeArguments = [type]; } return {injectedType, typeArguments}; diff --git a/packages/core/schematics/test/inject_migration_spec.ts b/packages/core/schematics/test/inject_migration_spec.ts index b42b4203930..8782ef67bee 100644 --- a/packages/core/schematics/test/inject_migration_spec.ts +++ b/packages/core/schematics/test/inject_migration_spec.ts @@ -114,7 +114,7 @@ describe('inject migration', () => { ``, `@Directive()`, `class MyDir {`, - ` private foo = inject(FOO_TOKEN);`, + ` private foo = inject(FOO_TOKEN);`, `}`, ]); }); @@ -271,7 +271,7 @@ describe('inject migration', () => { ` ]`, `})`, `class MyDir {`, - ` private foo = inject(FOO_TOKEN, { optional: true });`, + ` private foo = inject(FOO_TOKEN, { optional: true });`, `}`, ]); }); @@ -1431,7 +1431,7 @@ describe('inject migration', () => { `@Directive()`, `class MyDir {`, ` private foo = inject(Foo);`, - ` readonly bar = inject(BAR_TOKEN);`, + ` readonly bar = inject(BAR_TOKEN);`, ``, ` private value: number = this.foo.getValue();`, ` private otherValue: string = this.bar.getOtherValue();`, @@ -1470,7 +1470,7 @@ describe('inject migration', () => { `@Directive()`, `class MyDir {`, ` private foo = inject(Foo);`, - ` readonly bar = inject(BAR_TOKEN);`, + ` readonly bar = inject(BAR_TOKEN);`, ``, ` private value: number = this.foo.getValue();`, ` private otherValue: string;`, @@ -1599,7 +1599,7 @@ describe('inject migration', () => { `@Directive()`, `class MyDir {`, ` private foo = inject(Foo);`, - ` readonly bar = inject(BAR_TOKEN);`, + ` readonly bar = inject(BAR_TOKEN);`, ``, ` private 'my-value': number = this.foo.getValue();`, ` private 'my-other-value': string = this.bar.getOtherValue();`,