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_MODULE_NOT_FOUND with Yarn 4.6.0 #6646

Closed
1 task done
Methuselah96 opened this issue Jan 6, 2025 · 8 comments
Closed
1 task done

[Bug?]: ERR_MODULE_NOT_FOUND with Yarn 4.6.0 #6646

Methuselah96 opened this issue Jan 6, 2025 · 8 comments
Labels
bug Something isn't working

Comments

@Methuselah96
Copy link

Methuselah96 commented Jan 6, 2025

Self-service

  • I'd be willing to implement a fix

Describe the bug

Module resolution fails for some combination of CJS and ESM as of Yarn 4.6.0. It works when using Yarn 4.5.3 or npm (with both Node 22.12.0 and Node 22.11.0).

To reproduce

  1. Clone https://github.com/Methuselah96/test-jest
  2. Run yarn install
  3. Run yarn test
  4. Observe error:
Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'babel-jest' imported from C:\Users\nbier\Documents\test-jest\jestTransformer.js
Did you mean to import "babel-jest/build/index.js"?
    at packageResolve (node:internal/modules/esm/resolve:857:9)
    at moduleResolve (node:internal/modules/esm/resolve:926:18)
    at defaultResolve (node:internal/modules/esm/resolve:1056:11)
    at ModuleLoader.defaultResolve (node:internal/modules/esm/loader:654:12)
    at #cachedDefaultResolve (node:internal/modules/esm/loader:603:25)
    at ModuleLoader.getModuleJobForRequire (node:internal/modules/esm/loader:353:53)
    at new ModuleJobSync (node:internal/modules/esm/module_job:341:34)
    at ModuleLoader.importSyncForRequire (node:internal/modules/esm/loader:326:11)
    at loadESMFromCJS (node:internal/modules/cjs/loader:1414:24)
    at Module._compile (node:internal/modules/cjs/loader:1547:5)

Environment

System:
    OS: Windows 11 10.0.26100
    CPU: (20) x64 13th Gen Intel(R) Core(TM) i5-13500
  Binaries:
    Node: 22.12.0 - ~\AppData\Local\Temp\xfs-d16ac07e\node.CMD
    Yarn: 4.6.0 - ~\AppData\Local\Temp\xfs-d16ac07e\yarn.CMD
    npm: 10.9.0 - C:\Program Files\nodejs\npm.CMD
    bun: 1.1.0 - ~\.bun\bin\bun.EXE

Additional context

No response

@Methuselah96 Methuselah96 added the bug Something isn't working label Jan 6, 2025
@flex-jonghyen
Copy link

@clemyan Could you let me know there is any progress?

@clemyan
Copy link
Member

clemyan commented Feb 13, 2025

I have dug into this and determined this happens when all of the following is true

  • PnP is enabled
  • require(esm) is enabled (default for Node 22.12.0+)
  • module.registerHooks() is not available (as of this writing, Node <23.5.0)
  • require(esm) is used
  • the required esm has a static import to something that needs PnP to resolve (e.g. a dependency)

If all the the above is true, then the require(esm) call throws.

The PnP runtime injects itself into the Node process by monkey-patching the CJS module loader (e.g. Module._resolveFilename). As far as I can tell, the implementation of require(esm) bypasses this monkey-patch: static imports of the required ESM to go through neither the CJS loader pipeline nor the ESM loader pipeline (--loader/module.register()), making it so that no third-party code can inject itself into the resolution process. Fortunately, seems like the implementation of module.registerHooks() has "fixed" that and those imports go through the CJS loader once again. See nodejs/node#52697 (comment).

The reason this issue does not happen for Yarn 4.5.3 is due to how Jest loads the transformer — it attempts require-ing it first, then fallback to a dynamic import if an ERR_REQUIRE_ESM is thrown. The underlying assumption is that requiring an ESM would throw such an error, which fails to take into account new features in node like require(esm) or stripping types. See jestjs/jest#14013.

Support for require(esm) in PnP was added in Yarn 4.6.0 (via #6639). Before that, the PnP runtime itself would throw an ERR_REQUIRE_ESM. In Yarn 4.6.0, if require(esm) is enabled, instead we just fallback to Node's default behavior, which in the situation described, throws an ERR_MODULE_NOT_FOUND instead of an ERR_REQUIRE_ESM.

This can be fixed/worked around by:

  • jest fixing its loading logic
  • using a node version where --experimental-require-module is not default (at the time of this writing, <22.12.0)
  • running node with --no-experimental-require-module (setting it via NODE_OPTIONS if necessary)
  • using a node version where module.registerHooks() is available (as of this writing, >=23.5.0)
  • using a dynamic import instead: export default import('babel-jest').then((module) => module.default.createTransformer())

As far as I can tell, the only way to fix this issue on our side is to revert our support for require(esm), which breaks many other projects. As such, I'm closing this issue as a wontfix for now. If anyone has addition insight into the issue or can find a feasible solution on Yarn's side, we can re-open it.

@clemyan clemyan closed this as not planned Won't fix, can't repro, duplicate, stale Feb 13, 2025
@Methuselah96
Copy link
Author

Methuselah96 commented Feb 13, 2025

Thanks for the update. I was already wary of Yarn PnP needing to patch Node and TypeScript in order for it to function. I have a workaround to get my Jest example working, but this issue also causes my Storybook build to fail, and I haven't found a trivial workaround for that. It's probably time for me to move off of Yarn PnP, despite it's faster installation times.

@flex-jonghyen
Copy link

flex-jonghyen commented Feb 17, 2025

@clemyan

I feel difficult to understand about this decision.
I understood PnP need module.registerHooks() to support require(esm).
If it is, Is not it true that PnP does not yet support ⁠require(esm)?
Would not reverting the code added 4.6.0 be the right move?

(I am using translator, I apologize if my tone as aggressive.)

@clemyan
Copy link
Member

clemyan commented Feb 19, 2025

(This is a complicated topic and this is all based on my current understanding after doing some digging but I am open to being corrected and reconsider)

First, let's talk about jest. When jest tries to load a transformer, it first attempts to require it, then falls back to dynamically import it but only if the require failed with ERR_REQUIRE_ESM. The underlying assumption of that implementation is require(esm) would always fail with that error, which was the case before Node made require(esm) possible. However, currently, this logic is flawed as that is unable to support many features of Node itself, even without PnP in the mix:

  • require-ing an ESM with top-level await: Throws ERR_REQUIRE_ASYNC_MODULE
  • --loader / module.register: Static imports from an ESM invokes these hooks if it is import'ed, but not if it is require'd
  • module.registerHooks: These hooks can throw any error
  • package.json exports: Can make require('foo') and import 'foo' both work but with different results

just to name a few.


Now let's talk about Node. One of the things needed for Yarn PnP to work currently is the ability for userland code to inject itself into the resolution pipeline. When Node runs a require('foo') or import 'foo', PnP needs to insert itself between that and Node's default resolution pipeline. Node has (at least conceptually) two loader pipelines: synchronous and asynchronous. There are a number of ways to inject userland code into them:

  • Synchronous
    • Monkey-patching the pipeline (unofficial)
    • module.registerHooks (official, experimental)
  • Asynchronous
    • --loader / --import + module.register (official, experimental)

But there are many ways to load modules. For example, one may require an ESM that has a dynamic import('foo') "call". The question is then figure out what invokes which pipeline (again, as far as I understand; I haven't tested all of these):

Initial load \ Transitive load require static import dynamic import()
require Sync ?? Async
static import Async Async Async
dynamic import() Async Async Async

This issue is a case of requireing a module with a static import (the "??" case). The problem is that, for some versions of Node, this goes through neither the sync pipeline nor the async pipeline, making impossible for userland code to customize the import. Specifically, this happens on Node versions where module.registerHooks is not available (Node <23.5.0 as of this writing).

More generally, we are currently in a transitional period as the experimental loaders API gets iterated upon and a lot of churn is happening. Unfortunately, Node has not provided official alternatives to monkey-patching CJS until recently and there is little to no effort made to preserve the unofficial APIs (at no fault of anyone IMO) while fleshing out the loaders API.


So where does that leave us? How can we address this issue, if at all?

  • We can duplicate the entire Node loader pipeline in PnP?
    • Based on a cursory look at the loader pipeline, it uses a lot of internal functions not available in userland
    • Even if we can work around that, reimplementing the entire Node loader pipeline would be a huge maintenance effort, while simultaneously introduce massive bloat to both the Yarn binary and PnP hook for all users when the issue at hand only affect a small portion of users and has known workarounds
  • We can intercept the compile step during require, detect if an ESM is being imported, and rewrite the imports?
    • Needs to include an entire JS parsing and code transforming pipeline, so same problem with maintenance and bloat
    • Probably incur a massive performance hit because every ESM is parsed twice
  • We can revert fix(pnp): support require(esm) #6639?
    • While PnP does not support all cases of require(esm), there are project that uses the non-problematic cases and needs this change to run properly. It is not feasible for us to break those in order to workaround jest's flawed loading logic.
    • We cannot make everything work because Node has made that impossible. But fix(pnp): support require(esm) #6639 at least make it work outside of the problematic cases. Reverting that means we regress from "support require(esm) outside of some specific circumstances" to "does not support require(esm) at all".

All in all, it is impossible to fully support require(esm) at this moment (even without this issue because require(esm) is explicitly experimental). I believe there are no feasible way to fix this issue without breaking other (arguably more "correct") uses of PnP + require(esm), making this issue unactionable

Again, I'm open to being corrected and reconsider if someone has more insight into this.

@flex-jonghyen
Copy link

(I am using an AI translator. I checked the translated text, but I ask for your understanding.)

First of all, thank you for your detailed response.
I have carefully read your post and understand that this is not an easy problem.

Here’s my understanding:

  • In problematic cases (such as with Jest, Storybook, etc.), it was assumed that Node would fail with an ⁠ERR_REQUIRE_ESM error when attempting to ⁠require(esm) before Node supported it, but that is no longer the case.
  • The problem primarily occurs when doing a ⁠require(static import), because Node versions <23.5.0 do not provide ⁠module.registerHooks, making it impossible to handle this properly.
  • Although reverting fix(pnp): support require(esm) #6639 is an option, we have decided not to support ⁠require(esm) in all cases where it would work without issues, due to some problematic cases.

If my understanding is correct, it means that I am currently facing an actual problematic situation.

To resolve this issue, I would like to know more about the ⁠package.json exports that you mentioned.

As you mentioned, most of the problems are more pronounced when packages (or modules) being require(esm)d do not provide an exports field.

It seems that the only option I have is to resolve the issue with the problematic packages by patching them to include an exports field. I would be curious to hear your thoughts on this.

Thank you, as always, for your dedication to Yarn.

@clemyan
Copy link
Member

clemyan commented Feb 20, 2025

I mentioned package.json exports because it is something that jest's loading logic cannot fully support. I'm not saying it's a workaround.

Imagine I have a custom transformer:

// jest.config.js
export default {
  transform: {
    '^.+\\.(js|jsx|ts|tsx)$': 'my-transformer',
  },
};
{
  "name": "my-transformer"
  "exports": {
    ".": {
      "require": "./some.cjs",
      "import": "./other.mjs"
    },
    "./package.json": "./package.json"
  }
}

In this case, both require('my-transformer') and import('my-transformer') works, but uses two different modules as the transformer. Jest's loading logic just assumes require is intended, without even checking for import.


As far actual workarounds, the ones I listed in my original response should work:

  • Upgrade Node to version >=23.5.0; or
  • If you don't need require(esm), disable it
    • downgrade to Node <=22.11.0; or
    • run Node with --no-experimental-require-module

Patching a package's package.json with exports can work, but it is not very nice to maintain.

For your specific case, I would have to see a reproduction to help further.

@flex-jonghyen
Copy link

flex-jonghyen commented Feb 21, 2025

@clemyan

Thank your comment.
I understanded that exports field is not silver bullet of this problem.

Could I check a module in package that is type:module and has no exports field can resolve well?

I made reproduce of my case

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

3 participants