From 20f3a00a36105f319a1e7570a11e30b638fc4cef Mon Sep 17 00:00:00 2001 From: Gil Tayar Date: Sat, 10 Jul 2021 08:40:57 +0300 Subject: [PATCH 1/2] fixes #4675: ESM swallowing MODULE_NOT_FOUND errors in case of type:module --- lib/esm-utils.js | 16 ++++++++++- test/integration/esm.spec.js | 27 ++++++++++++++++--- .../fixtures/esm/type-module/package.json | 3 +++ ...hat-imports-non-existing-module.fixture.js | 6 +++++ 4 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 test/integration/fixtures/esm/type-module/package.json create mode 100644 test/integration/fixtures/esm/type-module/test-that-imports-non-existing-module.fixture.js diff --git a/lib/esm-utils.js b/lib/esm-utils.js index 1e1230592a..fd51e012cc 100644 --- a/lib/esm-utils.js +++ b/lib/esm-utils.js @@ -52,7 +52,21 @@ exports.requireOrImport = hasStableEsmImplementation err.code === 'ERR_UNKNOWN_FILE_EXTENSION' || err.code === 'ERR_UNSUPPORTED_DIR_IMPORT' ) { - return require(file); + try { + return require(file); + } catch (requireErr) { + if (requireErr.code === 'ERR_REQUIRE_ESM') { + // This happens when the test file is a JS file, but via type:module is actually ESM, + // AND has an import to a file that doesn't exist. + // This throws an `ERR_MODULE_NOT_FOUND` // error above, + // and when we try to `require` it here, it throws an `ERR_REQUIRE_ESM`. + // What we want to do is throw the original error (the `ERR_MODULE_NOT_FOUND`), + // and not the `ERR_REQUIRE_ESM` error, which is a red herring. + throw err; + } else { + throw requireErr; + } + } } else { throw err; } diff --git a/test/integration/esm.spec.js b/test/integration/esm.spec.js index 7cf1c39071..0986448992 100644 --- a/test/integration/esm.spec.js +++ b/test/integration/esm.spec.js @@ -1,8 +1,11 @@ 'use strict'; var path = require('path'); -var helpers = require('./helpers'); -var run = helpers.runMochaJSON; -var runMochaAsync = helpers.runMochaAsync; +const { + runMochaJSON: run, + runMochaAsync, + invokeMochaAsync, + resolveFixturePath +} = require('./helpers'); var utils = require('../../lib/utils'); var args = +process.versions.node.split('.')[0] >= 13 ? [] : ['--experimental-modules']; @@ -81,4 +84,22 @@ describe('esm', function() { expect(result, 'to have passed test count', 1); }); + + it('should throw an ERR_MODULE_NOT_FOUND and not ERR_REQUIRE_ESM if file imports a non-existing module', async function() { + const fixture = + 'esm/type-module/test-that-imports-non-existing-module.fixture.js'; + + const [, promise] = invokeMochaAsync( + [resolveFixturePath(fixture), '--unhandled-rejections=warn'], + {stdio: 'pipe'} + ); + const result = await promise; + + expect(result, 'to have failed with output', /ERR_MODULE_NOT_FOUND/); + expect( + result, + 'to have failed with output', + /test-that-imports-non-existing-module/ + ); + }); }); diff --git a/test/integration/fixtures/esm/type-module/package.json b/test/integration/fixtures/esm/type-module/package.json new file mode 100644 index 0000000000..aead43de36 --- /dev/null +++ b/test/integration/fixtures/esm/type-module/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} \ No newline at end of file diff --git a/test/integration/fixtures/esm/type-module/test-that-imports-non-existing-module.fixture.js b/test/integration/fixtures/esm/type-module/test-that-imports-non-existing-module.fixture.js new file mode 100644 index 0000000000..c75f4ee18e --- /dev/null +++ b/test/integration/fixtures/esm/type-module/test-that-imports-non-existing-module.fixture.js @@ -0,0 +1,6 @@ +import assert from 'assert'; +import './this-module-does-not-exist.js'; + +it('should not pass (or even run) because above import will fail', () => { + assert(true); +}); From 1d6a72b0e2630988d919307e45a403cb60e40a4f Mon Sep 17 00:00:00 2001 From: Gil Tayar Date: Tue, 13 Jul 2021 16:40:09 +0300 Subject: [PATCH 2/2] review comment fixes --- test/integration/esm.spec.js | 23 ++++++------------- .../fixtures/esm/type-module/package.json | 2 +- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/test/integration/esm.spec.js b/test/integration/esm.spec.js index 0986448992..7398af4ec5 100644 --- a/test/integration/esm.spec.js +++ b/test/integration/esm.spec.js @@ -1,11 +1,6 @@ 'use strict'; var path = require('path'); -const { - runMochaJSON: run, - runMochaAsync, - invokeMochaAsync, - resolveFixturePath -} = require('./helpers'); +const {runMochaJSON: run, runMochaAsync} = require('./helpers'); var utils = require('../../lib/utils'); var args = +process.versions.node.split('.')[0] >= 13 ? [] : ['--experimental-modules']; @@ -89,17 +84,13 @@ describe('esm', function() { const fixture = 'esm/type-module/test-that-imports-non-existing-module.fixture.js'; - const [, promise] = invokeMochaAsync( - [resolveFixturePath(fixture), '--unhandled-rejections=warn'], - {stdio: 'pipe'} - ); - const result = await promise; + const err = await runMochaAsync(fixture, ['--unhandled-rejections=warn'], { + stdio: 'pipe' + }).catch(err => err); - expect(result, 'to have failed with output', /ERR_MODULE_NOT_FOUND/); - expect( - result, - 'to have failed with output', - /test-that-imports-non-existing-module/ + expect(err.output, 'to contain', 'ERR_MODULE_NOT_FOUND').and( + 'to contain', + 'test-that-imports-non-existing-module' ); }); }); diff --git a/test/integration/fixtures/esm/type-module/package.json b/test/integration/fixtures/esm/type-module/package.json index aead43de36..3dbc1ca591 100644 --- a/test/integration/fixtures/esm/type-module/package.json +++ b/test/integration/fixtures/esm/type-module/package.json @@ -1,3 +1,3 @@ { "type": "module" -} \ No newline at end of file +}