From 02bab8cf90bba06ee723e87938e436918dc86bb0 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Tue, 13 Aug 2019 16:08:53 -0700 Subject: [PATCH] fix(ivy): in ngcc, handle inline exports in commonjs code (#32129) One of the compiler's tasks is to enumerate the exports of a given ES module. This can happen for example to resolve `foo.bar` where `foo` is a namespace import: ```typescript import * as foo from './foo'; @NgModule({ directives: [foo.DIRECTIVES], }) ``` In this case, the compiler must enumerate the exports of `foo.ts` in order to evaluate the expression `foo.DIRECTIVES`. When this operation occurs under ngcc, it must deal with the different module formats and types of exports that occur. In commonjs code, a problem arises when certain exports are downleveled. ```typescript export const DIRECTIVES = [ FooDir, BarDir, ]; ``` can be downleveled to: ```javascript exports.DIRECTIVES = [ FooDir, BarDir, ``` Previously, ngtsc and ngcc expected that any export would have an associated `ts.Declaration` node. `export class`, `export function`, etc. all retain `ts.Declaration`s even when downleveled. But the `export const` construct above does not. Therefore, ngcc would not detect `DIRECTIVES` as an export of `foo.ts`, and the evaluation of `foo.DIRECTIVES` would therefore fail. To solve this problem, the core concept of an exported `Declaration` according to the `ReflectionHost` API is split into a `ConcreteDeclaration` which has a `ts.Declaration`, and an `InlineDeclaration` which instead has a `ts.Expression`. Differentiating between these allows ngcc to return an `InlineDeclaration` for `DIRECTIVES` and correctly keep track of this export. PR Close #32129 --- .../module_with_providers_analyzer.ts | 4 +- .../src/analysis/ngcc_references_registry.ts | 8 +-- .../analysis/private_declarations_analyzer.ts | 8 +-- .../ngcc/src/host/commonjs_host.ts | 32 +++++++---- .../ngcc/src/host/esm2015_host.ts | 15 +++-- .../compiler-cli/ngcc/src/host/esm5_host.ts | 2 +- .../compiler-cli/ngcc/src/host/ngcc_host.ts | 5 +- .../undecorated_parent_migration.ts | 2 +- .../ngcc/test/host/commonjs_host_spec.ts | 32 ++++++++++- .../ngcc/test/host/esm2015_host_spec.ts | 5 +- .../test/host/esm5_host_import_helper_spec.ts | 2 +- .../ngcc/test/host/esm5_host_spec.ts | 5 +- .../ngcc/test/host/umd_host_spec.ts | 2 +- .../src/ngtsc/imports/src/emitter.ts | 8 ++- .../partial_evaluator/src/interpreter.ts | 24 +++++--- .../src/ngtsc/reflection/src/host.ts | 57 +++++++++++++++---- .../src/ngtsc/reflection/test/ts_host_spec.ts | 5 +- 17 files changed, 157 insertions(+), 59 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts index 6de29258cf..1f8550fdf1 100644 --- a/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {ReferencesRegistry} from '../../../src/ngtsc/annotations'; import {Reference} from '../../../src/ngtsc/imports'; -import {ClassDeclaration, Declaration} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, ConcreteDeclaration} from '../../../src/ngtsc/reflection'; import {ModuleWithProvidersFunction, NgccReflectionHost} from '../host/ngcc_host'; import {hasNameIdentifier, isDefined} from '../utils'; @@ -23,7 +23,7 @@ export interface ModuleWithProvidersInfo { /** * The NgModule class declaration (in the .d.ts file) to add as a type parameter. */ - ngModule: Declaration; + ngModule: ConcreteDeclaration; } export type ModuleWithProvidersAnalyses = Map; diff --git a/packages/compiler-cli/ngcc/src/analysis/ngcc_references_registry.ts b/packages/compiler-cli/ngcc/src/analysis/ngcc_references_registry.ts index ddfd746b28..ba8e0dbb2d 100644 --- a/packages/compiler-cli/ngcc/src/analysis/ngcc_references_registry.ts +++ b/packages/compiler-cli/ngcc/src/analysis/ngcc_references_registry.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {ReferencesRegistry} from '../../../src/ngtsc/annotations'; import {Reference} from '../../../src/ngtsc/imports'; -import {Declaration, ReflectionHost} from '../../../src/ngtsc/reflection'; +import {ConcreteDeclaration, ReflectionHost} from '../../../src/ngtsc/reflection'; import {hasNameIdentifier} from '../utils'; /** @@ -20,7 +20,7 @@ import {hasNameIdentifier} from '../utils'; * from libraries that are compiled by ngcc. */ export class NgccReferencesRegistry implements ReferencesRegistry { - private map = new Map(); + private map = new Map(); constructor(private host: ReflectionHost) {} @@ -34,7 +34,7 @@ export class NgccReferencesRegistry implements ReferencesRegistry { // Only store relative references. We are not interested in literals. if (ref.bestGuessOwningModule === null && hasNameIdentifier(ref.node)) { const declaration = this.host.getDeclarationOfIdentifier(ref.node.name); - if (declaration && hasNameIdentifier(declaration.node)) { + if (declaration && declaration.node !== null && hasNameIdentifier(declaration.node)) { this.map.set(declaration.node.name, declaration); } } @@ -45,5 +45,5 @@ export class NgccReferencesRegistry implements ReferencesRegistry { * Create and return a mapping for the registered resolved references. * @returns A map of reference identifiers to reference declarations. */ - getDeclarationMap(): Map { return this.map; } + getDeclarationMap(): Map { return this.map; } } diff --git a/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts index 53fc3e60d2..b7061d7d6d 100644 --- a/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; import {AbsoluteFsPath, absoluteFromSourceFile} from '../../../src/ngtsc/file_system'; -import {Declaration} from '../../../src/ngtsc/reflection'; +import {ConcreteDeclaration} from '../../../src/ngtsc/reflection'; import {NgccReflectionHost} from '../host/ngcc_host'; import {hasNameIdentifier, isDefined} from '../utils'; import {NgccReferencesRegistry} from './ngcc_references_registry'; @@ -40,15 +40,15 @@ export class PrivateDeclarationsAnalyzer { private getPrivateDeclarations( rootFiles: ts.SourceFile[], - declarations: Map): PrivateDeclarationsAnalyses { - const privateDeclarations: Map = new Map(declarations); + declarations: Map): PrivateDeclarationsAnalyses { + const privateDeclarations: Map = new Map(declarations); const exportAliasDeclarations: Map = new Map(); rootFiles.forEach(f => { const exports = this.host.getExportsOfModule(f); if (exports) { exports.forEach((declaration, exportedName) => { - if (hasNameIdentifier(declaration.node)) { + if (declaration.node !== null && hasNameIdentifier(declaration.node)) { if (privateDeclarations.has(declaration.node.name)) { const privateDeclaration = privateDeclarations.get(declaration.node.name) !; if (privateDeclaration.node !== declaration.node) { diff --git a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts index 08b549efb2..9b4ca11f30 100644 --- a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts +++ b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts @@ -92,9 +92,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { for (const statement of this.getModuleStatements(sourceFile)) { if (isCommonJsExportStatement(statement)) { const exportDeclaration = this.extractCommonJsExportDeclaration(statement); - if (exportDeclaration !== null) { - moduleMap.set(exportDeclaration.name, exportDeclaration.declaration); - } + moduleMap.set(exportDeclaration.name, exportDeclaration.declaration); } else if (isReexportStatement(statement)) { const reexports = this.extractCommonJsReexports(statement, sourceFile); for (const reexport of reexports) { @@ -106,14 +104,22 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { } private extractCommonJsExportDeclaration(statement: CommonJsExportStatement): - CommonJsExportDeclaration|null { + CommonJsExportDeclaration { const exportExpression = statement.expression.right; const declaration = this.getDeclarationOfExpression(exportExpression); - if (declaration === null) { - return null; - } const name = statement.expression.left.name.text; - return {name, declaration}; + if (declaration !== null) { + return {name, declaration}; + } else { + return { + name, + declaration: { + node: null, + expression: exportExpression, + viaModule: null, + }, + }; + } } private extractCommonJsReexports(statement: ReexportStatement, containingFile: ts.SourceFile): @@ -126,8 +132,14 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { const viaModule = stripExtension(importedFile.fileName); const importedExports = this.getExportsOfModule(importedFile); if (importedExports !== null) { - importedExports.forEach( - (decl, name) => reexports.push({name, declaration: {node: decl.node, viaModule}})); + importedExports.forEach((decl, name) => { + if (decl.node !== null) { + reexports.push({name, declaration: {node: decl.node, viaModule}}); + } else { + reexports.push( + {name, declaration: {node: null, expression: decl.expression, viaModule}}); + } + }); } } return reexports; diff --git a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts index 8a26ddb23d..8f59dda3ef 100644 --- a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../../src/ngtsc/file_system'; -import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, CtorParameter, Declaration, Decorator, TypeScriptReflectionHost, isDecoratorIdentifier, reflectObjectLiteral} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, ConcreteDeclaration, CtorParameter, Declaration, Decorator, TypeScriptReflectionHost, isDecoratorIdentifier, reflectObjectLiteral} from '../../../src/ngtsc/reflection'; import {Logger} from '../logging/logger'; import {BundleProgram} from '../packages/bundle_program'; import {findAll, getNameText, hasNameIdentifier, isDefined, stripDollarSuffix} from '../utils'; @@ -243,7 +243,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N // The identifier may have been of an additional class assignment such as `MyClass_1` that was // present as alias for `MyClass`. If so, resolve such aliases to their original declaration. - if (superDeclaration !== null) { + if (superDeclaration !== null && superDeclaration.node !== null) { const aliasedIdentifier = this.resolveAliasedClassIdentifier(superDeclaration.node); if (aliasedIdentifier !== null) { return this.getDeclarationOfIdentifier(aliasedIdentifier); @@ -409,6 +409,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N if (!exports) return []; const infos: ModuleWithProvidersFunction[] = []; exports.forEach((declaration, name) => { + if (declaration.node === null) { + return; + } if (this.isClass(declaration.node)) { this.getMembersOfClass(declaration.node).forEach(member => { if (member.isStatic) { @@ -497,7 +500,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N const aliasedIdentifier = initializer.left; const aliasedDeclaration = this.getDeclarationOfIdentifier(aliasedIdentifier); - if (aliasedDeclaration === null) { + if (aliasedDeclaration === null || aliasedDeclaration.node === null) { throw new Error( `Unable to locate declaration of ${aliasedIdentifier.text} in "${statement.getText()}"`); } @@ -1407,7 +1410,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } const ngModuleDeclaration = this.getDeclarationOfExpression(ngModuleValue); - if (!ngModuleDeclaration) { + if (!ngModuleDeclaration || ngModuleDeclaration.node === null) { throw new Error( `Cannot find a declaration for NgModule ${ngModuleValue.getText()} referenced in "${declaration!.getText()}"`); } @@ -1416,7 +1419,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } return { name, - ngModule: ngModuleDeclaration as Declaration, declaration, container + ngModule: ngModuleDeclaration as ConcreteDeclaration, declaration, container }; } @@ -1430,7 +1433,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } const namespaceDecl = this.getDeclarationOfIdentifier(expression.expression); - if (!namespaceDecl || !ts.isSourceFile(namespaceDecl.node)) { + if (!namespaceDecl || namespaceDecl.node === null || !ts.isSourceFile(namespaceDecl.node)) { return null; } diff --git a/packages/compiler-cli/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/ngcc/src/host/esm5_host.ts index 723db3dc79..7c425b541c 100644 --- a/packages/compiler-cli/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm5_host.ts @@ -140,7 +140,7 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { const outerClassNode = getClassDeclarationFromInnerFunctionDeclaration(id.parent); const declaration = super.getDeclarationOfIdentifier(outerClassNode ? outerClassNode.name : id); - if (!declaration || !ts.isVariableDeclaration(declaration.node) || + if (!declaration || declaration.node === null || !ts.isVariableDeclaration(declaration.node) || declaration.node.initializer !== undefined || // VariableDeclaration => VariableDeclarationList => VariableStatement => IIFE Block !ts.isBlock(declaration.node.parent.parent.parent)) { diff --git a/packages/compiler-cli/ngcc/src/host/ngcc_host.ts b/packages/compiler-cli/ngcc/src/host/ngcc_host.ts index 1730b1b029..79bf887626 100644 --- a/packages/compiler-cli/ngcc/src/host/ngcc_host.ts +++ b/packages/compiler-cli/ngcc/src/host/ngcc_host.ts @@ -6,7 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; -import {ClassDeclaration, ClassSymbol, Declaration, Decorator, ReflectionHost} from '../../../src/ngtsc/reflection'; + +import {ClassDeclaration, ClassSymbol, ConcreteDeclaration, Decorator, ReflectionHost} from '../../../src/ngtsc/reflection'; export const PRE_R3_MARKER = '__PRE_R3__'; export const POST_R3_MARKER = '__POST_R3__'; @@ -39,7 +40,7 @@ export interface ModuleWithProvidersFunction { * The declaration of the class that the `ngModule` property on the `ModuleWithProviders` object * refers to. */ - ngModule: Declaration; + ngModule: ConcreteDeclaration; } /** diff --git a/packages/compiler-cli/ngcc/src/migrations/undecorated_parent_migration.ts b/packages/compiler-cli/ngcc/src/migrations/undecorated_parent_migration.ts index a49ac6f042..255febb016 100644 --- a/packages/compiler-cli/ngcc/src/migrations/undecorated_parent_migration.ts +++ b/packages/compiler-cli/ngcc/src/migrations/undecorated_parent_migration.ts @@ -72,7 +72,7 @@ export class UndecoratedParentMigration implements Migration { } const baseClazz = host.reflectionHost.getDeclarationOfIdentifier(baseClassExpr) !.node; - if (!isClassDeclaration(baseClazz)) { + if (baseClazz === null || !isClassDeclaration(baseClazz)) { return null; } diff --git a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts index 3e1a062139..331b453dbc 100644 --- a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts @@ -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, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection'; +import {ClassMemberKind, CtorParameter, Import, 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'; @@ -31,6 +31,7 @@ runInEachFileSystem(() => { let SIMPLE_ES2015_CLASS_FILE: TestFile; let SIMPLE_CLASS_FILE: TestFile; let FOO_FUNCTION_FILE: TestFile; + let INLINE_EXPORT_FILE: TestFile; let INVALID_DECORATORS_FILE: TestFile; let INVALID_DECORATOR_ARGS_FILE: TestFile; let INVALID_PROP_DECORATORS_FILE: TestFile; @@ -164,6 +165,18 @@ exports.foo = foo; `, }; + INLINE_EXPORT_FILE = { + name: _('/inline_export.js'), + contents: ` +var core = require('@angular/core'); +function foo() {} +foo.decorators = [ + { type: core.Directive, args: [{ selector: '[ignored]' },] } +]; +exports.directives = [foo]; +`, + }; + INVALID_DECORATORS_FILE = { name: _('/invalid_decorators.js'), contents: ` @@ -1629,7 +1642,7 @@ exports.ExternalModule = ExternalModule; const exportDeclarations = host.getExportsOfModule(file); expect(exportDeclarations).not.toBe(null); expect(Array.from(exportDeclarations !.entries()) - .map(entry => [entry[0], entry[1].node.getText(), entry[1].viaModule])) + .map(entry => [entry[0], entry[1].node !.getText(), entry[1].viaModule])) .toEqual([ ['Directive', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'], ['a', `a = 'a'`, './a_module'], @@ -1655,7 +1668,7 @@ exports.ExternalModule = ExternalModule; const exportDeclarations = host.getExportsOfModule(file); expect(exportDeclarations).not.toBe(null); expect(Array.from(exportDeclarations !.entries()) - .map(entry => [entry[0], entry[1].node.getText(), entry[1].viaModule])) + .map(entry => [entry[0], entry[1].node !.getText(), entry[1].viaModule])) .toEqual([ ['Directive', `Directive: FnWithArg<(clazz: any) => any>`, _('/b_module')], ['a', `a = 'a'`, _('/b_module')], @@ -1673,6 +1686,19 @@ exports.ExternalModule = ExternalModule; ['xtra2', `xtra2 = 'xtra2'`, _('/xtra_module')], ]); }); + + it('should handle inline exports', () => { + loadTestFiles([INLINE_EXPORT_FILE]); + const {program, host: compilerHost} = makeTestBundleProgram(_('/inline_export.js')); + const host = new CommonJsReflectionHost(new MockLogger(), false, program, compilerHost); + const file = getSourceFileOrError(program, _('/inline_export.js')); + const exportDeclarations = host.getExportsOfModule(file); + expect(exportDeclarations).not.toBeNull(); + const decl = exportDeclarations !.get('directives') as InlineDeclaration; + expect(decl).not.toBeUndefined(); + expect(decl.node).toBeNull(); + expect(decl.expression).toBeDefined(); + }); }); describe('getClassSymbol()', () => { diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts index 2064b944e6..206e93b018 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts @@ -1538,8 +1538,9 @@ runInEachFileSystem(() => { 'SomeClass', ]); - const values = Array.from(exportDeclarations !.values()) - .map(declaration => [declaration.node.getText(), declaration.viaModule]); + const values = + Array.from(exportDeclarations !.values()) + .map(declaration => [declaration.node !.getText(), declaration.viaModule]); expect(values).toEqual([ [`Directive: FnWithArg<(clazz: any) => any>`, null], [`a = 'a'`, null], diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts index 900a46c5cb..1d6aa4c66e 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts @@ -399,7 +399,7 @@ export { SomeDirective }; const declaration = host.getDeclarationOfIdentifier(ngModuleRef !); expect(declaration).not.toBe(null); - expect(declaration !.node.getText()).toContain('function HttpClientXsrfModule()'); + expect(declaration !.node !.getText()).toContain('function HttpClientXsrfModule()'); }); }); describe('getVariableValue', () => { diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts index e475213324..2745d649c6 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts @@ -1842,8 +1842,9 @@ runInEachFileSystem(() => { 'SomeClass', ]); - const values = Array.from(exportDeclarations !.values()) - .map(declaration => [declaration.node.getText(), declaration.viaModule]); + const values = + Array.from(exportDeclarations !.values()) + .map(declaration => [declaration.node !.getText(), declaration.viaModule]); expect(values).toEqual([ [`Directive: FnWithArg<(clazz: any) => any>`, null], [`a = 'a'`, null], diff --git a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts index 0791c6da37..f0e69157b4 100644 --- a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts @@ -1741,7 +1741,7 @@ runInEachFileSystem(() => { const exportDeclarations = host.getExportsOfModule(file); expect(exportDeclarations).not.toBe(null); expect(Array.from(exportDeclarations !.entries()) - .map(entry => [entry[0], entry[1].node.getText(), entry[1].viaModule])) + .map(entry => [entry[0], entry[1].node !.getText(), entry[1].viaModule])) .toEqual([ ['Directive', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'], ['a', `a = 'a'`, '/a_module'], diff --git a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts index 4eb2c7d69e..dac28d7845 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts @@ -176,7 +176,13 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { return null; } const exportMap = new Map(); - exports.forEach((declaration, name) => { exportMap.set(declaration.node, name); }); + exports.forEach((declaration, name) => { + // It's okay to skip inline declarations, since by definition they're not target-able with a + // ts.Declaration anyway. + if (declaration.node !== null) { + exportMap.set(declaration.node, name); + } + }); return exportMap; } } diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts index ec147edf72..87544a61be 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {Reference} from '../../imports'; import {OwningModule} from '../../imports/src/references'; -import {Declaration, ReflectionHost} from '../../reflection'; +import {Declaration, InlineDeclaration, ReflectionHost} from '../../reflection'; import {isDeclaration} from '../../util/src/typescript'; import {ArrayConcatBuiltinFn, ArraySliceBuiltinFn} from './builtin'; @@ -20,6 +20,7 @@ import {BuiltinFn, EnumValue, ResolvedValue, ResolvedValueArray, ResolvedValueMa import {evaluateTsHelperInline} from './ts_helpers'; + /** * Tracks the scope of a function body, which includes `ResolvedValue`s for the parameters of that * body. @@ -223,8 +224,13 @@ export class StaticInterpreter { return DynamicValue.fromUnknownIdentifier(node); } } - const result = - this.visitDeclaration(decl.node, {...context, ...joinModuleContext(context, node, decl)}); + const declContext = {...context, ...joinModuleContext(context, node, decl)}; + // The identifier's declaration is either concrete (a ts.Declaration exists for it) or inline + // (a direct reference to a ts.Expression). + // TODO(alxhub): remove cast once TS is upgraded in g3. + const result = decl.node !== null ? + this.visitDeclaration(decl.node, declContext) : + this.visitExpression((decl as InlineDeclaration).expression, declContext); if (result instanceof Reference) { // Only record identifiers to non-synthetic references. Synthetic references may not have the // same value at runtime as they do at compile time, so it's not legal to refer to them by the @@ -322,10 +328,14 @@ export class StaticInterpreter { } const map = new Map(); declarations.forEach((decl, name) => { - const value = this.visitDeclaration( - decl.node, { - ...context, ...joinModuleContext(context, node, decl), - }); + const declContext = { + ...context, ...joinModuleContext(context, node, decl), + }; + // Visit both concrete and inline declarations. + // TODO(alxhub): remove cast once TS is upgraded in g3. + const value = decl.node !== null ? + this.visitDeclaration(decl.node, declContext) : + this.visitExpression((decl as InlineDeclaration).expression, declContext); map.set(name, value); }); return map; diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index 2349911919..6666b53c3f 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -342,28 +342,65 @@ export interface Import { } /** - * The declaration of a symbol, along with information about how it was imported into the - * application. + * Base type for all `Declaration`s. */ -export interface Declaration { - /** - * TypeScript reference to the declaration itself. - */ - node: T; - +export interface BaseDeclaration { /** * The absolute module path from which the symbol was imported into the application, if the symbol * was imported via an absolute module (even through a chain of re-exports). If the symbol is part * of the application and was not imported from an absolute path, this will be `null`. */ viaModule: string|null; + + /** + * TypeScript reference to the declaration itself, if one exists. + */ + node: T|null; } +/** + * A declaration that has an associated TypeScript `ts.Declaration`. + * + * The alternative is an `InlineDeclaration`. + */ +export interface ConcreteDeclaration extends + BaseDeclaration { + node: T; +} + +/** + * A declaration that does not have an associated TypeScript `ts.Declaration`, only a + * `ts.Expression`. + * + * This can occur in some downlevelings when an `export const VAR = ...;` (a `ts.Declaration`) is + * transpiled to an assignment statement (e.g. `exports.VAR = ...;`). There is no `ts.Declaration` + * associated with `VAR` in that case, only an expression. + */ +export interface InlineDeclaration extends BaseDeclaration { + node: null; + + /** + * The `ts.Expression` which constitutes the value of the declaration. + */ + expression: ts.Expression; +} + +/** + * The declaration of a symbol, along with information about how it was imported into the + * application. + * + * This can either be a `ConcreteDeclaration` if the underlying TypeScript node for the symbol is an + * actual `ts.Declaration`, or an `InlineDeclaration` if the declaration was transpiled in certain + * downlevelings to a `ts.Expression` instead. + */ +export type Declaration = + ConcreteDeclaration| InlineDeclaration; + /** * Abstracts reflection operations on a TypeScript AST. * - * Depending on the format of the code being interpreted, different concepts are represented with - * different syntactical structures. The `ReflectionHost` abstracts over those differences and + * Depending on the format of the code being interpreted, different concepts are represented + * with different syntactical structures. The `ReflectionHost` abstracts over those differences and * presents a single API by which the compiler can query specific information about the AST. * * All operations on the `ReflectionHost` require the use of TypeScript `ts.Node`s with binding diff --git a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts index d2692a33a7..8f2bfe8615 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts @@ -307,8 +307,9 @@ runInEachFileSystem(() => { } else if (directTargetDecl === null) { return fail('No declaration found for DirectTarget'); } - expect(targetDecl.node.getSourceFile().fileName).toBe(_('/node_modules/absolute/index.ts')); - expect(ts.isClassDeclaration(targetDecl.node)).toBe(true); + expect(targetDecl.node !.getSourceFile().fileName) + .toBe(_('/node_modules/absolute/index.ts')); + expect(ts.isClassDeclaration(targetDecl.node !)).toBe(true); expect(directTargetDecl.viaModule).toBe('absolute'); expect(directTargetDecl.node).toBe(targetDecl.node); });