fix(compiler-cli): use correct module import for types behind a `forwardRef` (#42887)

The static interpreter assumed that a foreign function expression would
have to be imported from the absolute module specifier that was used for
the foreign function itself. This assumption does not hold for the
`forwardRef` foreign function resolver, as that extracts the resolved
expression from the function's argument, which is not behind the
absolute module import of the `forwardRef` function.

The prior behavior has worked for the typical usage of `forwardRef`,
when it is contained within the same source file as where the static
evaluation started. In that case, the resulting reference would
incorrectly have an absolute module guess of `@angular/core`, but the
local identifier emit strategy was capable of emitting the reference
without generating an import using the absolute module guess.

In the scenario where the static interpreter would first have to follow
a reference to a different source that contained the `forwardRef` would
the compilation fail. In that case, there is no local identifier
available such that the absolute module emitter would try to locate the
imported symbol from `@angular/core`. which fails as the symbol is not
exported from there.

This commit fixes the issue by checking whether a foreign expression
occurs in the same source file as the call expression. If it does, then
the absolute module specifier that was used to resolve the call
expression is ignored.

Fixes #42865

PR Close #42887
This commit is contained in:
JoostK 2021-07-17 22:50:03 +02:00 committed by Alex Rickabaugh
parent ca7d4c3403
commit 70c3461be3
4 changed files with 162 additions and 6 deletions

View File

@ -463,13 +463,14 @@ export class StaticInterpreter {
node, DynamicValue.fromExternalReference(node.expression, lhs));
}
// If the function is declared in a different file, resolve the foreign function expression
// using the absolute module name of that file (if any).
if (lhs.bestGuessOwningModule !== null) {
// If the foreign expression occurs in a different file, then assume that the owning module
// of the call expression should also be used for the resolved foreign expression.
if (expr.getSourceFile() !== node.expression.getSourceFile() &&
lhs.bestGuessOwningModule !== null) {
context = {
...context,
absoluteModuleName: lhs.bestGuessOwningModule.specifier,
resolutionContext: node.getSourceFile().fileName,
resolutionContext: lhs.bestGuessOwningModule.resolutionContext,
};
}

View File

@ -14,10 +14,10 @@ import {DependencyTracker} from '../../incremental/api';
import {Declaration, DeclarationKind, isConcreteDeclaration, KnownDeclaration, SpecialDeclarationKind, TypeScriptReflectionHost} from '../../reflection';
import {getDeclaration, makeProgram} from '../../testing';
import {DynamicValue} from '../src/dynamic';
import {PartialEvaluator} from '../src/interface';
import {ForeignFunctionResolver, PartialEvaluator} from '../src/interface';
import {EnumValue, ResolvedValue} from '../src/result';
import {evaluate, firstArgFfr, makeEvaluator, makeExpression, owningModuleOf} from './utils';
import {arrowReturnValueFfr, evaluate, firstArgFfr, makeEvaluator, makeExpression, owningModuleOf, returnTypeFfr} from './utils';
runInEachFileSystem(() => {
describe('ngtsc metadata', () => {
@ -646,6 +646,96 @@ runInEachFileSystem(() => {
expect(id.text).toEqual('Target');
});
it('should not associate an owning module when a FFR-resolved expression is within the originating source file',
() => {
const resolved = evaluate(
`import {forwardRef} from 'forward';
class Foo {}`,
'forwardRef(() => Foo)', [{
name: _('/node_modules/forward/index.d.ts'),
contents: `export declare function forwardRef<T>(fn: () => T): T;`,
}],
arrowReturnValueFfr);
if (!(resolved instanceof Reference)) {
return fail('Expected expression to resolve to a reference');
}
expect((resolved.node as ts.ClassDeclaration).name!.text).toBe('Foo');
expect(resolved.bestGuessOwningModule).toBeNull();
});
it('should not associate an owning module when a FFR-resolved expression is imported using a relative import',
() => {
const resolved = evaluate(
`import {forwardRef} from 'forward';
import {Foo} from './foo';`,
'forwardRef(() => Foo)',
[
{
name: _('/node_modules/forward/index.d.ts'),
contents: `export declare function forwardRef<T>(fn: () => T): T;`,
},
{
name: _('/foo.ts'),
contents: `export class Foo {}`,
}
],
arrowReturnValueFfr);
if (!(resolved instanceof Reference)) {
return fail('Expected expression to resolve to a reference');
}
expect((resolved.node as ts.ClassDeclaration).name!.text).toBe('Foo');
expect(resolved.bestGuessOwningModule).toBeNull();
});
it('should associate an owning module when a FFR-resolved expression is imported using an absolute import',
() => {
const {expression, checker} = makeExpression(
`import {forwardRef} from 'forward';
import {Foo} from 'external';`,
`forwardRef(() => Foo)`, [
{
name: _('/node_modules/forward/index.d.ts'),
contents: `export declare function forwardRef<T>(fn: () => T): T;`,
},
{
name: _('/node_modules/external/index.d.ts'),
contents: `export declare class Foo {}`,
}
]);
const evaluator = makeEvaluator(checker);
const resolved = evaluator.evaluate(expression, arrowReturnValueFfr);
if (!(resolved instanceof Reference)) {
return fail('Expected expression to resolve to a reference');
}
expect((resolved.node as ts.ClassDeclaration).name!.text).toBe('Foo');
expect(resolved.bestGuessOwningModule).toEqual({
specifier: 'external',
resolutionContext: expression.getSourceFile().fileName,
});
});
it('should associate an owning module when a FFR-resolved expression is within the foreign file',
() => {
const {expression, checker} =
makeExpression(`import {external} from 'external';`, `external()`, [{
name: _('/node_modules/external/index.d.ts'),
contents: `
export declare class Foo {}
export declare function external(): Foo;
`
}]);
const evaluator = makeEvaluator(checker);
const resolved = evaluator.evaluate(expression, returnTypeFfr);
if (!(resolved instanceof Reference)) {
return fail('Expected expression to resolve to a reference');
}
expect((resolved.node as ts.ClassDeclaration).name!.text).toBe('Foo');
expect(resolved.bestGuessOwningModule).toEqual({
specifier: 'external',
resolutionContext: expression.getSourceFile().fileName,
});
});
it('should resolve functions with more than one statement to a complex function call', () => {
const value = evaluate(`function foo(bar) { const b = bar; return b; }`, 'foo("test")');

View File

@ -62,3 +62,14 @@ export function firstArgFfr(
args: ReadonlyArray<ts.Expression>): ts.Expression {
return args[0];
}
export const arrowReturnValueFfr: ForeignFunctionResolver = (_ref, args) => {
// Extracts the `Foo` from `() => Foo`.
return (args[0] as ts.ArrowFunction).body as ts.Expression;
};
export const returnTypeFfr: ForeignFunctionResolver = (ref) => {
// Extract the `Foo` from the return type of the `external` function declaration.
return ((ref.node as ts.FunctionDeclaration).type as ts.TypeReferenceNode).typeName as
ts.Identifier;
};

View File

@ -1558,6 +1558,60 @@ function allTests(os: string) {
expect(jsContents).toContain('exports: function () { return [BarModule]; }');
});
it('should use relative import for forward references that were resolved from a relative file',
() => {
env.write('dir.ts', `
import {Directive, forwardRef} from '@angular/core';
export const useFoo = forwardRef(() => Foo);
@Directive({selector: 'foo'})
export class Foo {}
`);
env.write('test.ts', `
import {NgModule} from '@angular/core';
import {useFoo} from './dir';
@NgModule({
declarations: [useFoo],
})
export class FooModule {}
`);
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('import * as i1 from "./dir";');
expect(jsContents).toContain('declarations: [i1.Foo]');
});
it('should use absolute import for forward references that were resolved from an absolute file',
() => {
env.write('dir.ts', `
import {Directive, forwardRef} from '@angular/core';
export const useFoo = forwardRef(() => Foo);
@Directive({selector: 'foo'})
export class Foo {}
`);
env.write('test.ts', `
import {forwardRef, NgModule} from '@angular/core';
import {useFoo} from 'dir';
@NgModule({
declarations: [useFoo],
})
export class FooModule {}
`);
env.driveMain();
const jsContents = env.getContents('test.js');
expect(jsContents).toContain('import * as i1 from "dir";');
expect(jsContents).toContain('declarations: [i1.Foo]');
});
it('should compile Pipes without errors', () => {
env.write('test.ts', `
import {Pipe} from '@angular/core';