fix(pnp): throw ERR_REQUIRE_ESM when requiring an ES Module

[merceyz]

What's the problem this PR addresses?

Calling require on an ES Module throws SyntaxError: Cannot use import statement outside a module instead of ERR_REQUIRE_ESM

Fixes #4016

How did you fix it?

If the .js file is in a module context (package.json contains type: module) throw ERR_REQUIRE_ESM.

Checklist

• I have read the Contributing Guide.
• I have set the packages that need to be released for my changes to be effective.
• I will check that all automated PR checks pass before the PR gets reviewed.

[merceyz]

This fails without this PR when run outside of the CI where file watchers are enabled.

[paul-soporan]

I know it's taken from the Node codebase but ending the line with === is quite an unusual pattern for ours.

[merceyz]

Indeed, our linting/formatting setup should be updated to handle this.

[swandir]

I'm curious is this problem unique to cjs loader specifically?

Does this happen because Node.js can't locate corresponding package.json for an import resolved through PnP?

[merceyz]

Yes, the issue is that it uses internalModuleReadJSON which we can't patch so it's unable to read the zip archives. Ref nodejs/node#33423

https://github.com/nodejs/node/blob/3743406b0a44e13de491c8590386a964dbe327bb/lib/internal/modules/cjs/loader.js#L1121
https://github.com/nodejs/node/blob/3743406b0a44e13de491c8590386a964dbe327bb/lib/internal/modules/cjs/loader.js#L329
https://github.com/nodejs/node/blob/3743406b0a44e13de491c8590386a964dbe327bb/lib/internal/modules/cjs/loader.js#L296
https://github.com/nodejs/node/blob/5f2bb885f8027e2789a1fa8679d242162b4a8633/lib/internal/modules/package_json_reader.js#L21

[swandir]

Thanks!

A somewhat similar issue affect bundlers, which too want to locate package.json (and maybe other configs) to handle advanced resolution.

In theory this kind of things should be handled via plugins providing custom resolution, but it can be tricky (vitejs/vite#6493 (comment)) in practice.

I guess per-module resolution hooks are not completely sufficient abstraction for implementing PnP support?

I wonder whether the best approach is to try handling such edge cases in pnp bundler plugins, or rather to add an ability to read zip archives into a bundler 🤔

[merceyz]

As long as projects use require.resolve and fs to interact with dependencies it should just work out of the box

[swandir]

Yes, the problem is non-js tooling like esbuild that is more common recently.

[arcanis]

Next.js fallbacks to the Node API when it detects the path matches a pattern.