diff --git a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts index 4ca4fdcbc1..00771a60fa 100644 --- a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts +++ b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts @@ -12,7 +12,7 @@ import {absoluteFrom} from '../../../src/ngtsc/file_system'; import {Declaration, Import} from '../../../src/ngtsc/reflection'; import {Logger} from '../logging/logger'; import {BundleProgram} from '../packages/bundle_program'; -import {FactoryMap, getTsHelperFnFromIdentifier, isDefined, stripExtension} from '../utils'; +import {FactoryMap, isDefined} from '../utils'; import {ExportDeclaration, ExportStatement, findNamespaceOfIdentifier, findRequireCallReference, isExportStatement, isRequireCall, isWildcardReexportStatement, RequireCall, WildcardReexportStatement} from './commonjs_umd_utils'; import {Esm5ReflectionHost} from './esm5_host'; @@ -43,7 +43,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { } getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null { - return this.getCommonJsImportedDeclaration(id) || super.getDeclarationOfIdentifier(id); + return this.getCommonJsModuleDeclaration(id) || super.getDeclarationOfIdentifier(id); } getExportsOfModule(module: ts.Node): Map|null { @@ -99,7 +99,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { const moduleMap = new Map(); for (const statement of this.getModuleStatements(sourceFile)) { if (isExportStatement(statement)) { - const exportDeclaration = this.extractCommonJsExportDeclaration(statement); + const exportDeclaration = this.extractBasicCommonJsExportDeclaration(statement); moduleMap.set(exportDeclaration.name, exportDeclaration.declaration); } else if (isWildcardReexportStatement(statement)) { const reexports = this.extractCommonJsWildcardReexports(statement, sourceFile); @@ -111,23 +111,10 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { return moduleMap; } - private extractCommonJsExportDeclaration(statement: ExportStatement): ExportDeclaration { + private extractBasicCommonJsExportDeclaration(statement: ExportStatement): ExportDeclaration { const exportExpression = statement.expression.right; - const declaration = this.getDeclarationOfExpression(exportExpression); const name = statement.expression.left.name.text; - if (declaration !== null) { - return {name, declaration}; - } else { - return { - name, - declaration: { - node: null, - known: null, - expression: exportExpression, - viaModule: null, - }, - }; - } + return this.extractCommonJsExportDeclaration(name, exportExpression); } private extractCommonJsWildcardReexports( @@ -152,18 +139,13 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { return []; } - const viaModule = stripExtension(importedFile.fileName); + const viaModule = importPath.startsWith('./') ? null : importPath; const reexports: ExportDeclaration[] = []; - importedExports.forEach((decl, name) => { - if (decl.node !== null) { - reexports.push({ - name, - declaration: {node: decl.node, known: null, viaModule, identity: decl.identity} - }); - } else { - reexports.push( - {name, declaration: {node: null, known: null, expression: decl.expression, viaModule}}); + importedExports.forEach((declaration, name) => { + if (viaModule !== null && declaration.viaModule === null) { + declaration = {...declaration, viaModule}; } + reexports.push({name, declaration}); }); return reexports; } @@ -175,19 +157,38 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { return nsIdentifier && findRequireCallReference(nsIdentifier, this.checker); } - private getCommonJsImportedDeclaration(id: ts.Identifier): Declaration|null { - const importInfo = this.getImportOfIdentifier(id); - if (importInfo === null) { + private extractCommonJsExportDeclaration(name: string, expression: ts.Expression): + ExportDeclaration { + const declaration = this.getDeclarationOfExpression(expression); + if (declaration !== null) { + return {name, declaration}; + } else { + return { + name, + declaration: {node: null, known: null, expression, viaModule: null}, + }; + } + } + + /** + * Handle the case where the identifier represents a reference to a whole CommonJS + * module, i.e. the result of a call to `require(...)`. + * + * @param id the identifier whose declaration we are looking for. + * @returns a declaration if `id` refers to a CommonJS module, or `null` otherwise. + */ + private getCommonJsModuleDeclaration(id: ts.Identifier): Declaration|null { + const requireCall = findRequireCallReference(id, this.checker); + if (requireCall === null) { return null; } - - const importedFile = this.resolveModuleName(importInfo.from, id.getSourceFile()); - if (importedFile === undefined) { + const importPath = requireCall.arguments[0].text; + const module = this.resolveModuleName(importPath, id.getSourceFile()); + if (module === undefined) { return null; } - - const viaModule = !importInfo.from.startsWith('.') ? importInfo.from : null; - return {node: importedFile, known: getTsHelperFnFromIdentifier(id), viaModule, identity: null}; + const viaModule = importPath.startsWith('./') ? null : importPath; + return {node: module, known: null, viaModule, identity: null}; } private resolveModuleName(moduleName: string, containingFile: ts.SourceFile): ts.SourceFile diff --git a/packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts b/packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts index 52fe529f9a..c49113aaf2 100644 --- a/packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts +++ b/packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts @@ -56,7 +56,7 @@ export interface RequireCall extends ts.CallExpression { * `ts.Identifier` corresponding to `` will be returned). Otherwise return `null`. */ export function findNamespaceOfIdentifier(id: ts.Identifier): ts.Identifier|null { - return id.parent && ts.isPropertyAccessExpression(id.parent) && + return id.parent && ts.isPropertyAccessExpression(id.parent) && id.parent.name === id && ts.isIdentifier(id.parent.expression) ? id.parent.expression : 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 400bf9962e..97565d497b 100644 --- a/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts @@ -1649,7 +1649,7 @@ exports.MissingClass2 = MissingClass2; bundle.program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedVariableDeclaration); const classDecorators = host.getDecoratorsOfDeclaration(classNode)!; - const identifierOfDirective = + const namespaceIdentifier = (((classDecorators[0].node as ts.ObjectLiteralExpression).properties[0] as ts.PropertyAssignment) .initializer as ts.PropertyAccessExpression) @@ -1657,7 +1657,7 @@ exports.MissingClass2 = MissingClass2; const expectedDeclarationNode = getSourceFileOrError(bundle.program, _('/node_modules/@angular/core/index.d.ts')); - const actualDeclaration = host.getDeclarationOfIdentifier(identifierOfDirective); + const actualDeclaration = host.getDeclarationOfIdentifier(namespaceIdentifier); expect(actualDeclaration).not.toBe(null); expect(actualDeclaration!.node).toBe(expectedDeclarationNode); expect(actualDeclaration!.viaModule).toBe('@angular/core'); @@ -1858,10 +1858,11 @@ exports.MissingClass2 = MissingClass2; const bundle = makeTestBundleProgram(testFile.name); const host = createHost(bundle, new CommonJsReflectionHost(new MockLogger(), false, bundle)); - const tslibSourceFile = getSourceFileOrError(bundle.program, tslibFile.name); - const testForHelper = - createTestForTsHelper(bundle.program, host, testFile, () => tslibSourceFile); + const testForHelper = createTestForTsHelper( + bundle.program, host, testFile, + helperName => getDeclaration( + bundle.program, tslibFile.name, helperName, ts.isFunctionDeclaration)); testForHelper('a', '__assign', KnownDeclaration.TsHelperAssign, 'tslib'); testForHelper('b', '__spread', KnownDeclaration.TsHelperSpread, 'tslib'); @@ -2093,20 +2094,20 @@ exports.MissingClass2 = MissingClass2; expect(Array.from(exportDeclarations!.entries()) .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')], - ['b', `b = a_module.a`, _('/b_module')], - ['c', `a = 'a'`, _('/b_module')], - ['d', `b = a_module.a`, _('/b_module')], - ['e', `e = 'e'`, _('/b_module')], - ['DirectiveX', `Directive: FnWithArg<(clazz: any) => any>`, _('/b_module')], + ['Directive', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'], + ['a', `a = 'a'`, null], + ['b', `b = a_module.a`, null], + ['c', `a = 'a'`, null], + ['d', `b = a_module.a`, null], + ['e', `e = 'e'`, null], + ['DirectiveX', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'], [ 'SomeClass', `SomeClass = (function() {\n function SomeClass() {}\n return SomeClass;\n}())`, - _('/b_module') + null ], - ['xtra1', `xtra1 = 'xtra1'`, _('/xtra_module')], - ['xtra2', `xtra2 = 'xtra2'`, _('/xtra_module')], + ['xtra1', `xtra1 = 'xtra1'`, null], + ['xtra2', `xtra2 = 'xtra2'`, null], ]); }); @@ -2123,20 +2124,20 @@ exports.MissingClass2 = MissingClass2; expect(Array.from(exportDeclarations!.entries()) .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')], - ['b', `b = a_module.a`, _('/b_module')], - ['c', `a = 'a'`, _('/b_module')], - ['d', `b = a_module.a`, _('/b_module')], - ['e', `e = 'e'`, _('/b_module')], - ['DirectiveX', `Directive: FnWithArg<(clazz: any) => any>`, _('/b_module')], + ['Directive', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'], + ['a', `a = 'a'`, null], + ['b', `b = a_module.a`, null], + ['c', `a = 'a'`, null], + ['d', `b = a_module.a`, null], + ['e', `e = 'e'`, null], + ['DirectiveX', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'], [ 'SomeClass', `SomeClass = (function() {\n function SomeClass() {}\n return SomeClass;\n}())`, - _('/b_module') + null ], - ['xtra1', `xtra1 = 'xtra1'`, _('/xtra_module')], - ['xtra2', `xtra2 = 'xtra2'`, _('/xtra_module')], + ['xtra1', `xtra1 = 'xtra1'`, null], + ['xtra2', `xtra2 = 'xtra2'`, 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 ef75e92ba8..0cd2dbd33b 100644 --- a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts @@ -591,8 +591,8 @@ runInEachFileSystem(() => { { name: _('/b_module.js'), contents: `(function (global, factory) {\n` + - ` typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core'), require('/a_module')) :\n` + - ` typeof define === 'function' && define.amd ? define('b_module', ['exports', '@angular/core', 'a_module'], factory) :\n` + + ` typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core'), require('./a_module')) :\n` + + ` typeof define === 'function' && define.amd ? define('b_module', ['exports', '@angular/core', './a_module'], factory) :\n` + ` (factory(global.b_module));\n` + `}(this, (function (exports, core, a_module) { 'use strict';\n` + ` var b = a_module.a;\n` + @@ -1931,9 +1931,10 @@ runInEachFileSystem(() => { const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle)); const {factoryFn} = parseStatementForUmdModule( getSourceFileOrError(bundle.program, testFile.name).statements[0])!; - const tslibSourceFile = getSourceFileOrError(bundle.program, tslibFile.name); - - const testForHelper = createTestForTsHelper(host, factoryFn, () => tslibSourceFile); + const testForHelper = createTestForTsHelper( + host, factoryFn, + (_fn, helperName) => getDeclaration( + bundle.program, tslibFile.name, helperName, ts.isFunctionDeclaration)); testForHelper('a', '__assign', KnownDeclaration.TsHelperAssign, 'tslib'); testForHelper('b', '__spread', KnownDeclaration.TsHelperSpread, 'tslib'); @@ -2173,9 +2174,9 @@ runInEachFileSystem(() => { .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'], + ['a', `a = 'a'`, null], ['b', `b = a_module.a`, null], - ['c', `a = 'a'`, '/a_module'], + ['c', `a = 'a'`, null], ['d', `b = a_module.a`, null], ['e', `e = 'e'`, null], ['DirectiveX', `Directive: FnWithArg<(clazz: any) => any>`, '@angular/core'],