Skip to content

Commit 965a688

Browse files
alxhubatscott
authored andcommitted
fix(compiler-cli): use ModuleWithProviders type if static eval fails (angular#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 angular#37126
1 parent ab3f4c9 commit 965a688

File tree

2 files changed

+83
-11
lines changed

2 files changed

+83
-11
lines changed

packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import * as ts from 'typescript';
1111
import {Reference} from '../../imports';
1212
import {OwningModule} from '../../imports/src/references';
1313
import {DependencyTracker} from '../../incremental/api';
14-
import {ConcreteDeclaration, Declaration, EnumMember, InlineDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
14+
import {ConcreteDeclaration, Declaration, EnumMember, FunctionDefinition, InlineDeclaration, ReflectionHost, SpecialDeclarationKind} from '../../reflection';
1515
import {isDeclaration} from '../../util/src/typescript';
1616

1717
import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin';
@@ -445,21 +445,54 @@ export class StaticInterpreter {
445445
};
446446
}
447447

448-
const res = this.visitExpression(expr, context);
449-
if (res instanceof Reference) {
450-
// This Reference was created synthetically, via a foreign function resolver. The real
451-
// runtime value of the function expression may be different than the foreign function
452-
// resolved value, so mark the Reference as synthetic to avoid it being misinterpreted.
453-
res.synthetic = true;
448+
return this.visitFfrExpression(expr, context);
449+
}
450+
451+
let res: ResolvedValue = this.visitFunctionBody(node, fn, context);
452+
453+
// If the result of attempting to resolve the function body was a DynamicValue, attempt to use
454+
// the foreignFunctionResolver if one is present. This could still potentially yield a usable
455+
// value.
456+
if (res instanceof DynamicValue && context.foreignFunctionResolver !== undefined) {
457+
const ffrExpr = context.foreignFunctionResolver(lhs, node.arguments);
458+
if (ffrExpr !== null) {
459+
// The foreign function resolver was able to extract an expression from this function. See
460+
// if that expression leads to a non-dynamic result.
461+
const ffrRes = this.visitFfrExpression(ffrExpr, context);
462+
if (!(ffrRes instanceof DynamicValue)) {
463+
// FFR yielded an actual result that's not dynamic, so use that instead of the original
464+
// resolution.
465+
res = ffrRes;
466+
}
454467
}
455-
return res;
456468
}
457469

458-
const body = fn.body;
459-
if (body.length !== 1 || !ts.isReturnStatement(body[0])) {
470+
return res;
471+
}
472+
473+
/**
474+
* Visit an expression which was extracted from a foreign-function resolver.
475+
*
476+
* This will process the result and ensure it's correct for FFR-resolved values, including marking
477+
* `Reference`s as synthetic.
478+
*/
479+
private visitFfrExpression(expr: ts.Expression, context: Context): ResolvedValue {
480+
const res = this.visitExpression(expr, context);
481+
if (res instanceof Reference) {
482+
// This Reference was created synthetically, via a foreign function resolver. The real
483+
// runtime value of the function expression may be different than the foreign function
484+
// resolved value, so mark the Reference as synthetic to avoid it being misinterpreted.
485+
res.synthetic = true;
486+
}
487+
return res;
488+
}
489+
490+
private visitFunctionBody(node: ts.CallExpression, fn: FunctionDefinition, context: Context):
491+
ResolvedValue {
492+
if (fn.body === null || fn.body.length !== 1 || !ts.isReturnStatement(fn.body[0])) {
460493
return DynamicValue.fromUnknown(node);
461494
}
462-
const ret = body[0] as ts.ReturnStatement;
495+
const ret = fn.body[0] as ts.ReturnStatement;
463496

464497
const args = this.evaluateFunctionArguments(node, context);
465498
const newScope: Scope = new Map<ts.ParameterDeclaration, ResolvedValue>();

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2390,6 +2390,45 @@ runInEachFileSystem(os => {
23902390
});
23912391

23922392
describe('unwrapping ModuleWithProviders functions', () => {
2393+
it('should use a local ModuleWithProviders-annotated return type if a function is not statically analyzable',
2394+
() => {
2395+
env.write(`module.ts`, `
2396+
import {NgModule, ModuleWithProviders} from '@angular/core';
2397+
2398+
export function notStaticallyAnalyzable(): ModuleWithProviders<SomeModule> {
2399+
console.log('this interferes with static analysis');
2400+
return {
2401+
ngModule: SomeModule,
2402+
providers: [],
2403+
};
2404+
}
2405+
2406+
@NgModule()
2407+
export class SomeModule {}
2408+
`);
2409+
2410+
env.write('test.ts', `
2411+
import {NgModule} from '@angular/core';
2412+
import {notStaticallyAnalyzable} from './module';
2413+
2414+
@NgModule({
2415+
imports: [notStaticallyAnalyzable()]
2416+
})
2417+
export class TestModule {}
2418+
`);
2419+
2420+
env.driveMain();
2421+
2422+
const jsContents = env.getContents('test.js');
2423+
expect(jsContents).toContain('imports: [notStaticallyAnalyzable()]');
2424+
2425+
const dtsContents = env.getContents('test.d.ts');
2426+
expect(dtsContents).toContain(`import * as i1 from "./module";`);
2427+
expect(dtsContents)
2428+
.toContain(
2429+
'i0.ɵɵNgModuleDefWithMeta<TestModule, never, [typeof i1.SomeModule], never>');
2430+
});
2431+
23932432
it('should extract the generic type and include it in the module\'s declaration', () => {
23942433
env.write(`test.ts`, `
23952434
import {NgModule} from '@angular/core';

0 commit comments

Comments
 (0)