fix(ngcc): resolve imports in `.d.ts` files for UMD/CommonJS bundles (#32619)

In ngcc's reflection host for UMD and CommonJS bundles, custom logic is
present to resolve import details of an identifier. However, this custom
logic is unable to resolve an import for an identifier inside of
declaration files, as such files use the regular ESM import syntax.

As a consequence of this limitation, ngtsc is unable to resolve
`ModuleWithProviders` imports that are declared in an external library.
In that situation, ngtsc determines the type of the actual `NgModule`
that is imported, by looking in the library's declaration files for the
generic type argument on `ModuleWithProviders`. In this process, ngtsc
resolves the import for the `ModuleWithProviders` identifier to verify
that it is indeed the `ModuleWithProviders` type from `@angular/core`.
So, when the UMD reflection host was in use this resolution would fail,
therefore no `NgModule` type could be detected.

This commit fixes the bug by using the regular import resolution logic
in addition to the custom resolution logic that is required for UMD
and CommonJS bundles.

Fixes #31791

PR Close #32619
This commit is contained in:
JoostK 2019-09-12 21:08:21 +02:00 committed by Kara Erickson
parent c4e039a43a
commit 3c7da767d8
4 changed files with 59 additions and 2 deletions

View File

@ -26,6 +26,11 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
}
getImportOfIdentifier(id: ts.Identifier): Import|null {
const superImport = super.getImportOfIdentifier(id);
if (superImport !== null) {
return superImport;
}
const requireCall = this.findCommonJsImport(id);
if (requireCall === null) {
return null;

View File

@ -25,6 +25,11 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
}
getImportOfIdentifier(id: ts.Identifier): Import|null {
const superImport = super.getImportOfIdentifier(id);
if (superImport !== null) {
return superImport;
}
const importParameter = this.findUmdImportParameter(id);
const from = importParameter && this.getUmdImportPath(importParameter);
return from !== null ? {from, name: id.text} : null;

View File

@ -9,7 +9,7 @@ import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {ClassMemberKind, CtorParameter, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {ClassMemberKind, CtorParameter, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {CommonJsReflectionHost} from '../../src/host/commonjs_host';
@ -1558,6 +1558,30 @@ exports.ExternalModule = ExternalModule;
expect(importOfIdent).toEqual({name: 'a', from: './file_a'});
});
it('should find the import of an identifier in a declaration file', () => {
loadTestFiles([
{
name: _('/index.d.ts'),
contents: `
import {MyClass} from './myclass.d.ts';
export declare const a: MyClass;`
},
{
name: _('/myclass.d.ts'),
contents: `export declare class MyClass {}`,
}
]);
const {program, host: compilerHost} = makeTestBundleProgram(_('/index.d.ts'));
const host = new CommonJsReflectionHost(new MockLogger(), false, program, compilerHost);
const variableNode =
getDeclaration(program, _('/index.d.ts'), 'a', isNamedVariableDeclaration);
const identifier =
((variableNode.type as ts.TypeReferenceNode).typeName as ts.Identifier);
const importOfIdent = host.getImportOfIdentifier(identifier !);
expect(importOfIdent).toEqual({name: 'MyClass', from: './myclass.d.ts'});
});
it('should return null if the identifier was not imported', () => {
loadTestFiles(IMPORTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/index.js'));

View File

@ -13,7 +13,6 @@ import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/test
import {ClassMemberKind, CtorParameter, Import, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
import {getIifeBody} from '../../src/host/esm5_host';
import {UmdReflectionHost} from '../../src/host/umd_host';
import {MockLogger} from '../helpers/mock_logger';
@ -1658,6 +1657,30 @@ runInEachFileSystem(() => {
expect(importOfIdent).toEqual({name: 'a', from: './file_a'});
});
it('should find the import of an identifier in a declaration file', () => {
loadTestFiles([
{
name: _('/index.d.ts'),
contents: `
import {MyClass} from './myclass.d.ts';
export declare const a: MyClass;`
},
{
name: _('/myclass.d.ts'),
contents: `export declare class MyClass {}`,
}
]);
const {program, host: compilerHost} = makeTestBundleProgram(_('/index.d.ts'));
const host = new UmdReflectionHost(new MockLogger(), false, program, compilerHost);
const variableNode =
getDeclaration(program, _('/index.d.ts'), 'a', isNamedVariableDeclaration);
const identifier =
((variableNode.type as ts.TypeReferenceNode).typeName as ts.Identifier);
const importOfIdent = host.getImportOfIdentifier(identifier !);
expect(importOfIdent).toEqual({name: 'MyClass', from: './myclass.d.ts'});
});
it('should return null if the identifier was not imported', () => {
loadTestFiles(IMPORTS_FILES);
const {program, host: compilerHost} = makeTestBundleProgram(_('/index.js'));