fix(core): missing-injectable migration should handle forwardRef (#33286)
Currently the migration is unable to migrate instances where the provider definition uses `forwardRef`. Since this is a common pattern, we should support that from within the migration. The solution to the problem is adding a foreign function resolver to the `PartialEvaluator`. This basically matches the usage of the static evaluation that is used by the ngtsc annotations. PR Close #33286
This commit is contained in:
parent
4b81bb5c97
commit
eeecbf28e4
@ -11,6 +11,7 @@ ts_library(
|
|||||||
"//packages/core/schematics/test:__pkg__",
|
"//packages/core/schematics/test:__pkg__",
|
||||||
],
|
],
|
||||||
deps = [
|
deps = [
|
||||||
|
"//packages/compiler-cli/src/ngtsc/annotations",
|
||||||
"//packages/compiler-cli/src/ngtsc/imports",
|
"//packages/compiler-cli/src/ngtsc/imports",
|
||||||
"//packages/compiler-cli/src/ngtsc/partial_evaluator",
|
"//packages/compiler-cli/src/ngtsc/partial_evaluator",
|
||||||
"//packages/compiler-cli/src/ngtsc/reflection",
|
"//packages/compiler-cli/src/ngtsc/reflection",
|
||||||
|
@ -6,6 +6,7 @@
|
|||||||
* found in the LICENSE file at https://angular.io/license
|
* found in the LICENSE file at https://angular.io/license
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
import {forwardRefResolver} from '@angular/compiler-cli/src/ngtsc/annotations/src/util';
|
||||||
import {Reference} from '@angular/compiler-cli/src/ngtsc/imports';
|
import {Reference} from '@angular/compiler-cli/src/ngtsc/imports';
|
||||||
import {DynamicValue, PartialEvaluator, ResolvedValue} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
|
import {DynamicValue, PartialEvaluator, ResolvedValue} from '@angular/compiler-cli/src/ngtsc/partial_evaluator';
|
||||||
import {TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection';
|
import {TypeScriptReflectionHost} from '@angular/compiler-cli/src/ngtsc/reflection';
|
||||||
@ -67,7 +68,7 @@ export class MissingInjectableTransform {
|
|||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
|
|
||||||
const evaluatedExpr = this.partialEvaluator.evaluate(module.providersExpr);
|
const evaluatedExpr = this._evaluateExpression(module.providersExpr);
|
||||||
|
|
||||||
if (!Array.isArray(evaluatedExpr)) {
|
if (!Array.isArray(evaluatedExpr)) {
|
||||||
return [{
|
return [{
|
||||||
@ -89,7 +90,7 @@ export class MissingInjectableTransform {
|
|||||||
|
|
||||||
// Migrate "providers" on directives and components if defined.
|
// Migrate "providers" on directives and components if defined.
|
||||||
if (directive.providersExpr) {
|
if (directive.providersExpr) {
|
||||||
const evaluatedExpr = this.partialEvaluator.evaluate(directive.providersExpr);
|
const evaluatedExpr = this._evaluateExpression(directive.providersExpr);
|
||||||
if (!Array.isArray(evaluatedExpr)) {
|
if (!Array.isArray(evaluatedExpr)) {
|
||||||
return [
|
return [
|
||||||
{node: directive.providersExpr, message: `Providers are not statically analyzable.`}
|
{node: directive.providersExpr, message: `Providers are not statically analyzable.`}
|
||||||
@ -100,7 +101,7 @@ export class MissingInjectableTransform {
|
|||||||
|
|
||||||
// Migrate "viewProviders" on components if defined.
|
// Migrate "viewProviders" on components if defined.
|
||||||
if (directive.viewProvidersExpr) {
|
if (directive.viewProvidersExpr) {
|
||||||
const evaluatedExpr = this.partialEvaluator.evaluate(directive.viewProvidersExpr);
|
const evaluatedExpr = this._evaluateExpression(directive.viewProvidersExpr);
|
||||||
if (!Array.isArray(evaluatedExpr)) {
|
if (!Array.isArray(evaluatedExpr)) {
|
||||||
return [
|
return [
|
||||||
{node: directive.viewProvidersExpr, message: `Providers are not statically analyzable.`}
|
{node: directive.viewProvidersExpr, message: `Providers are not statically analyzable.`}
|
||||||
@ -150,6 +151,14 @@ export class MissingInjectableTransform {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Evaluates the given TypeScript expression using the partial evaluator with
|
||||||
|
* the foreign function resolver for handling "forwardRef" calls.
|
||||||
|
*/
|
||||||
|
private _evaluateExpression(expr: ts.Expression): ResolvedValue {
|
||||||
|
return this.partialEvaluator.evaluate(expr, forwardRefResolver);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Visits the given resolved value of a provider. Providers can be nested in
|
* Visits the given resolved value of a provider. Providers can be nested in
|
||||||
* arrays and we need to recursively walk through the providers to be able to
|
* arrays and we need to recursively walk through the providers to be able to
|
||||||
|
@ -48,6 +48,10 @@ describe('Missing injectable migration', () => {
|
|||||||
// Switch into the temporary directory path. This allows us to run
|
// Switch into the temporary directory path. This allows us to run
|
||||||
// the schematic against our custom unit test tree.
|
// the schematic against our custom unit test tree.
|
||||||
shx.cd(tmpDirPath);
|
shx.cd(tmpDirPath);
|
||||||
|
|
||||||
|
writeFile('/node_modules/@angular/core/index.d.ts', `
|
||||||
|
export declare function forwardRef(fn: Function);
|
||||||
|
`);
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@ -135,6 +139,24 @@ describe('Missing injectable migration', () => {
|
|||||||
.toContain(`{ ${type}, Injectable } from '@angular/core`);
|
.toContain(`{ ${type}, Injectable } from '@angular/core`);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it(`should migrate object literal provider with forwardRef in ${type}`, async() => {
|
||||||
|
writeFile('/index.ts', `
|
||||||
|
import {${type}, forwardRef} from '@angular/core';
|
||||||
|
|
||||||
|
@${type}({${propName}: [{provide: forwardRef(() => MyService)}]})
|
||||||
|
export class TestClass {}
|
||||||
|
|
||||||
|
export class MyService {}
|
||||||
|
`);
|
||||||
|
|
||||||
|
await runMigration();
|
||||||
|
|
||||||
|
expect(warnOutput.length).toBe(0);
|
||||||
|
expect(tree.readContent('/index.ts')).toMatch(/@Injectable\(\)\s+export class MyService/);
|
||||||
|
expect(tree.readContent('/index.ts'))
|
||||||
|
.toContain(`{ ${type}, forwardRef, Injectable } from '@angular/core`);
|
||||||
|
});
|
||||||
|
|
||||||
it(`should not migrate object literal provider with "useValue" in ${type}`, async() => {
|
it(`should not migrate object literal provider with "useValue" in ${type}`, async() => {
|
||||||
writeFile('/index.ts', `
|
writeFile('/index.ts', `
|
||||||
import {${type}} from '@angular/core';
|
import {${type}} from '@angular/core';
|
||||||
|
Loading…
x
Reference in New Issue
Block a user