From e9fb5fdb894b30c78e6f23f51f3a634ce325a7d0 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 18 Dec 2019 14:03:05 +0000 Subject: [PATCH] fix(ngcc): handle UMD re-exports (#34254) In TS we can re-export imports using statements of the form: ``` export * from 'some-import'; ``` This is downleveled in UMD to: ``` function factory(exports, someImport) { function __export(m) { for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p]; } __export(someImport); } ``` This commit adds support for this. PR Close #34254 --- .../ngcc/src/host/commonjs_host.ts | 8 +- .../compiler-cli/ngcc/src/host/umd_host.ts | 79 +++++++++++++++---- packages/compiler-cli/ngcc/src/utils.ts | 4 + .../ngcc/test/host/umd_host_spec.ts | 65 ++++++++++++++- 4 files changed, 129 insertions(+), 27 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts index 92699f3675..18a0bed45d 100644 --- a/packages/compiler-cli/ngcc/src/host/commonjs_host.ts +++ b/packages/compiler-cli/ngcc/src/host/commonjs_host.ts @@ -11,7 +11,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 {isDefined} from '../utils'; +import {isDefined, stripExtension} from '../utils'; import {Esm5ReflectionHost} from './esm5_host'; import {NgccClassSymbol} from './ngcc_host'; @@ -152,12 +152,12 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost { return []; } - const viaModule = stripExtension(importedFile.fileName); const importedExports = this.getExportsOfModule(importedFile); if (importedExports === null) { return []; } + const viaModule = stripExtension(importedFile.fileName); const reexports: CommonJsExportDeclaration[] = []; importedExports.forEach((decl, name) => { if (decl.node !== null) { @@ -259,10 +259,6 @@ function isReexportStatement(statement: ts.Statement): statement is ReexportStat statement.expression.arguments.length === 1; } -function stripExtension(fileName: string): string { - return fileName.replace(/\..+$/, ''); -} - function getOrDefault(map: Map, key: K, factory: (key: K) => V): V { if (!map.has(key)) { map.set(key, factory(key)); diff --git a/packages/compiler-cli/ngcc/src/host/umd_host.ts b/packages/compiler-cli/ngcc/src/host/umd_host.ts index f91dfe1d97..23c78ea31a 100644 --- a/packages/compiler-cli/ngcc/src/host/umd_host.ts +++ b/packages/compiler-cli/ngcc/src/host/umd_host.ts @@ -12,6 +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 {stripExtension} from '../utils'; import {Esm5ReflectionHost, stripParentheses} from './esm5_host'; export class UmdReflectionHost extends Esm5ReflectionHost { @@ -32,7 +33,10 @@ export class UmdReflectionHost extends Esm5ReflectionHost { return superImport; } - const importParameter = this.findUmdImportParameter(id); + // 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); + const importParameter = nsIdentifier && this.findUmdImportParameter(nsIdentifier); const from = importParameter && this.getUmdImportPath(importParameter); return from !== null ? {from, name: id.text} : null; } @@ -107,14 +111,19 @@ export class UmdReflectionHost extends Esm5ReflectionHost { private computeExportsOfUmdModule(sourceFile: ts.SourceFile): Map|null { const moduleMap = new Map(); - const exportStatements = this.getModuleStatements(sourceFile).filter(isUmdExportStatement); - const exportDeclarations = - exportStatements.map(statement => this.extractUmdExportDeclaration(statement)); - exportDeclarations.forEach(decl => { - if (decl) { - moduleMap.set(decl.name, decl.declaration); + for (const statement of this.getModuleStatements(sourceFile)) { + if (isUmdExportStatement(statement)) { + const declaration = this.extractUmdExportDeclaration(statement); + if (declaration !== null) { + moduleMap.set(declaration.name, declaration.declaration); + } + } else if (isReexportStatement(statement)) { + const reexports = this.extractUmdReexports(statement, sourceFile); + for (const reexport of reexports) { + moduleMap.set(reexport.name, reexport.declaration); + } } - }); + } return moduleMap; } @@ -130,16 +139,43 @@ export class UmdReflectionHost extends Esm5ReflectionHost { return {name, declaration}; } - private findUmdImportParameter(id: ts.Identifier): ts.ParameterDeclaration|null { - // 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); - const nsSymbol = nsIdentifier && this.checker.getSymbolAtLocation(nsIdentifier) || null; + private extractUmdReexports(statement: ReexportStatement, containingFile: ts.SourceFile): + UmdExportDeclaration[] { + const importParameter = this.findUmdImportParameter(statement.expression.arguments[0]); + const importPath = importParameter && this.getUmdImportPath(importParameter); + if (importPath === null) { + return []; + } + const importedFile = this.resolveModuleName(importPath, containingFile); + if (importedFile === undefined) { + return []; + } - // Is the namespace a parameter on a UMD factory function, e.g. `function factory(this, core)`? - // If so then return its declaration. - const nsDeclaration = nsSymbol && nsSymbol.valueDeclaration; - return nsDeclaration && ts.isParameter(nsDeclaration) ? nsDeclaration : null; + const importedExports = this.getExportsOfModule(importedFile); + if (importedExports === null) { + return []; + } + + const viaModule = stripExtension(importedFile.fileName); + const reexports: UmdExportDeclaration[] = []; + 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; + } + + /** + * Is the identifier a parameter on a UMD factory function, e.g. `function factory(this, core)`? + * If so then return its declaration. + */ + private findUmdImportParameter(id: ts.Identifier): ts.ParameterDeclaration|null { + const symbol = id && this.checker.getSymbolAtLocation(id) || null; + const declaration = symbol && symbol.valueDeclaration; + return declaration && ts.isParameter(declaration) ? declaration : null; } private getUmdImportedDeclaration(id: ts.Identifier): Declaration|null { @@ -237,6 +273,15 @@ interface UmdExportDeclaration { declaration: Declaration; } +type ReexportStatement = ts.ExpressionStatement & {expression: {arguments: [ts.Identifier]}}; +function isReexportStatement(statement: ts.Statement): statement is ReexportStatement { + return ts.isExpressionStatement(statement) && ts.isCallExpression(statement.expression) && + ts.isIdentifier(statement.expression.expression) && + statement.expression.expression.text === '__export' && + statement.expression.arguments.length === 1 && + ts.isIdentifier(statement.expression.arguments[0]); +} + function getRequiredModulePath(wrapperFn: ts.FunctionExpression, paramIndex: number): string { const statement = wrapperFn.body.statements[0]; if (!ts.isExpressionStatement(statement)) { diff --git a/packages/compiler-cli/ngcc/src/utils.ts b/packages/compiler-cli/ngcc/src/utils.ts index 96ced4227a..de9f66bb70 100644 --- a/packages/compiler-cli/ngcc/src/utils.ts +++ b/packages/compiler-cli/ngcc/src/utils.ts @@ -115,3 +115,7 @@ export function resolveFileWithPostfixes( export function stripDollarSuffix(value: string): string { return value.replace(/\$\d+$/, ''); } + +export function stripExtension(fileName: string): string { + return fileName.replace(/\..+$/, ''); +} 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 66a59ac8dc..21997fb6a4 100644 --- a/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/umd_host_spec.ts @@ -546,10 +546,10 @@ runInEachFileSystem(() => { name: _('/index.js'), contents: ` (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('./a_module'), require('./b_module')) : - typeof define === 'function' && define.amd ? define('index', ['exports', './a_module', './b_module'], factory) : - (factory(global.index, global.a_module, global.b_module)); - }(this, (function (exports, a_module, b_module) { 'use strict'; + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('./a_module'), require('./b_module'), require('./wildcard_reexports')) : + typeof define === 'function' && define.amd ? define('index', ['exports', './a_module', './b_module', './wildcard_reexports], factory) : + (factory(global.index, global.a_module, global.b_module, global.wildcard_reexports)); + }(this, (function (exports, a_module, b_module, wildcard_reexports) { 'use strict'; }))); ` }, @@ -590,6 +590,35 @@ runInEachFileSystem(() => { exports.SomeClass = SomeClass; })));`, }, + { + name: _('/xtra_module.js'), + contents: ` +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) : + typeof define === 'function' && define.amd ? define('xtra_module', ['exports'], factory) : + (factory(global.xtra_module)); +}(this, (function (exports) { 'use strict'; + var xtra1 = 'xtra1'; + var xtra2 = 'xtra2'; + exports.xtra1 = xtra1; + exports.xtra2 = xtra2; +})));`, + }, + { + name: _('/wildcard_reexports.js'), + contents: ` +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('./b_module'), require('./xtra_module')) : + typeof define === 'function' && define.amd ? define('wildcard_reexports', ['exports', './b_module', './xtra_module'], factory) : + (factory(global.wildcard_reexports, b_module, xtra_module)); +}(this, (function (exports, b_module, xtra_module) { 'use strict'; + function __export(m) { + for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p]; + } +__export(b_module); +__export(xtra_module); +})));`, + } ]; FUNCTION_BODY_FILE = { @@ -1862,6 +1891,34 @@ runInEachFileSystem(() => { expect(classSymbol !.implementation.valueDeclaration).toBe(node); }); + it('should handle wildcard re-exports of other modules', () => { + loadFakeCore(getFileSystem()); + loadTestFiles(EXPORTS_FILES); + const bundle = makeTestBundleProgram(_('/index.js')); + const host = new UmdReflectionHost(new MockLogger(), false, bundle); + const file = getSourceFileOrError(bundle.program, _('/wildcard_reexports.js')); + 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])) + .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')], + [ + 'SomeClass', + `SomeClass = (function() {\n function SomeClass() {}\n return SomeClass;\n }())`, + _('/b_module') + ], + ['xtra1', `xtra1 = 'xtra1'`, _('/xtra_module')], + ['xtra2', `xtra2 = 'xtra2'`, _('/xtra_module')], + ]); + }); + it('should return the class symbol for an ES5 class (outer variable declaration)', () => { loadTestFiles([SIMPLE_CLASS_FILE]); const bundle = makeTestBundleProgram(SIMPLE_CLASS_FILE.name);