fix(migrations): preserve type when using inject decorator (#57389)

crisbeto · dylhunn · commit 6b4357fae45f · 2024-08-15T12:11:59.000-04:00
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
diff --git 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
@@ -114,7 +114,7 @@ describe('inject migration', () => {
       ``,
       `@Directive()`,
       `class MyDir {`,
-      `  private foo = inject(FOO_TOKEN);`,
+      `  private foo = inject<Foo>(FOO_TOKEN);`,
       `}`,
     ]);
   });
@@ -271,7 +271,7 @@ describe('inject migration', () => {
       `  ]`,
       `})`,
       `class MyDir {`,
-      `  private foo = inject(FOO_TOKEN, { optional: true });`,
+      `  private foo = inject<Foo>(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>(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>(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>(BAR_TOKEN);`,
         ``,
         `  private 'my-value': number = this.foo.getValue();`,
         `  private 'my-other-value': string = this.bar.getOtherValue();`,

Original file line number	Diff line number	Diff line change
`@@ -526,6 +526,18 @@ function migrateInjectDecorator(`
`526`	`526`	`injectedType = arrowFn.body.getText();`
`527`	`527`	`}`
`528`	`528`	`}`
	`529`	`+ } else if (`
	`530`	+ // Pass the type for cases like `@Inject(FOO_TOKEN) foo: Foo`, because:
	`531`	`+ // 1. It guarantees that the type stays the same as before.`
	`532`	`+ // 2. Avoids leaving unused imports behind.`
	`533`	+ // We only do this for type references since the `@Inject` pattern above is fairly common and
	`534`	`+ // apps don't necessarily type their injection tokens correctly, whereas doing it for literal`
	`535`	`+ // types will add a lot of noise to the generated code.`
	`536`	`+ type &&`
	`537`	`+ (ts.isTypeReferenceNode(type) \|\|`
	`538`	`+ (ts.isUnionTypeNode(type) && type.types.some(ts.isTypeReferenceNode)))`
	`539`	`+ ) {`
	`540`	`+ typeArguments = [type];`
`529`	`541`	`}`
`530`	`542`
`531`	`543`	`return {injectedType, typeArguments};`