Repository navigation
[Bug?]: ERR_REQUIRE_ESM in Yarn PnP mode using Node.js 22 with --experimental-require-module flag #6336
Description
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 startEnvironment
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/pnpmAdditional context
Would expect this to work, thanks to nodejs/node#51977.
Disabling Yarn PnP works: https://codesandbox.io/p/devbox/nervous-leavitt-forked-9mgdvt
Reacted by Utwo
Activity
adrian-gierakowski commentedon Oct 9, 2024
I need this as well
adrian-gierakowski commentedon Oct 14, 2024
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-modulenode option is detected?adrian-gierakowski commentedon Oct 16, 2024
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 commentedon Oct 18, 2024
This flag is now enabled by default in node 23 so I guess this issue should be prioritised
JakobJingleheimer commentedon Dec 3, 2024
cc @arcanis nodejs/node#56040 (comment)
joyeecheung commentedon Dec 3, 2024
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_modulenow, so the patch @adrian-gierakowski came up with can probably be greatly simplified ;)joyeecheung commentedon Dec 3, 2024
cc @merceyz who authored #4024
merceyz commentedon Dec 23, 2024
Should be fixed by #6639, sorry for not getting to this sooner.
We only throw when the extension is
.jsandpkg.type === 'module'.The test that started failing after unflagging
require(esm)requires a.mjsfile which we didn't need to handle as the correct error was thrown by Node.js already.fix(pnp): support `require(esm)` (yarnpkg#6639)