From df4f15db11b8c8f4b83ca84497bb49fb9447df54 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Thu, 14 Sep 2017 15:12:26 -0700 Subject: [PATCH 1/2] Consider the commonjs module indicator as an indicator that something is effectively an external module --- src/compiler/transformers/module/module.ts | 2 +- src/compiler/utilities.ts | 2 +- tests/baselines/reference/dynamicRequire.js | 9 +++++--- .../reference/javascriptCommonjsModule.js | 23 +++++++++++++++++++ .../javascriptCommonjsModule.symbols | 13 +++++++++++ .../reference/javascriptCommonjsModule.types | 15 ++++++++++++ .../built/node_modules/m1/index.js | 23 +++++++++++-------- .../built/node_modules/m1/relative.js | 5 +++- .../compiler/javascriptCommonjsModule.ts | 11 +++++++++ 9 files changed, 87 insertions(+), 16 deletions(-) create mode 100644 tests/baselines/reference/javascriptCommonjsModule.js create mode 100644 tests/baselines/reference/javascriptCommonjsModule.symbols create mode 100644 tests/baselines/reference/javascriptCommonjsModule.types create mode 100644 tests/cases/compiler/javascriptCommonjsModule.ts diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index 08c1fccfe16d5..83bd243362ec0 100644 --- a/src/compiler/transformers/module/module.ts +++ b/src/compiler/transformers/module/module.ts @@ -56,7 +56,7 @@ namespace ts { * @param node The SourceFile node. */ function transformSourceFile(node: SourceFile) { - if (node.isDeclarationFile || !(isExternalModule(node) || compilerOptions.isolatedModules || node.transformFlags & TransformFlags.ContainsDynamicImport)) { + if (node.isDeclarationFile || !(isEffectiveExternalModule(node, compilerOptions) || node.transformFlags & TransformFlags.ContainsDynamicImport)) { return node; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 47cf2038f03b6..f60f67ae95fcb 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -444,7 +444,7 @@ namespace ts { } export function isEffectiveExternalModule(node: SourceFile, compilerOptions: CompilerOptions) { - return isExternalModule(node) || compilerOptions.isolatedModules; + return isExternalModule(node) || compilerOptions.isolatedModules || !!node.commonJsModuleIndicator; } /* @internal */ diff --git a/tests/baselines/reference/dynamicRequire.js b/tests/baselines/reference/dynamicRequire.js index 36737ff7a04e3..84ee2ec7c68e6 100644 --- a/tests/baselines/reference/dynamicRequire.js +++ b/tests/baselines/reference/dynamicRequire.js @@ -4,6 +4,9 @@ function foo(name) { } //// [a_out.js] -function foo(name) { - var s = require("t/" + name); -} +define("a", ["require", "exports"], function (require, exports) { + "use strict"; + function foo(name) { + var s = require("t/" + name); + } +}); diff --git a/tests/baselines/reference/javascriptCommonjsModule.js b/tests/baselines/reference/javascriptCommonjsModule.js new file mode 100644 index 0000000000000..ab517f63805e1 --- /dev/null +++ b/tests/baselines/reference/javascriptCommonjsModule.js @@ -0,0 +1,23 @@ +//// [index.js] +class Foo {} + +class Bar extends Foo {} + +module.exports = Bar; + + +//// [index.js] +var tslib_1 = require("tslib"); +var Foo = /** @class */ (function () { + function Foo() { + } + return Foo; +}()); +var Bar = /** @class */ (function (_super) { + tslib_1.__extends(Bar, _super); + function Bar() { + return _super !== null && _super.apply(this, arguments) || this; + } + return Bar; +}(Foo)); +module.exports = Bar; diff --git a/tests/baselines/reference/javascriptCommonjsModule.symbols b/tests/baselines/reference/javascriptCommonjsModule.symbols new file mode 100644 index 0000000000000..923e0b3467b9f --- /dev/null +++ b/tests/baselines/reference/javascriptCommonjsModule.symbols @@ -0,0 +1,13 @@ +=== tests/cases/compiler/index.js === +class Foo {} +>Foo : Symbol(Foo, Decl(index.js, 0, 0)) + +class Bar extends Foo {} +>Bar : Symbol(Bar, Decl(index.js, 0, 12)) +>Foo : Symbol(Foo, Decl(index.js, 0, 0)) + +module.exports = Bar; +>module : Symbol(export=, Decl(index.js, 2, 24)) +>exports : Symbol(export=, Decl(index.js, 2, 24)) +>Bar : Symbol(Bar, Decl(index.js, 0, 12)) + diff --git a/tests/baselines/reference/javascriptCommonjsModule.types b/tests/baselines/reference/javascriptCommonjsModule.types new file mode 100644 index 0000000000000..88126ce473727 --- /dev/null +++ b/tests/baselines/reference/javascriptCommonjsModule.types @@ -0,0 +1,15 @@ +=== tests/cases/compiler/index.js === +class Foo {} +>Foo : Foo + +class Bar extends Foo {} +>Bar : Bar +>Foo : Foo + +module.exports = Bar; +>module.exports = Bar : typeof Bar +>module.exports : any +>module : any +>exports : any +>Bar : typeof Bar + diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js index 46d38afba6fa2..cc9972956204d 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js @@ -1,10 +1,13 @@ -var m2 = require('m2'); -var rel = require('./relative'); -/** - * @param {string} p1 The first param - */ -exports.f1 = function (p1) { - return 42; -}; -exports.f2 = m2; -exports.rel = rel.relativeProp; +define(["require", "exports"], function (require, exports) { + "use strict"; + var m2 = require('m2'); + var rel = require('./relative'); + /** + * @param {string} p1 The first param + */ + exports.f1 = function (p1) { + return 42; + }; + exports.f2 = m2; + exports.rel = rel.relativeProp; +}); diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js index 13432076541ac..3d2bbfc0d274a 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js @@ -1 +1,4 @@ -exports.relativeProp = true; +define(["require", "exports"], function (require, exports) { + "use strict"; + exports.relativeProp = true; +}); diff --git a/tests/cases/compiler/javascriptCommonjsModule.ts b/tests/cases/compiler/javascriptCommonjsModule.ts new file mode 100644 index 0000000000000..dde30580f70ac --- /dev/null +++ b/tests/cases/compiler/javascriptCommonjsModule.ts @@ -0,0 +1,11 @@ +// @allowJS: true +// @outDir: ./out +// @module: commonjs +// @noEmitHelpers: true +// @importHelpers: true +// @filename: index.js +class Foo {} + +class Bar extends Foo {} + +module.exports = Bar; From bc030f1367658e381a1a48381349770491c6472d Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 8 Nov 2017 18:39:19 -0800 Subject: [PATCH 2/2] Only use commonjs module indicator when targeting commonjs --- src/compiler/utilities.ts | 2 +- tests/baselines/reference/dynamicRequire.js | 9 +++----- .../built/node_modules/m1/index.js | 23 ++++++++----------- .../built/node_modules/m1/relative.js | 5 +--- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index f60f67ae95fcb..3299def07204f 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -444,7 +444,7 @@ namespace ts { } export function isEffectiveExternalModule(node: SourceFile, compilerOptions: CompilerOptions) { - return isExternalModule(node) || compilerOptions.isolatedModules || !!node.commonJsModuleIndicator; + return isExternalModule(node) || compilerOptions.isolatedModules || ((getEmitModuleKind(compilerOptions) === ModuleKind.CommonJS) && !!node.commonJsModuleIndicator); } /* @internal */ diff --git a/tests/baselines/reference/dynamicRequire.js b/tests/baselines/reference/dynamicRequire.js index 84ee2ec7c68e6..36737ff7a04e3 100644 --- a/tests/baselines/reference/dynamicRequire.js +++ b/tests/baselines/reference/dynamicRequire.js @@ -4,9 +4,6 @@ function foo(name) { } //// [a_out.js] -define("a", ["require", "exports"], function (require, exports) { - "use strict"; - function foo(name) { - var s = require("t/" + name); - } -}); +function foo(name) { + var s = require("t/" + name); +} diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js index cc9972956204d..46d38afba6fa2 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/index.js @@ -1,13 +1,10 @@ -define(["require", "exports"], function (require, exports) { - "use strict"; - var m2 = require('m2'); - var rel = require('./relative'); - /** - * @param {string} p1 The first param - */ - exports.f1 = function (p1) { - return 42; - }; - exports.f2 = m2; - exports.rel = rel.relativeProp; -}); +var m2 = require('m2'); +var rel = require('./relative'); +/** + * @param {string} p1 The first param + */ +exports.f1 = function (p1) { + return 42; +}; +exports.f2 = m2; +exports.rel = rel.relativeProp; diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js index 3d2bbfc0d274a..13432076541ac 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/maxDepthExceeded/built/node_modules/m1/relative.js @@ -1,4 +1 @@ -define(["require", "exports"], function (require, exports) { - "use strict"; - exports.relativeProp = true; -}); +exports.relativeProp = true;