diff --git a/packages/compiler-cli/src/ngtsc/imports/src/references.ts b/packages/compiler-cli/src/ngtsc/imports/src/references.ts index c597e75b95..78101a2ba5 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/references.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/references.ts @@ -49,6 +49,14 @@ export class Reference { private identifiers: ts.Identifier[] = []; + /** + * Indicates that the Reference was created synthetically, not as a result of natural value + * resolution. + * + * This is used to avoid misinterpreting the Reference in certain contexts. + */ + synthetic = false; + private _alias: Expression|null = null; constructor(readonly node: T, bestGuessOwningModule: OwningModule|null = null) { diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts index 001e813b1d..138de061ba 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -218,7 +218,12 @@ export class StaticInterpreter { const result = this.visitDeclaration(decl.node, {...context, ...joinModuleContext(context, node, decl)}); if (result instanceof Reference) { - result.addIdentifier(node); + // Only record identifiers to non-synthetic references. Synthetic references may not have the + // same value at runtime as they do at compile time, so it's not legal to refer to them by the + // identifier here. + if (!result.synthetic) { + result.addIdentifier(node); + } } else if (result instanceof DynamicValue) { return DynamicValue.fromDynamicInput(node, result); } @@ -404,7 +409,14 @@ export class StaticInterpreter { }; } - return this.visitExpression(expr, context); + 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; } const body = fn.body; diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts index f434dea4cc..65c270dc9b 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts @@ -12,7 +12,7 @@ import {Reference} from '../../imports'; import {TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; import {DynamicValue} from '../src/dynamic'; -import {PartialEvaluator} from '../src/interface'; +import {ForeignFunctionResolver, PartialEvaluator} from '../src/interface'; import {EnumValue, ResolvedValue} from '../src/result'; function makeSimpleProgram(contents: string): ts.Program { @@ -41,11 +41,12 @@ function makeExpression( } function evaluate( - code: string, expr: string, supportingFiles: {name: string, contents: string}[] = []): T { + code: string, expr: string, supportingFiles: {name: string, contents: string}[] = [], + foreignFunctionResolver?: ForeignFunctionResolver): T { const {expression, checker, program, options, host} = makeExpression(code, expr, supportingFiles); const reflectionHost = new TypeScriptReflectionHost(checker); const evaluator = new PartialEvaluator(reflectionHost, checker); - return evaluator.evaluate(expression) as T; + return evaluator.evaluate(expression, foreignFunctionResolver) as T; } describe('ngtsc metadata', () => { @@ -313,8 +314,34 @@ describe('ngtsc metadata', () => { evaluate(`enum Test { VALUE = 'test', } const value = \`a.\${Test.VALUE}.b\`;`, 'value'); expect(value).toBe('a.test.b'); }); + + it('should not attach identifiers to FFR-resolved values', () => { + const value = evaluate( + ` + declare function foo(arg: any): any; + class Target {} + + const indir = foo(Target); + const value = indir; + `, + 'value', [], firstArgFfr); + if (!(value instanceof Reference)) { + return fail('Expected value to be a Reference'); + } + const id = value.getIdentityIn(value.node.getSourceFile()); + if (id === null) { + return fail('Expected value to have an identity'); + } + expect(id.text).toEqual('Target'); + }); }); function owningModuleOf(ref: Reference): string|null { return ref.bestGuessOwningModule !== null ? ref.bestGuessOwningModule.specifier : null; } + +function firstArgFfr( + node: Reference, + args: ReadonlyArray): ts.Expression { + return args[0]; +} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index d55682106a..9535dbf276 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -1251,6 +1251,36 @@ describe('ngtsc behavioral tests', () => { .toContain( 'i0.ɵNgModuleDefWithMeta'); }); + + it('should not reference a constant with a ModuleWithProviders value in ngModuleDef imports', + () => { + env.tsconfig(); + env.write('dep.d.ts', ` + import {ModuleWithProviders, ɵNgModuleDefWithMeta as NgModuleDefWithMeta} from '@angular/core'; + + export declare class DepModule { + static forRoot(arg1: any, arg2: any): ModuleWithProviders; + static ngModuleDef: NgModuleDefWithMeta; + } + `); + env.write('test.ts', ` + import {NgModule, ModuleWithProviders} from '@angular/core'; + import {DepModule} from './dep'; + + @NgModule({}) + export class Base {} + + const mwp = DepModule.forRoot(1,2); + + @NgModule({ + imports: [mwp], + }) + export class Module {} + `); + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents).toContain('imports: [i1.DepModule]'); + }); }); it('should unwrap a ModuleWithProviders-like function if a matching literal type is provided for it',