From 0106d19785ecb834a40d2458601d9da7ec3dab02 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 25 Oct 2018 09:06:48 -0700 Subject: [PATCH 1/2] 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. --- src/compiler/checker.ts | 2 +- .../moduleExportAliasImported.symbols | 19 +++++++++++++++ .../reference/moduleExportAliasImported.types | 23 +++++++++++++++++++ .../salsa/moduleExportAliasImported.ts | 12 ++++++++++ 4 files changed, 55 insertions(+), 1 deletion(-) 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 fd786799b8b4c..8862960558ea4 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/moduleExportAliasImported.symbols b/tests/baselines/reference/moduleExportAliasImported.symbols new file mode 100644 index 0000000000000..9649cc0a3b64c --- /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 0000000000000..9691acfab2254 --- /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 0000000000000..95c029813828d --- /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') From 56cbb183e8a8a012a591994c8dce9158901f49b2 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 25 Oct 2018 10:06:43 -0700 Subject: [PATCH 2/2] Update baselines --- .../reference/errorForConflictingExportEqualsValue.types | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/baselines/reference/errorForConflictingExportEqualsValue.types b/tests/baselines/reference/errorForConflictingExportEqualsValue.types index b99151691203f..5d2b63f2e9ae0 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"