Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task
NMinhNguyen opened this issue Jun 12, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@NMinhNguyen
Copy link

NMinhNguyen commented Jun 12, 2024

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

@NMinhNguyen NMinhNguyen added the bug Something isn't working label Jun 12, 2024
@adrian-gierakowski
Copy link

I need this as well

@adrian-gierakowski
Copy link

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
Copy link

adrian-gierakowski commented 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
Copy link

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

@JakobJingleheimer
Copy link

@joyeecheung
Copy link

joyeecheung commented 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_module now, so the patch @adrian-gierakowski came up with can probably be greatly simplified ;)

@joyeecheung
Copy link

cc @merceyz who authored #4024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants