Skip to content

Commit

Permalink
fix: wrong error thrown if loader is used (mochajs#4807)
Browse files Browse the repository at this point in the history
  • Loading branch information
giltayar authored and KuznetsovRoman committed Sep 6, 2022
1 parent fe0ae17 commit 79622b8
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 4 deletions.
21 changes: 18 additions & 3 deletions lib/nodejs/esm-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,30 @@ exports.requireOrImport = hasStableEsmImplementation
err.code === 'ERR_UNSUPPORTED_DIR_IMPORT'
) {
try {
// Importing a file usually works, but the resolution of `import` is the ESM
// resolution algorithm, and not the CJS resolution algorithm. So in this case
// if we fail, we may have failed because we tried the ESM resolution and failed
// So we try to `require` it
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,
if (
requireErr.code === 'ERR_REQUIRE_ESM' ||
(requireErr instanceof SyntaxError &&
requireErr
.toString()
.includes('Cannot use import statement outside a module'))
) {
// ERR_REQUIRE_ESM 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,
// 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.
//
// SyntaxError happens when in an edge case: when we're using an ESM loader that loads
// a `test.ts` file (i.e. unrecognized extension), and that file includes an unknown
// import (which thows an ERR_MODULE_NOT_FOUND). require-ing it will throw the
// syntax error, because we cannot require a file that has import-s.
throw err;
} else {
throw requireErr;
Expand Down
21 changes: 21 additions & 0 deletions test/integration/esm.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,25 @@ describe('esm', function () {
'test-that-imports-non-existing-module'
);
});

it('should throw an ERR_MODULE_NOT_FOUND and not ERR_REQUIRE_ESM if file imports a non-existing module with a loader', async function () {
const fixture =
'esm/loader-with-module-not-found/test-that-imports-non-existing-module.fixture.ts';

const err = await runMochaAsync(
fixture,
[
'--unhandled-rejections=warn',
'--loader=./test/integration/fixtures/esm/loader-with-module-not-found/loader-that-recognizes-ts.mjs'
],
{
stdio: 'pipe'
}
).catch(err => err);

expect(err.output, 'to contain', 'ERR_MODULE_NOT_FOUND').and(
'to contain',
'non-existent-package'
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import path from 'path'
import {fileURLToPath} from 'url'

/**
* @param {string} specifier
* @param {{
* conditions: !Array<string>,
* parentURL: !(string | undefined),
* }} context
* @param {Function} defaultResolve
* @returns {Promise<{ url: string }>}
*/
export async function resolve(specifier, context, defaultResolve) {
const extension = path.extname(
fileURLToPath(/**@type {import('url').URL}*/ (new URL(specifier, context.parentURL))),
)
return await defaultResolve(specifier.replace('.ts', '.mjs'), context, defaultResolve)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'non-existent-package';
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This file will be resolved to `test-that-imports-non-existing-module.fixture.mjs` by the loader
import 'non-existent-package';
6 changes: 5 additions & 1 deletion test/integration/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,11 @@ function createSubprocess(args, done, opts = {}) {
* @returns {string} Resolved filepath
*/
function resolveFixturePath(fixture) {
if (path.extname(fixture) !== '.js' && path.extname(fixture) !== '.mjs') {
if (
path.extname(fixture) !== '.js' &&
path.extname(fixture) !== '.mjs' &&
path.extname(fixture) !== '.ts'
) {
fixture += '.fixture.js';
}
return path.isAbsolute(fixture)
Expand Down

0 comments on commit 79622b8

Please sign in to comment.