fix: revert "feat: use require to load esm (#5366)"

[legendecas]

PR Checklist

• Addresses an existing open issue:
  • 🐛 Bug: Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './esm-register' is not defined by "exports #5380
  • feat: use require to load esm #5366 (comment)
• That issue was marked as status: accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

This reverts commit 41e24a2.

[voxpelli]

Since pinning to previous version works I would say that we skip revert unless we have an estimated double digit percentage regressions (almost impossible to know though) or the estimated fix may be complex / impossible (if we can't get a fix out within a week)

Thoughts?

[legendecas]

From the issue tracker, there are non trivial amount of workflows were broken with #5366. I would prefer a revert and re-land it with proper test coverages (#5382).

[thatsmydoing]

Another argument for reverting is engines claims to support nodejs 18 but I don't think require for ESM is supported there at all https://nodejs.org/api/modules.html#loading-ecmascript-modules-using-require (and you get a different error when you try to do this).

It also needs a flag --experimental-require-module prior to nodejs 20.19 and 22.12 which is also newer than the listed engines.

[legendecas]

Another argument for reverting is engines claims to support nodejs 18.

This was covered by process.features.require_module, which is only present after v20.19.0.

[voxpelli]

Another argument for reverting is engines claims to support nodejs 18 but I don't think require for ESM is supported there at all https://nodejs.org/api/modules.html#loading-ecmascript-modules-using-require (and you get a different error when you try to do this).

It also needs a flag --experimental-require-module prior to nodejs 20.19 and 22.12 which is also newer than the listed engines.

@thatsmydoing It is properly feature flagged and a progressive enhancement:

mocha/lib/nodejs/esm-utils.js

Lines 106 to 110 in 5d617f3

	if (process.features.require_module) {
	exports.requireOrImport = requireModule;
	} else {
	exports.requireOrImport = tryImportAndRequire;
	}

---

@legendecas Of course, I don't want to put undue stress on eg. you, but I also do think that we can eg. fix some of this by what I mentioned in #5380 (comment), in other words add a || err.code === 'ERR_PACKAGE_PATH_NOT_EXPORTED' to here:

mocha/lib/nodejs/esm-utils.js

Lines 96 to 98 in 5d617f3

	if (
	err.code === 'ERR_REQUIRE_ASYNC_MODULE'
	) {

[thatsmydoing]

Ah sorry, I had missed that. That said, it does seem like require ESM interacts differently with loaders across node versions. For example, #5388 seems to work fine in node 22 and up but not in node 20.19.2 so there seem to be other complicating factors.

[voxpelli]

Yes, it's tracked across #5380, #5382, #5387, #5388 as well as a few PR:s

If someone can unify them that would be great

[JoshuaKGoldberg]

I'm trying to fix the root issue. I agree with @voxpelli that if we can fix this quickly, we should just push that as a fix. Will post back soon with some findings.

[Borewit]

Ah sorry, I had missed that. That said, it does seem like require ESM interacts differently with loaders across node versions. For example, #5388 seems to work fine in node 22 and up but not in node 20.19.2 so there seem to be other complicating factors.

Issues is not present in Node version ≥ 22.15.0. It is reproducable with 22.14.0 and older.

[JoshuaKGoldberg]

Confirmed that this would fix the reported issues. However, since #5384 fixes the issue without a revert, I think we should go with #5384.

[legendecas]

Closing as #5384 landed.

[JoshuaKGoldberg]

@legendecas I do want to say though, you've been really helpful & very proactive - we really appreciate it. Thanks for helping us with this very tricky area of code! ❤️