Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ngcc): consistently delegate to TypeScript host for typing files #36089

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 1 addition & 7 deletions packages/compiler-cli/ngcc/src/host/commonjs_host.ts
Expand Up @@ -34,11 +34,6 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
}

getImportOfIdentifier(id: ts.Identifier): Import|null {
const superImport = super.getImportOfIdentifier(id);
if (superImport !== null) {
return superImport;
}

gkalpak marked this conversation as resolved.
Show resolved Hide resolved
const requireCall = this.findCommonJsImport(id);
if (requireCall === null) {
return null;
Expand All @@ -47,8 +42,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
}

getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
return (!id.getSourceFile().isDeclarationFile && this.getCommonJsImportedDeclaration(id)) ||
super.getDeclarationOfIdentifier(id);
return this.getCommonJsImportedDeclaration(id) || super.getDeclarationOfIdentifier(id);
}

getExportsOfModule(module: ts.Node): Map<string, Declaration>|null {
Expand Down
157 changes: 157 additions & 0 deletions packages/compiler-cli/ngcc/src/host/delegating_host.ts
@@ -0,0 +1,157 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import * as ts from 'typescript';

import {ClassDeclaration, ClassMember, CtorParameter, Declaration, Decorator, FunctionDefinition, Import, ReflectionHost} from '../../../src/ngtsc/reflection';
import {isFromDtsFile} from '../../../src/ngtsc/util/src/typescript';

import {ModuleWithProvidersFunction, NgccClassSymbol, NgccReflectionHost, SwitchableVariableDeclaration} from './ngcc_host';

/**
* A reflection host implementation that delegates reflector queries depending on whether they
* reflect on declaration files (for dependent libraries) or source files within the entry-point
* that is being compiled. The first type of queries are handled by the regular TypeScript
* reflection host, whereas the other queries are handled by an `NgccReflectionHost` that is
* specific to the entry-point's format.
*/
export class DelegatingReflectionHost implements NgccReflectionHost {
constructor(private tsHost: ReflectionHost, private ngccHost: NgccReflectionHost) {}

getConstructorParameters(clazz: ClassDeclaration): CtorParameter[]|null {
if (isFromDtsFile(clazz)) {
return this.tsHost.getConstructorParameters(clazz);
}
return this.ngccHost.getConstructorParameters(clazz);
}

getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
if (isFromDtsFile(id)) {
return this.tsHost.getDeclarationOfIdentifier(id);
}
return this.ngccHost.getDeclarationOfIdentifier(id);
}

getDecoratorsOfDeclaration(declaration: ts.Declaration): Decorator[]|null {
if (isFromDtsFile(declaration)) {
return this.tsHost.getDecoratorsOfDeclaration(declaration);
}
return this.ngccHost.getDecoratorsOfDeclaration(declaration);
}

getDefinitionOfFunction(fn: ts.Node): FunctionDefinition|null {
if (isFromDtsFile(fn)) {
return this.tsHost.getDefinitionOfFunction(fn);
}
return this.ngccHost.getDefinitionOfFunction(fn);
}

getDtsDeclaration(declaration: ts.Declaration): ts.Declaration|null {
if (isFromDtsFile(declaration)) {
return this.tsHost.getDtsDeclaration(declaration);
}
return this.ngccHost.getDtsDeclaration(declaration);
}

getExportsOfModule(module: ts.Node): Map<string, Declaration>|null {
if (isFromDtsFile(module)) {
return this.tsHost.getExportsOfModule(module);
}
return this.ngccHost.getExportsOfModule(module);
}

getGenericArityOfClass(clazz: ClassDeclaration): number|null {
if (isFromDtsFile(clazz)) {
return this.tsHost.getGenericArityOfClass(clazz);
}
return this.ngccHost.getGenericArityOfClass(clazz);
}

getImportOfIdentifier(id: ts.Identifier): Import|null {
if (isFromDtsFile(id)) {
return this.tsHost.getImportOfIdentifier(id);
}
return this.ngccHost.getImportOfIdentifier(id);
}

getInternalNameOfClass(clazz: ClassDeclaration): ts.Identifier {
if (isFromDtsFile(clazz)) {
return this.tsHost.getInternalNameOfClass(clazz);
}
return this.ngccHost.getInternalNameOfClass(clazz);
}

getAdjacentNameOfClass(clazz: ClassDeclaration): ts.Identifier {
if (isFromDtsFile(clazz)) {
return this.tsHost.getAdjacentNameOfClass(clazz);
}
return this.ngccHost.getAdjacentNameOfClass(clazz);
}

getMembersOfClass(clazz: ClassDeclaration): ClassMember[] {
if (isFromDtsFile(clazz)) {
return this.tsHost.getMembersOfClass(clazz);
}
return this.ngccHost.getMembersOfClass(clazz);
}

getVariableValue(declaration: ts.VariableDeclaration): ts.Expression|null {
if (isFromDtsFile(declaration)) {
return this.tsHost.getVariableValue(declaration);
}
return this.ngccHost.getVariableValue(declaration);
}

hasBaseClass(clazz: ClassDeclaration): boolean {
if (isFromDtsFile(clazz)) {
return this.tsHost.hasBaseClass(clazz);
}
return this.ngccHost.hasBaseClass(clazz);
}

getBaseClassExpression(clazz: ClassDeclaration): ts.Expression|null {
if (isFromDtsFile(clazz)) {
return this.tsHost.getBaseClassExpression(clazz);
}
return this.ngccHost.getBaseClassExpression(clazz);
}

isClass(node: ts.Node): node is ClassDeclaration {
if (isFromDtsFile(node)) {
return this.tsHost.isClass(node);
}
return this.ngccHost.isClass(node);
}

// Note: the methods below are specific to ngcc and the entry-point that is being compiled, so
// they don't take declaration files into account.

findClassSymbols(sourceFile: ts.SourceFile): NgccClassSymbol[] {
gkalpak marked this conversation as resolved.
Show resolved Hide resolved
return this.ngccHost.findClassSymbols(sourceFile);
}

getClassSymbol(node: ts.Node): NgccClassSymbol|undefined {
return this.ngccHost.getClassSymbol(node);
}

getDecoratorsOfSymbol(symbol: NgccClassSymbol): Decorator[]|null {
return this.ngccHost.getDecoratorsOfSymbol(symbol);
}

getModuleWithProvidersFunctions(sf: ts.SourceFile): ModuleWithProvidersFunction[] {
return this.ngccHost.getModuleWithProvidersFunctions(sf);
}

getSwitchableDeclarations(module: ts.Node): SwitchableVariableDeclaration[] {
return this.ngccHost.getSwitchableDeclarations(module);
}

getEndOfClass(classSymbol: NgccClassSymbol): ts.Node {
return this.ngccHost.getEndOfClass(classSymbol);
}
}
7 changes: 0 additions & 7 deletions packages/compiler-cli/ngcc/src/host/esm5_host.ts
Expand Up @@ -41,8 +41,6 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
* @param clazz a `ClassDeclaration` representing the class over which to reflect.
*/
hasBaseClass(clazz: ClassDeclaration): boolean {
if (super.hasBaseClass(clazz)) return true;

const classSymbol = this.getClassSymbol(clazz);
if (classSymbol === undefined) {
return false;
Expand All @@ -58,11 +56,6 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
}

getBaseClassExpression(clazz: ClassDeclaration): ts.Expression|null {
const superBaseClassIdentifier = super.getBaseClassExpression(clazz);
if (superBaseClassIdentifier) {
return superBaseClassIdentifier;
}

const classSymbol = this.getClassSymbol(clazz);
if (classSymbol === undefined) {
return null;
Expand Down
8 changes: 1 addition & 7 deletions packages/compiler-cli/ngcc/src/host/umd_host.ts
Expand Up @@ -33,11 +33,6 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
}

getImportOfIdentifier(id: ts.Identifier): Import|null {
const superImport = super.getImportOfIdentifier(id);
if (superImport !== null) {
return superImport;
}

// Is `id` a namespaced property access, e.g. `Directive` in `core.Directive`?
// If so capture the symbol of the namespace, e.g. `core`.
const nsIdentifier = findNamespaceOfIdentifier(id);
Expand All @@ -47,8 +42,7 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
}

getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
return (!id.getSourceFile().isDeclarationFile && this.getUmdImportedDeclaration(id)) ||
super.getDeclarationOfIdentifier(id);
return this.getUmdImportedDeclaration(id) || super.getDeclarationOfIdentifier(id);
}

getExportsOfModule(module: ts.Node): Map<string, Declaration>|null {
Expand Down
8 changes: 6 additions & 2 deletions packages/compiler-cli/ngcc/src/packages/transformer.ts
Expand Up @@ -8,13 +8,15 @@
import * as ts from 'typescript';

import {FileSystem} from '../../../src/ngtsc/file_system';
import {TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
import {DecorationAnalyzer} from '../analysis/decoration_analyzer';
import {ModuleWithProvidersAnalyses, ModuleWithProvidersAnalyzer} from '../analysis/module_with_providers_analyzer';
import {NgccReferencesRegistry} from '../analysis/ngcc_references_registry';
import {ExportInfo, PrivateDeclarationsAnalyzer} from '../analysis/private_declarations_analyzer';
import {SwitchMarkerAnalyses, SwitchMarkerAnalyzer} from '../analysis/switch_marker_analyzer';
import {CompiledFile} from '../analysis/types';
import {CommonJsReflectionHost} from '../host/commonjs_host';
import {DelegatingReflectionHost} from '../host/delegating_host';
import {Esm2015ReflectionHost} from '../host/esm2015_host';
import {Esm5ReflectionHost} from '../host/esm5_host';
import {NgccReflectionHost} from '../host/ngcc_host';
Expand Down Expand Up @@ -69,7 +71,9 @@ export class Transformer {
* @returns information about the files that were transformed.
*/
transform(bundle: EntryPointBundle): TransformResult {
const reflectionHost = this.getHost(bundle);
const ngccReflectionHost = this.getHost(bundle);
const tsReflectionHost = new TypeScriptReflectionHost(bundle.src.program.getTypeChecker());
const reflectionHost = new DelegatingReflectionHost(tsReflectionHost, ngccReflectionHost);

// Parse and analyze the files.
const {decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses,
Expand All @@ -81,7 +85,7 @@ export class Transformer {
}

// Transform the source files and source maps.
const srcFormatter = this.getRenderingFormatter(reflectionHost, bundle);
const srcFormatter = this.getRenderingFormatter(ngccReflectionHost, bundle);

const renderer = new Renderer(reflectionHost, srcFormatter, this.fs, this.logger, bundle);
let renderedFiles = renderer.renderProgram(
Expand Down
57 changes: 0 additions & 57 deletions packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
Expand Up @@ -1607,30 +1607,6 @@ 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 bundle = makeTestBundleProgram(_('/index.d.ts'));
const host = new CommonJsReflectionHost(new MockLogger(), false, bundle);
const variableNode =
getDeclaration(bundle.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 bundle = makeTestBundleProgram(_('/index.js'));
Expand Down Expand Up @@ -1816,39 +1792,6 @@ exports.ExternalModule = ExternalModule;
expect(importOfIdent.viaModule).toBe('lib');
});

it('should return the correct declaration of an identifier imported in a typings file',
() => {
const files = [
{
name: _('/node_modules/test-package/index.d.ts'),
contents: `
import {SubModule} from 'sub_module';
export const x = SubModule;
`,
},
{
name: _('/node_modules/sub_module/index.d.ts'),
contents: 'export class SubModule {}',
}
];
loadTestFiles(files);
const bundle = makeTestBundleProgram(files[0].name);
const host = new CommonJsReflectionHost(new MockLogger(), false, bundle);
const expectedDeclaration = getDeclaration(
bundle.program, files[1].name, 'SubModule', isNamedClassDeclaration);
const x =
getDeclaration(bundle.program, files[0].name, 'x', isNamedVariableDeclaration);
if (x.initializer === undefined || !ts.isIdentifier(x.initializer)) {
return fail('Expected constant `x` to have an identifer as an initializer.');
}
const decl = host.getDeclarationOfIdentifier(x.initializer);
if (decl === null) {
return fail('Expected to find a declaration for ' + x.initializer.getText());
}
expect(decl.viaModule).toEqual('sub_module');
expect(decl.node).toBe(expectedDeclaration);
});

gkalpak marked this conversation as resolved.
Show resolved Hide resolved
it('should recognize TypeScript helpers (as function declarations)', () => {
const file: TestFile = {
name: _('/test.js'),
Expand Down
56 changes: 0 additions & 56 deletions packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
Expand Up @@ -1776,29 +1776,6 @@ 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 bundle = makeTestBundleProgram(_('/index.d.ts'));
const host = new UmdReflectionHost(new MockLogger(), false, bundle);
const variableNode =
getDeclaration(bundle.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 bundle = makeTestBundleProgram(_('/index.js'));
Expand Down Expand Up @@ -1942,39 +1919,6 @@ runInEachFileSystem(() => {
expect(actualDeclaration !.viaModule).toBe('@angular/core');
});

it('should return the correct declaration of an identifier imported in a typings file',
() => {

const FILES = [
{
name: _('/node_modules/test-package/index.d.ts'),
contents: `
import {SubModule} from 'sub_module';
export const x = SubModule;
`,
},
{
name: _('/node_modules/sub_module/index.d.ts'),
contents: `export class SubModule {}`,
}
];
loadTestFiles(FILES);
const bundle = makeTestBundleProgram(FILES[0].name);
const host = new UmdReflectionHost(new MockLogger(), false, bundle);
const expectedDeclaration =
getDeclaration(bundle.program, FILES[1].name, 'SubModule', isNamedClassDeclaration);
const x = getDeclaration(bundle.program, FILES[0].name, 'x', isNamedVariableDeclaration);
if (x.initializer === undefined || !ts.isIdentifier(x.initializer)) {
return fail('Expected constant `x` to have an identifer as an initializer.');
}
const decl = host.getDeclarationOfIdentifier(x.initializer);
if (decl === null) {
return fail('Expected to find a declaration for ' + x.initializer.getText());
}
expect(decl.viaModule).toEqual('sub_module');
expect(decl.node).toBe(expectedDeclaration);
});
gkalpak marked this conversation as resolved.
Show resolved Hide resolved

it('should recognize TypeScript helpers (as function declarations)', () => {
const file: TestFile = {
name: _('/test.js'),
Expand Down