Skip to content

Commit 8fc5212

Browse files
committed
fix(core): The migration to the inject function handle overridden services correctly.
Fix angular#62232
1 parent 059fb06 commit 8fc5212

2 files changed

Lines changed: 151 additions & 3 deletions

File tree

packages/core/schematics/ng-generate/inject-migration/migration.ts

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,8 @@ function createInjectReplacementCall(
427427
}
428428
}
429429

430+
const originalParameterType = type ? type.getText() : '';
431+
430432
for (const decorator of decorators) {
431433
if (decorator.moduleName !== moduleName) {
432434
continue;
@@ -514,7 +516,33 @@ function createInjectReplacementCall(
514516
);
515517
}
516518

517-
return replaceNodePlaceholder(param.getSourceFile(), expression, injectedType, printer);
519+
let result = replaceNodePlaceholder(sourceFile, expression, injectedType, printer);
520+
521+
if (originalParameterType && injectedType && needsExplicitCast(originalParameterType, injectedType, param, localTypeChecker)) {
522+
let expressionWithoutTypes: ts.CallExpression | ts.NonNullExpression | ts.BinaryExpression = ts.factory.createCallExpression(injectRef, undefined, args);
523+
524+
if (hasOptionalDecorator && options.nonNullableOptional) {
525+
const hasNullableType = param.questionToken != null || (param.type != null && isNullableType(param.type));
526+
527+
if (!hasNullableType) {
528+
expressionWithoutTypes = ts.factory.createNonNullExpression(expressionWithoutTypes);
529+
}
530+
}
531+
532+
if (param.initializer) {
533+
expressionWithoutTypes = ts.factory.createBinaryExpression(
534+
expressionWithoutTypes,
535+
ts.SyntaxKind.QuestionQuestionToken,
536+
param.initializer,
537+
);
538+
}
539+
540+
result = replaceNodePlaceholder(sourceFile, expressionWithoutTypes, injectedType, printer);
541+
return `${result} as ${originalParameterType}`;
542+
543+
} else {
544+
return result;
545+
}
518546
}
519547

520548
/**
@@ -864,13 +892,14 @@ function replaceParameterReferencesInInitializer(
864892
localTypeChecker
865893
.getSymbolAtLocation(node)
866894
?.declarations?.some((decl) =>
867-
constructor.parameters.includes(decl as ts.ParameterDeclaration),
868-
)
895+
constructor.parameters.includes(decl as ts.ParameterDeclaration),
896+
)
869897
) {
870898
insertLocations.push(node.getStart() - initializer.getStart());
871899
}
872900
ts.forEachChild(node, walk);
873901
}
902+
874903
walk(initializer);
875904

876905
const initializerText = initializer.getText();
@@ -892,3 +921,80 @@ function isStringType(node: ts.Expression, checker: ts.TypeChecker): boolean {
892921
// stringLiteral here is to cover const strings inferred as literal type.
893922
return !!(type.flags & ts.TypeFlags.String || type.flags & ts.TypeFlags.StringLiteral);
894923
}
924+
925+
/**
926+
* Check if necessary an explicit cast for parameter
927+
* @param originalType Parameter original type
928+
* @param injectedType Service type to inject
929+
* @param param The parameter to analyze
930+
* @param localTypeChecker Type checker for types analysis
931+
*/
932+
function needsExplicitCast(
933+
originalType: string,
934+
injectedType: string,
935+
param: ts.ParameterDeclaration,
936+
localTypeChecker: ts.TypeChecker,
937+
): boolean {
938+
939+
if (originalType === injectedType) {
940+
return false;
941+
}
942+
943+
if (!param.type) {
944+
return false;
945+
}
946+
947+
try {
948+
const parameterType = localTypeChecker.getTypeFromTypeNode(param.type);
949+
const parameterSymbol = parameterType.getSymbol();
950+
951+
if (!parameterSymbol) {
952+
return false;
953+
}
954+
955+
const parameterDeclaration = parameterSymbol.valueDeclaration;
956+
if (!parameterDeclaration || !ts.isClassDeclaration(parameterDeclaration)) {
957+
return false;
958+
}
959+
960+
const hasInheritance = parameterDeclaration.heritageClauses?.some(clause =>
961+
clause.token === ts.SyntaxKind.ExtendsKeyword,
962+
);
963+
964+
if (hasInheritance) {
965+
return true;
966+
}
967+
968+
return checkInheritanceRelationship(originalType, injectedType);
969+
970+
} catch (error) {
971+
return false;
972+
}
973+
}
974+
975+
function checkInheritanceRelationship(
976+
derivedTypeName: string,
977+
baseTypeName: string,
978+
): boolean {
979+
980+
if (derivedTypeName.includes(baseTypeName)) {
981+
return true;
982+
}
983+
984+
const commonPatterns = [
985+
/Extended(.+)/,
986+
/Custom(.+)/,
987+
/Enhanced(.+)/,
988+
/(.+)Impl/,
989+
/(.+)Extended/,
990+
];
991+
992+
for (const pattern of commonPatterns) {
993+
const derivedMatch = derivedTypeName.match(pattern);
994+
if (derivedMatch && baseTypeName.includes(derivedMatch[1])) {
995+
return true;
996+
}
997+
}
998+
999+
return false;
1000+
}

packages/core/schematics/test/inject_migration_spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2933,5 +2933,47 @@ describe('inject migration', () => {
29332933
`}`,
29342934
]);
29352935
});
2936+
2937+
it('should migrate and handle overridden services correctly.', async () => {
2938+
writeFile(
2939+
'/dir.ts',
2940+
[
2941+
`import { Component, Inject, Injectable } from '@angular/core';`,
2942+
2943+
`@Injectable()`,
2944+
`export class FileService {`,
2945+
` getUrl() {`,
2946+
` return 'file';`,
2947+
` }`,
2948+
`}`,
2949+
2950+
`@Injectable()`,
2951+
`export class ExtendedFileService extends FileService {`,
2952+
`prefix: string = '';`,
2953+
2954+
` override getUrl() {`,
2955+
` return this.prefix + '/file';`,
2956+
` }`,
2957+
`}`,
2958+
2959+
`@Component({`,
2960+
` selector: 'app-root',`,
2961+
` providers: [{ provide: FileService, useClass: ExtendedFileService }],`,
2962+
` template: Hello world! {{fs.getUrl()}}`,
2963+
`})`,
2964+
`export class Playground {`,
2965+
` constructor(@Inject(FileService) protected fs: ExtendedFileService) {`,
2966+
` this.fs.prefix = 'a';`,
2967+
` }`,
2968+
`}`,
2969+
].join('\n'),
2970+
);
2971+
2972+
await runMigration();
2973+
2974+
expect(tree.readContent('/dir.ts')).toContain(
2975+
'protected fs = inject(FileService) as ExtendedFileService;'
2976+
);
2977+
});
29362978
});
29372979
});

0 commit comments

Comments
 (0)