[Bug?]: `ERR_REQUIRE_ESM` in Yarn PnP mode using Node.js 22 with `--experimental-require-module` flag

[NMinhNguyen]

Self-service

• I'd be willing to implement a fix

Describe the bug

ERR_REQUIRE_ESM when requiring an ES module from CommonJS using Yarn PnP with Node.js 22 with --experimental-require-module flag.

To reproduce

https://codesandbox.io/p/devbox/nervous-leavitt-ndnn53

yarn start

Environment

System:
  OS: Linux 6.1 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
  CPU: (2) X64 AMD EPYC
Binaries:
  Node: 22.3.0 - / tmp/xfs-d3661e12/node
  Yarn: 4.3.0 - /tmp/xfs-d3661e12/yarn
  npm: 10.8.1 - /usr/local/share/nvm/versions/node/v22.3.0/bin/npm
  pnpm: 8.15.6 - /usr/local/share/npm-global/bin/pnpm

Additional context

Would expect this to work, thanks to nodejs/node#51977.

Disabling Yarn PnP works: https://codesandbox.io/p/devbox/nervous-leavitt-forked-9mgdvt

[adrian-gierakowski]

I need this as well

[adrian-gierakowski]

I did a quick experiment, simply commenting out this code from the .pnp.cjs file which allowed me to require an ESM module from repl started with yarn node

@arcanis would you accept a PR disabling this error if --experimental-require-module node option is detected?

[adrian-gierakowski]

I now have this patch, created on top of @yarnpkg/cli/4.5.0, which I'm using in my project and everything works so far.

I copied some helpers from loader/node-options.js into nodeUtils.ts so that I can check if experimental-require-module (I guess there should be a way to avoid this duplication). I'm also checking that node version is >= 20.17.0 (which is the version in which this flag was added)

happy to submit this as a PR

[adrian-gierakowski]

This flag is now enabled by default in node 23 so I guess this issue should be prioritised

[JakobJingleheimer]

cc @arcanis nodejs/node#56040 (comment)

[joyeecheung]

The related tests are failing in CITGM: nodejs/node#56040 (comment) - already failing in v23 for a couple of months, and will fail soon on v22.x too after v22.12.0 unflags require(esm), I suppose if it never gets fixed we should consider removing yarn from CITGM until it gets fixed to keep CITGM clean, since it's a known issue that doesn't affect most usage of yarn, but rather should be considered a bug in yarn when pnp + require(esm) is used, since the monkey patching code is supposed to match the default behavior, which now no longer throws on require(esm).

Also IIUC the tests seem to be buggy as well? From #4024 they seem to be testing that ERR_REQUIRE_ESM is thrown by yarn when pnp is used, so should've been unaffected by what the default loading does. Yet it is, which means the tests are bypassing pnp which is what it tries to test.

By the way you can do the feature detection using process.features.require_module now, so the patch @adrian-gierakowski came up with can probably be greatly simplified ;)

[joyeecheung]

cc @merceyz who authored #4024

[merceyz]

Should be fixed by #6639, sorry for not getting to this sooner.

Also IIUC the tests seem to be buggy as well? From #4024 they seem to be testing that ERR_REQUIRE_ESM is thrown by yarn when pnp is used, so should've been unaffected by what the default loading does. Yet it is, which means the tests are bypassing pnp which is what it tries to test.

We only throw when the extension is .js and pkg.type === 'module'.
The test that started failing after unflagging require(esm) requires a .mjs file which we didn't need to handle as the correct error was thrown by Node.js already.