Skip to content

Commit

Permalink
fix(migrations): unwrap injected forwardRef (#57127)
Browse files Browse the repository at this point in the history
Updates the inject migration to unwrap the `forwardRef` call in cases like `constructor(@Inject(forwardRef(() => Foo)) foo: Foo);`, because the `forwardRef` will type the initializer to `any` and it shouldn't be necessary.

PR Close #57127
  • Loading branch information
crisbeto authored and dylhunn committed Jul 26, 2024
1 parent cb442a0 commit aae9646
Show file tree
Hide file tree
Showing 2 changed files with 150 additions and 7 deletions.
62 changes: 55 additions & 7 deletions packages/core/schematics/ng-generate/inject-migration/migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
parameterDeclaresProperty,
} from './analysis';
import {getAngularDecorators} from '../../utils/ng_decorators';
import {getImportOfIdentifier} from '../../utils/typescript/imports';

/**
* Placeholder used to represent expressions inside the AST.
Expand Down Expand Up @@ -336,13 +337,10 @@ function createInjectReplacementCall(
switch (decorator.name) {
case 'Inject':
if (firstArg) {
injectedType = firstArg.getText();

// `inject` no longer officially supports string injection so we need
// to cast to any. We maintain the type by passing it as a generic.
if (ts.isStringLiteralLike(firstArg)) {
typeArguments = [type || ts.factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)];
injectedType += ' as any';
const injectResult = migrateInjectDecorator(firstArg, type, localTypeChecker);
injectedType = injectResult.injectedType;
if (injectResult.typeArguments) {
typeArguments = injectResult.typeArguments;
}
}
break;
Expand Down Expand Up @@ -401,6 +399,56 @@ function createInjectReplacementCall(
return replaceNodePlaceholder(param.getSourceFile(), expression, injectedType, printer);
}

/**
* Migrates a parameter based on its `@Inject()` decorator.
* @param firstArg First argument to `@Inject()`.
* @param type Type of the parameter.
* @param localTypeChecker Type checker set up for the specific file.
*/
function migrateInjectDecorator(
firstArg: ts.Expression,
type: ts.TypeNode | undefined,
localTypeChecker: ts.TypeChecker,
) {
let injectedType = firstArg.getText();
let typeArguments: ts.TypeNode[] | null = null;

// `inject` no longer officially supports string injection so we need
// to cast to any. We maintain the type by passing it as a generic.
if (ts.isStringLiteralLike(firstArg)) {
typeArguments = [type || ts.factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)];
injectedType += ' as any';
} else if (
ts.isCallExpression(firstArg) &&
ts.isIdentifier(firstArg.expression) &&
firstArg.arguments.length === 1
) {
const callImport = getImportOfIdentifier(localTypeChecker, firstArg.expression);
const arrowFn = firstArg.arguments[0];

// If the first parameter is a `forwardRef`, unwrap it for a more
// accurate type and because it's no longer necessary.
if (
callImport !== null &&
callImport.name === 'forwardRef' &&
callImport.importModule === '@angular/core' &&
ts.isArrowFunction(arrowFn)
) {
if (ts.isBlock(arrowFn.body)) {
const returnStatement = arrowFn.body.statements.find((stmt) => ts.isReturnStatement(stmt));

if (returnStatement && returnStatement.expression) {
injectedType = returnStatement.expression.getText();
}
} else {
injectedType = arrowFn.body.getText();
}
}
}

return {injectedType, typeArguments};
}

/**
* Removes the parameters from a constructor. This is a bit more complex than just replacing an AST
* node, because `NodeArray.pos` includes any leading whitespace, but `NodeArray.end` does **not**
Expand Down
95 changes: 95 additions & 0 deletions packages/core/schematics/test/inject_migration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1166,4 +1166,99 @@ describe('inject migration', () => {
`}`,
]);
});

it('should unwrap forwardRef with an implicit return', async () => {
writeFile(
'/dir.ts',
[
`import { Directive, Inject, forwardRef } from '@angular/core';`,
``,
`@Directive()`,
`class MyDir {`,
` constructor(@Inject(forwardRef(() => Foo)) readonly foo: Foo) {}`,
`}`,
``,
`@Directive()`,
`class Foo {}`,
].join('\n'),
);

await runMigration();

expect(tree.readContent('/dir.ts').split('\n')).toEqual([
`import { Directive, Inject, forwardRef, inject } from '@angular/core';`,
``,
`@Directive()`,
`class MyDir {`,
` readonly foo = inject(Foo);`,
`}`,
``,
`@Directive()`,
`class Foo {}`,
]);
});

it('should unwrap forwardRef with an explicit return', async () => {
writeFile(
'/dir.ts',
[
`import { Directive, Inject, forwardRef } from '@angular/core';`,
``,
`@Directive()`,
`class MyDir {`,
` constructor(@Inject(forwardRef(() => {`,
` return Foo;`,
` })) readonly foo: Foo) {}`,
`}`,
``,
`@Directive()`,
`class Foo {}`,
].join('\n'),
);

await runMigration();

expect(tree.readContent('/dir.ts').split('\n')).toEqual([
`import { Directive, Inject, forwardRef, inject } from '@angular/core';`,
``,
`@Directive()`,
`class MyDir {`,
` readonly foo = inject(Foo);`,
`}`,
``,
`@Directive()`,
`class Foo {}`,
]);
});

it('should unwrap an aliased forwardRef', async () => {
writeFile(
'/dir.ts',
[
`import { Directive, Inject, forwardRef as aliasedForwardRef } from '@angular/core';`,
``,
`@Directive()`,
`class MyDir {`,
` constructor(@Inject(aliasedForwardRef(() => Foo)) readonly foo: Foo) {}`,
`}`,
``,
`@Directive()`,
`class Foo {}`,
].join('\n'),
);

await runMigration();

expect(tree.readContent('/dir.ts').split('\n')).toEqual([
`import { Directive, Inject, forwardRef as aliasedForwardRef, inject } from '@angular/core';`,
``,
`@Directive()`,
`class MyDir {`,
` readonly foo = inject(Foo);`,
`}`,
``,
`@Directive()`,
`class Foo {}`,
]);
});
});

0 comments on commit aae9646

Please sign in to comment.