fix(compiler-cli): use ModuleWithProviders type if static eval fails (#37126)
When the compiler encounters a function call within an NgModule imports section, it attempts to resolve it to an NgModule-annotated class by looking at the function body and evaluating the statements there. This evaluation can only understand simple functions which have a single return statement as their body. If the function the user writes is more complex than that, the compiler won't be able to understand it and previously the PartialEvaluator would return a "DynamicValue" for that import. With this change, in the event the function body resolution fails the PartialEvaluator will now attempt to use its foreign function resolvers to determine the correct result from the function's type signtaure instead. If the function is annotated with a correct ModuleWithProviders type, the compiler will be able to understand the import without static analysis of the function body. PR Close #37126
This commit is contained in:
parent
ab3f4c9fe1
commit
965a688c97
|
@ -11,7 +11,7 @@ import * as ts from 'typescript';
|
||||||
import {Reference} from '../../imports';
|
import {Reference} from '../../imports';
|
||||||
import {OwningModule} from '../../imports/src/references';
|
import {OwningModule} from '../../imports/src/references';
|
||||||
import {DependencyTracker} from '../../incremental/api';
|
import {DependencyTracker} from '../../incremental/api';
|
||||||
import {ConcreteDeclaration, Declaration, EnumMember, InlineDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
|
import {ConcreteDeclaration, Declaration, EnumMember, FunctionDefinition, InlineDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
|
||||||
import {isDeclaration} from '../../util/src/typescript';
|
import {isDeclaration} from '../../util/src/typescript';
|
||||||
|
|
||||||
import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin';
|
import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin';
|
||||||
|
@ -445,21 +445,54 @@ export class StaticInterpreter {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const res = this.visitExpression(expr, context);
|
return this.visitFfrExpression(expr, context);
|
||||||
if (res instanceof Reference) {
|
|
||||||
// This Reference was created synthetically, via a foreign function resolver. The real
|
|
||||||
// runtime value of the function expression may be different than the foreign function
|
|
||||||
// resolved value, so mark the Reference as synthetic to avoid it being misinterpreted.
|
|
||||||
res.synthetic = true;
|
|
||||||
}
|
|
||||||
return res;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const body = fn.body;
|
let res: ResolvedValue = this.visitFunctionBody(node, fn, context);
|
||||||
if (body.length !== 1 || !ts.isReturnStatement(body[0])) {
|
|
||||||
|
// If the result of attempting to resolve the function body was a DynamicValue, attempt to use
|
||||||
|
// the foreignFunctionResolver if one is present. This could still potentially yield a usable
|
||||||
|
// value.
|
||||||
|
if (res instanceof DynamicValue && context.foreignFunctionResolver !== undefined) {
|
||||||
|
const ffrExpr = context.foreignFunctionResolver(lhs, node.arguments);
|
||||||
|
if (ffrExpr !== null) {
|
||||||
|
// The foreign function resolver was able to extract an expression from this function. See
|
||||||
|
// if that expression leads to a non-dynamic result.
|
||||||
|
const ffrRes = this.visitFfrExpression(ffrExpr, context);
|
||||||
|
if (!(ffrRes instanceof DynamicValue)) {
|
||||||
|
// FFR yielded an actual result that's not dynamic, so use that instead of the original
|
||||||
|
// resolution.
|
||||||
|
res = ffrRes;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return res;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Visit an expression which was extracted from a foreign-function resolver.
|
||||||
|
*
|
||||||
|
* This will process the result and ensure it's correct for FFR-resolved values, including marking
|
||||||
|
* `Reference`s as synthetic.
|
||||||
|
*/
|
||||||
|
private visitFfrExpression(expr: ts.Expression, context: Context): ResolvedValue {
|
||||||
|
const res = this.visitExpression(expr, context);
|
||||||
|
if (res instanceof Reference) {
|
||||||
|
// This Reference was created synthetically, via a foreign function resolver. The real
|
||||||
|
// runtime value of the function expression may be different than the foreign function
|
||||||
|
// resolved value, so mark the Reference as synthetic to avoid it being misinterpreted.
|
||||||
|
res.synthetic = true;
|
||||||
|
}
|
||||||
|
return res;
|
||||||
|
}
|
||||||
|
|
||||||
|
private visitFunctionBody(node: ts.CallExpression, fn: FunctionDefinition, context: Context):
|
||||||
|
ResolvedValue {
|
||||||
|
if (fn.body === null || fn.body.length !== 1 || !ts.isReturnStatement(fn.body[0])) {
|
||||||
return DynamicValue.fromUnknown(node);
|
return DynamicValue.fromUnknown(node);
|
||||||
}
|
}
|
||||||
const ret = body[0] as ts.ReturnStatement;
|
const ret = fn.body[0] as ts.ReturnStatement;
|
||||||
|
|
||||||
const args = this.evaluateFunctionArguments(node, context);
|
const args = this.evaluateFunctionArguments(node, context);
|
||||||
const newScope: Scope = new Map<ts.ParameterDeclaration, ResolvedValue>();
|
const newScope: Scope = new Map<ts.ParameterDeclaration, ResolvedValue>();
|
||||||
|
|
|
@ -2390,6 +2390,45 @@ runInEachFileSystem(os => {
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('unwrapping ModuleWithProviders functions', () => {
|
describe('unwrapping ModuleWithProviders functions', () => {
|
||||||
|
it('should use a local ModuleWithProviders-annotated return type if a function is not statically analyzable',
|
||||||
|
() => {
|
||||||
|
env.write(`module.ts`, `
|
||||||
|
import {NgModule, ModuleWithProviders} from '@angular/core';
|
||||||
|
|
||||||
|
export function notStaticallyAnalyzable(): ModuleWithProviders<SomeModule> {
|
||||||
|
console.log('this interferes with static analysis');
|
||||||
|
return {
|
||||||
|
ngModule: SomeModule,
|
||||||
|
providers: [],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
@NgModule()
|
||||||
|
export class SomeModule {}
|
||||||
|
`);
|
||||||
|
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {NgModule} from '@angular/core';
|
||||||
|
import {notStaticallyAnalyzable} from './module';
|
||||||
|
|
||||||
|
@NgModule({
|
||||||
|
imports: [notStaticallyAnalyzable()]
|
||||||
|
})
|
||||||
|
export class TestModule {}
|
||||||
|
`);
|
||||||
|
|
||||||
|
env.driveMain();
|
||||||
|
|
||||||
|
const jsContents = env.getContents('test.js');
|
||||||
|
expect(jsContents).toContain('imports: [notStaticallyAnalyzable()]');
|
||||||
|
|
||||||
|
const dtsContents = env.getContents('test.d.ts');
|
||||||
|
expect(dtsContents).toContain(`import * as i1 from "./module";`);
|
||||||
|
expect(dtsContents)
|
||||||
|
.toContain(
|
||||||
|
'i0.ɵɵNgModuleDefWithMeta<TestModule, never, [typeof i1.SomeModule], never>');
|
||||||
|
});
|
||||||
|
|
||||||
it('should extract the generic type and include it in the module\'s declaration', () => {
|
it('should extract the generic type and include it in the module\'s declaration', () => {
|
||||||
env.write(`test.ts`, `
|
env.write(`test.ts`, `
|
||||||
import {NgModule} from '@angular/core';
|
import {NgModule} from '@angular/core';
|
||||||
|
|
Loading…
Reference in New Issue