🚀 Feature: if loading a file as CJS and ESM both fail, display both errors instead of just ESM

jedwards1211 commented 1 month ago

Feature Request Checklist

[x] I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
[x] I have searched for related issues and issues with the faq label, but none matched my issue.
[x] I want to provide a PR to resolve this

Overview

Ever since chai v5 dropped support for CJS, we get errors like this when we accidentally install chai 5+:

$ mocha 

 Exception during run: TypeError: Unknown file extension ".ts" for /Users/user/src/battalion/storage-site-sim/src/__tests__/SeededRNG.test.ts
    at Object.getFileProtocolModuleFormat [as file:] (node:internal/modules/esm/get_format:160:9)
    at defaultGetFormat (node:internal/modules/esm/get_format:203:36)
    at defaultLoad (node:internal/modules/esm/load:141:22)
    at ModuleLoader.load (node:internal/modules/esm/loader:409:7)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:291:45)
    at link (node:internal/modules/esm/module_job:76:21) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}

This threw us for a loop at first, because normally our tests work fine with @babel/register handling .ts files.

What's happening is first Mocha tries to load our file as CJS, which make it as far as require('chai') which fails with ERROR_REQUIRE_ESM. Then Mocha tries to load our file as ESM instead, which triggers the above error.

Alternatives

I've thought about adding code to our toolchain to check for chai v5+ and explicitly warn...but this same issue could occur with any package we're using in tests that drops CJS support

I guess another solution would be a mocharc option to force Mocha to only try one of CJS or ESM, not both.

I can't just put type: "commonjs" in my package.json because I made my toolchain run tests in CJS mode with @babel/register, and then in ESM mode with babel-register-esm, since I'm transpiling my TypeScript source to both CJS and ESM for publishing.

Additional Info

No response

JoshuaKGoldberg commented 3 weeks ago

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.

Would you be able to post a minimum reproduction project we could look at to better understand this?

jedwards1211 commented 3 weeks ago

Okay I'll work on making a repro soon

jedwards1211 commented 3 weeks ago

I don't think the extra noise would be worse, because what it currently displays is very confusing at first in a situation like this.

Theoretically the heuristic could be whether ERR_REQUIRE_ESM came from the user's own project files or from node_modules

mochajs / mocha