🐛 Bug: esm-utils.js#requireModule() masks more helpful requireErr in some cases

[trentm]

Bug Report Checklist

• This is NOT a security, npm audit, or GitHub Advisory issue.
• I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
• I have searched for related issues and issues with the faq label, but none matched my issue.
• I have 'smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, my usage of Mocha, or Mocha itself.
• I want to provide a PR to resolve this

Expected

If esm-utils.js cannot know for sure if a require() or import() error is most relevant, perhaps both should be shown to the user with context?

Actual

When using ts-node for test files (e.g. with "require": ["ts-node/register/transpile-only"], in .mocharc.json), a tsconfig.json that specifies CommonJS, and a Node.js version that does type-stripping by default (Node.js 22 and later),
then any accidental case that results in the require('./test/something.test.ts') throwing an error,
will result in mocha's esm-utils.js#requireModule() attempting to import(...) the test file.

If the import(...) also fails, then the importErr is (often) reported and the requireErr is not shown.

There is some guessing trying to decide which of importErr or requireErr to show.
opentelemetry-js-contrib.git another case where requireErr is more relevant, but importErr is shown.
The full case is described here: open-telemetry/opentelemetry-js-contrib#3418 (comment)

I made a smaller repro here: https://github.com/trentm/repro-mocha-unsupported-dir-import

Minimal, Complete and Verifiable Example

https://github.com/trentm/repro-mocha-unsupported-dir-import

Versions

% npx mocha --version
11.7.5
% node --version
v24.14.0

The combination here requires:

• a mocha which includes feat: use require to load esm #5366
• Node.js that supports require(esm), Node.js 20 and later.
• Node.js that does type-stripping by default, Node.js 22 and later.

Additional Info

I suppose a specific exclude for importErr.code === 'ERR_UNSUPPORTED_DIR_IMPORT' could be added to requireModule(), but I'm not sure if that completes the set of import() error codes that suggest "oops, this was likely a CommonJS file that we tried to import()".

[trentm]

Though I didn't check it, I'm happy to attempt a PR for this after some discussion on the best approach.

[JoshuaKGoldberg]

Coming over from #5795, summarizing old context...

We'd talked about this back in #5235 too. Quoting #5235 (comment):

This is tricky. If both CJS and ESM fail to import the file because it actually doesn't exist, then displaying both errors would be extra noise for users. If this were to be implemented it would probably need some kind of heuristic for determining whether the errors are different enough to both warrant being displayed. I don't know how straightforward that would be to implement.

After that, #5393 was posted with a similar issue. Fun. Its proposal was a bit different: to still only show one error, and be a bit more intelligent about which one error to show. #5408 is one of several "use (specifically import or require error) if (specific type of error) is thrown" attempts. #5498 and #5572 were in that line as well.

I'm still in favor of trying to get all the relevant import codes. Showing both import and require() errors to users would be extra noise. And given that we know the specific case in this issue of ERR_UNSUPPORTED_DIR_IMPORT, it feels to me like we're not yet at the point of giving up and showing both errors.

But @mark-wiemer I could easily be convinced if you want to push for doing both fixes?

[mark-wiemer]

@JoshuaKGoldberg this whole "two import methods" challenge continues to make my head spin. I'm still recovering from a minor illness and travelling (just got home less than 24 hours ago) so it might be a few days before I can wrap my head around everything. The Node import system is something I've been trying to deeply understand for a long time now, but progress remains a bit slow. #5795 has my draft postmortem and is where I will continue posting my notes

Reviewing the discussion just now in the oldest related issue, #5235, was fun: Node's behavior was changing as folks were discussing things :D Note that most of the discussion was from 2024, and I didn't start contributing to Mocha until 2025. (#5027). We're now* well into 2026, and while* some things are more stable, I'm sure there are more nuances we're unaware of and changes we'll have to deal with.

In general, I am in favor of "more info in console logs, even if that means some noise." Especially with errors, because I hate getting an error and not knowing what's going on. Even a "noisy" console log is something to google ask Copilot about ;)

In this specific case, yes, it seems like we could add "one more bandaid." But I worry that there are other unreported cases that aren't being covered. I'm in favor of marking this one as "accepting PRs." I think this "two import methods" challenge will continue to arise, but adding this transparency to failures will help folks diagnose things much faster.

@JoshuaKGoldberg can you elaborate what you mean by "doing both fixes"?

[mark-wiemer]

@JoshuaKGoldberg a simple compromise: If we encounter a "known" scenario, we report the best error. If we encounter an unknown scenario, we report both errors.

[jedwards1211]

I'd also be happy to work on a PR for anything that would improve the error messages, I've run into this problem quite a bit. And I have too much experience dealing with CJS vs ESM lol.

[mark-wiemer]

@jedwards1211 if you'd like to build off #5803, you're more than welcome :)

I'm reopening #5235 for "long-term" discussion and marking this one as accepting PRs for a "bandaid" fix