From ca25c957bfd8faa7a67cd4b1ac32808dd5eca723 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 3 Apr 2020 16:37:46 +0300 Subject: [PATCH] fix(ngcc): correctly detect imported TypeScript helpers (#36284) The `NgccReflectionHost`s have logic for detecting certain known declarations (such as `Object.assign()` and TypeScript helpers), which allows the `PartialEvaluator` to evaluate expressions it would not be able to statically evaluate otherwise. In #36089, `DelegatingReflectionHost` was introduced, which delegates to a TypeScript `ReflectionHost` when reflecting on TypeScript files, which for ngcc's case means `.d.ts` files of dependencies. As a result, ngcc lost the ability to detect TypeScript helpers imported from `tslib`, because `DelegatingReflectionHost` was not able to apply the known declaration detection logic while reflecting on `tslib`'s `.d.ts` files. This commit fixes this by ensuring `DelegatingReflectionHost` calls the `NgccReflectionHost`'s `detectKnownDeclaration()` method as necessary, even when using the TypeScript `ReflectionHost`. NOTE: The previous commit exposed a bug in ngcc that was hidden due to the tests' being inconsistent with how the `ReflectionHost`s are used in the actual program. The changes in this commit are verified by ensuring the failing tests are now passing (hence no new tests are added). PR Close #36284 --- .../ngcc/src/host/delegating_host.ts | 17 +++++++++++++++-- .../compiler-cli/ngcc/src/host/ngcc_host.ts | 15 ++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/host/delegating_host.ts b/packages/compiler-cli/ngcc/src/host/delegating_host.ts index 3bef3e432d..d4d36fc96f 100644 --- a/packages/compiler-cli/ngcc/src/host/delegating_host.ts +++ b/packages/compiler-cli/ngcc/src/host/delegating_host.ts @@ -32,7 +32,7 @@ export class DelegatingReflectionHost implements NgccReflectionHost { getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null { if (isFromDtsFile(id)) { - return this.tsHost.getDeclarationOfIdentifier(id); + return this.detectKnownDeclaration(this.tsHost.getDeclarationOfIdentifier(id)); } return this.ngccHost.getDeclarationOfIdentifier(id); } @@ -60,7 +60,13 @@ export class DelegatingReflectionHost implements NgccReflectionHost { getExportsOfModule(module: ts.Node): Map|null { if (isFromDtsFile(module)) { - return this.tsHost.getExportsOfModule(module); + const exportMap = this.tsHost.getExportsOfModule(module); + + if (exportMap !== null) { + exportMap.forEach(decl => this.detectKnownDeclaration(decl)); + } + + return exportMap; } return this.ngccHost.getExportsOfModule(module); } @@ -154,4 +160,11 @@ export class DelegatingReflectionHost implements NgccReflectionHost { getEndOfClass(classSymbol: NgccClassSymbol): ts.Node { return this.ngccHost.getEndOfClass(classSymbol); } + + detectKnownDeclaration(decl: null): null; + detectKnownDeclaration(decl: T): T; + detectKnownDeclaration(decl: T|null): T|null; + detectKnownDeclaration(decl: T|null): T|null { + return this.ngccHost.detectKnownDeclaration(decl); + } } diff --git a/packages/compiler-cli/ngcc/src/host/ngcc_host.ts b/packages/compiler-cli/ngcc/src/host/ngcc_host.ts index e1f3c07fde..eb16f662ea 100644 --- a/packages/compiler-cli/ngcc/src/host/ngcc_host.ts +++ b/packages/compiler-cli/ngcc/src/host/ngcc_host.ts @@ -7,12 +7,12 @@ */ import * as ts from 'typescript'; -import {ClassDeclaration, ConcreteDeclaration, Decorator, ReflectionHost} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, ConcreteDeclaration, Declaration, Decorator, ReflectionHost} from '../../../src/ngtsc/reflection'; export const PRE_R3_MARKER = '__PRE_R3__'; export const POST_R3_MARKER = '__POST_R3__'; -export type SwitchableVariableDeclaration = ts.VariableDeclaration & {initializer: ts.Identifier}; +export type SwitchableVariableDeclaration = ts.VariableDeclaration&{initializer: ts.Identifier}; export function isSwitchableVariableDeclaration(node: ts.Node): node is SwitchableVariableDeclaration { return ts.isVariableDeclaration(node) && !!node.initializer && @@ -47,7 +47,7 @@ export interface ModuleWithProvidersFunction { * The symbol corresponding to a "class" declaration. I.e. a `ts.Symbol` whose `valueDeclaration` is * a `ClassDeclaration`. */ -export type ClassSymbol = ts.Symbol & {valueDeclaration: ClassDeclaration}; +export type ClassSymbol = ts.Symbol&{valueDeclaration: ClassDeclaration}; /** * A representation of a class that accounts for the potential existence of two `ClassSymbol`s for a @@ -128,4 +128,13 @@ export interface NgccReflectionHost extends ReflectionHost { * @param classSymbol The class whose statements we want. */ getEndOfClass(classSymbol: NgccClassSymbol): ts.Node; + + /** + * Check whether a `Declaration` corresponds with a known declaration and set its `known` property + * to the appropriate `KnownDeclaration`. + * + * @param decl The `Declaration` to check or `null` if there is no declaration. + * @return The passed in `Declaration` (potentially enhanced with a `KnownDeclaration`). + */ + detectKnownDeclaration(decl: T|null): T|null; }