From dc9a066f65f713daefbb61ecd6b0557af430ab3d Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 25 Oct 2018 15:08:06 -0700 Subject: [PATCH] Do not merge commonJS exports into an alias (#28133) * Do not merge commonsjs exports onto an alias getCommonJSExportEquals merges export assignments and export property assignments. Something like this, which has no equivalent structure in TS: ```js module.exports = function() { } module.exports.expando = 1 ``` However, it is sometimes called with an alias, when its parent, resolveExternalModuleSymbol, is called with dontResolveAlias: true, and when the initialiser of the export assignment is an alias: ```js function alias() { } module.exports = alias module.exports.expando = 1 ``` In this case, (1) the actual value `alias` will have already merged in a previous call to getCommonJSExportEquals and (2) getTypeOfSymbol will follow the alias symbol to get the right type. So getCommonJSExportEquals should do nothing in this case. This bug manifests in the code for dynamic imports, which calls getTypeOfSymbol on the incorrectly merged alias, which now has enough value flags--Function, for example--to take the wrong branch and subsequently crash. * Update baselines --- src/compiler/checker.ts | 2 +- ...errorForConflictingExportEqualsValue.types | 2 +- .../moduleExportAliasImported.symbols | 19 +++++++++++++++ .../reference/moduleExportAliasImported.types | 23 +++++++++++++++++++ .../salsa/moduleExportAliasImported.ts | 12 ++++++++++ 5 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 tests/baselines/reference/moduleExportAliasImported.symbols create mode 100644 tests/baselines/reference/moduleExportAliasImported.types create mode 100644 tests/cases/conformance/salsa/moduleExportAliasImported.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4305227dab7..49caad2116e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2362,7 +2362,7 @@ namespace ts { } function getCommonJsExportEquals(exported: Symbol | undefined, moduleSymbol: Symbol): Symbol | undefined { - if (!exported || exported === unknownSymbol || exported === moduleSymbol || moduleSymbol.exports!.size === 1) { + if (!exported || exported === unknownSymbol || exported === moduleSymbol || moduleSymbol.exports!.size === 1 || exported.flags & SymbolFlags.Alias) { return exported; } const merged = cloneSymbol(exported); diff --git a/tests/baselines/reference/errorForConflictingExportEqualsValue.types b/tests/baselines/reference/errorForConflictingExportEqualsValue.types index b9915169120..5d2b63f2e9a 100644 --- a/tests/baselines/reference/errorForConflictingExportEqualsValue.types +++ b/tests/baselines/reference/errorForConflictingExportEqualsValue.types @@ -6,6 +6,6 @@ export = x; >x : any import("./a"); ->import("./a") : Promise +>import("./a") : Promise >"./a" : "./a" diff --git a/tests/baselines/reference/moduleExportAliasImported.symbols b/tests/baselines/reference/moduleExportAliasImported.symbols new file mode 100644 index 00000000000..9649cc0a3b6 --- /dev/null +++ b/tests/baselines/reference/moduleExportAliasImported.symbols @@ -0,0 +1,19 @@ +=== tests/cases/conformance/salsa/bug28014.js === +exports.version = 1 +>exports.version : Symbol(version) +>exports : Symbol(version, Decl(bug28014.js, 0, 0)) +>version : Symbol(version, Decl(bug28014.js, 0, 0)) + +function alias() { } +>alias : Symbol(alias, Decl(bug28014.js, 0, 19)) + +module.exports = alias +>module.exports : Symbol("tests/cases/conformance/salsa/bug28014", Decl(bug28014.js, 0, 0)) +>module : Symbol(export=, Decl(bug28014.js, 1, 20)) +>exports : Symbol(export=, Decl(bug28014.js, 1, 20)) +>alias : Symbol(alias, Decl(bug28014.js, 0, 19)) + +=== tests/cases/conformance/salsa/importer.js === +import('./bug28014') +>'./bug28014' : Symbol("tests/cases/conformance/salsa/bug28014", Decl(bug28014.js, 0, 0)) + diff --git a/tests/baselines/reference/moduleExportAliasImported.types b/tests/baselines/reference/moduleExportAliasImported.types new file mode 100644 index 00000000000..9691acfab22 --- /dev/null +++ b/tests/baselines/reference/moduleExportAliasImported.types @@ -0,0 +1,23 @@ +=== tests/cases/conformance/salsa/bug28014.js === +exports.version = 1 +>exports.version = 1 : 1 +>exports.version : number +>exports : typeof alias +>version : number +>1 : 1 + +function alias() { } +>alias : typeof alias + +module.exports = alias +>module.exports = alias : typeof alias +>module.exports : typeof alias +>module : { "tests/cases/conformance/salsa/bug28014": typeof alias; } +>exports : typeof alias +>alias : typeof alias + +=== tests/cases/conformance/salsa/importer.js === +import('./bug28014') +>import('./bug28014') : Promise<() => void> +>'./bug28014' : "./bug28014" + diff --git a/tests/cases/conformance/salsa/moduleExportAliasImported.ts b/tests/cases/conformance/salsa/moduleExportAliasImported.ts new file mode 100644 index 00000000000..95c02981382 --- /dev/null +++ b/tests/cases/conformance/salsa/moduleExportAliasImported.ts @@ -0,0 +1,12 @@ +// @allowJs: true +// @noEmit: true +// @checkJs: true +// @target: esnext +// @module: esnext +// @Filename: bug28014.js +exports.version = 1 +function alias() { } +module.exports = alias + +// @Filename: importer.js +import('./bug28014')