From 1e716219341f78fba300d93ca95da9d2cdd9e987 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 6 Oct 2019 13:20:54 -0400 Subject: [PATCH 1/3] module: allow unrestricted cjs requires --- lib/internal/modules/cjs/loader.js | 46 ++++++++++++--------- test/es-module/test-esm-type-flag-errors.js | 4 +- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 479044a26aaba9..3436e93d653f18 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -816,20 +816,32 @@ Module.prototype.load = function(filename) { const url = `${pathToFileURL(filename)}`; const module = ESMLoader.moduleMap.get(url); // Create module entry at load time to snapshot exports correctly - const exports = this.exports; - // Called from cjs translator - if (module !== undefined && module.module !== undefined) { - if (module.module.getStatus() >= kInstantiated) - module.module.setExport('default', exports); - } else { // preemptively cache - ESMLoader.moduleMap.set( - url, - new ModuleJob(ESMLoader, url, () => - new ModuleWrap(function() { - this.setExport('default', exports); - }, ['default'], url) - ) - ); + const ext = path.extname(filename); + // Only inject modules that are valid in the ES module system + let injectIntoESM = ext === '.js' || ext === '.cjs' || ext === '.json' || + ext === '.node'; + if (ext === '.js') { + const pkg = readPackageScope(filename); + // Do not inject CJS modules that break module type correctness + if (pkg && pkg.type === 'module') + injectIntoESM = false; + } + if (injectIntoESM) { + const exports = this.exports; + // Called from cjs translator + if (module !== undefined && module.module !== undefined) { + if (module.module.getStatus() >= kInstantiated) + module.module.setExport('default', exports); + } else { // preemptively cache + ESMLoader.moduleMap.set( + url, + new ModuleJob(ESMLoader, url, () => + new ModuleWrap(function() { + this.setExport('default', exports); + }, ['default'], url) + ) + ); + } } } }; @@ -963,12 +975,6 @@ Module.prototype._compile = function(content, filename) { // Native extension for .js Module._extensions['.js'] = function(module, filename) { - if (experimentalModules && filename.endsWith('.js')) { - const pkg = readPackageScope(filename); - if (pkg && pkg.type === 'module') { - throw new ERR_REQUIRE_ESM(filename); - } - } const content = fs.readFileSync(filename, 'utf8'); module._compile(content, filename); }; diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js index 8725fb62323b75..f8ba7130efe837 100644 --- a/test/es-module/test-esm-type-flag-errors.js +++ b/test/es-module/test-esm-type-flag-errors.js @@ -26,9 +26,9 @@ expect('--input-type=module', packageTypeModuleMain, try { require('../fixtures/es-modules/package-type-module/index.js'); - assert.fail('Expected CJS to fail loading from type: module package.'); } catch (e) { - assert(e.toString().match(/Error \[ERR_REQUIRE_ESM\]: Must use import to load ES Module:/)); + // Verify CommonJS load is attempted but fails on ES module syntax + assert(e instanceof SyntaxError); } function expect(opt = '', inputFile, want, wantsError = false) { From c3315501fc102a7b674d213ee7b9608931114e71 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 6 Oct 2019 14:33:25 -0400 Subject: [PATCH 2/3] reordering --- lib/internal/modules/cjs/loader.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 3436e93d653f18..e080dbaaf2afda 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -812,9 +812,6 @@ Module.prototype.load = function(filename) { this.loaded = true; if (experimentalModules) { - const ESMLoader = asyncESM.ESMLoader; - const url = `${pathToFileURL(filename)}`; - const module = ESMLoader.moduleMap.get(url); // Create module entry at load time to snapshot exports correctly const ext = path.extname(filename); // Only inject modules that are valid in the ES module system @@ -827,6 +824,9 @@ Module.prototype.load = function(filename) { injectIntoESM = false; } if (injectIntoESM) { + const ESMLoader = asyncESM.ESMLoader; + const url = `${pathToFileURL(filename)}`; + const module = ESMLoader.moduleMap.get(url); const exports = this.exports; // Called from cjs translator if (module !== undefined && module.module !== undefined) { From 94076169915757dcaad1afc4aef76b94705bacd8 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Mon, 7 Oct 2019 14:47:42 -0400 Subject: [PATCH 3/3] use assert.throws --- test/es-module/test-esm-type-flag-errors.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/es-module/test-esm-type-flag-errors.js b/test/es-module/test-esm-type-flag-errors.js index f8ba7130efe837..c259c5c5748eb7 100644 --- a/test/es-module/test-esm-type-flag-errors.js +++ b/test/es-module/test-esm-type-flag-errors.js @@ -24,12 +24,10 @@ expect('', packageWithoutTypeMain, 'package-without-type'); expect('--input-type=module', packageTypeModuleMain, 'ERR_INPUT_TYPE_NOT_ALLOWED', true); -try { +// Verify CommonJS load is attempted but fails on ES module syntax +assert.throws(() => { require('../fixtures/es-modules/package-type-module/index.js'); -} catch (e) { - // Verify CommonJS load is attempted but fails on ES module syntax - assert(e instanceof SyntaxError); -} +}, SyntaxError); function expect(opt = '', inputFile, want, wantsError = false) { // TODO: Remove when --experimental-modules is unflagged