Skip to content
Permalink
Browse files

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
  • Loading branch information...
JoostK authored and kara committed Sep 12, 2019
1 parent c4e039a commit 3c7da767d8faa2dd231f57cf75708e9f942bb9fe
@@ -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;
@@ -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;
@@ -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'));
@@ -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'));

0 comments on commit 3c7da76

Please sign in to comment.
You can’t perform that action at this time.