berry icon indicating copy to clipboard operation
berry copied to clipboard

[Bug?]: Yarn pnp fails with synckit and prettier plugin in eslint context

Open dbaeumer opened this issue 1 year ago • 20 comments

Self-service

  • [ ] I'd be willing to implement a fix

Describe the bug

Yarn pnp can't load synckit worker hence prettier plugin fails to run.

Why do I file this against yarn? Everything works as expected when installing the dependencies with npm.

To reproduce

Use NodeJS 20.9.0

Clone https://github.com/shermify/eslint-plugin-repro

> yarn install
> export NODE_PATH=".yarn/sdks"

Setting the NODE_PATH is equivalent to setting the VS Code eslint.nodePath setting.

start node REPL and execute the following commands (to ensure to mimic the executing since a VS Code extension):

require.resolve('eslint');

We will load the eslint npm module from .yarn/sdks

const library = require('eslint');
async function main() { const eslint = new (await library.loadESLint())(); const report = await eslint.lintText(`var test = "hello";`); console.log(report); }
main().then(() => { console.log('Promise resolved'); } ).catch((error) => { console.error(error); process.exitCode = 1; });

Observe: the promise never resolves. You can't even terminate NodeJS using Ctrl+C. You have to kill it.

I debugged it in the context of the VS Code ESLint extension and the findings are here: https://github.com/microsoft/vscode-eslint/issues/1856#issuecomment-2162280404 and in the following comments.

Environment

System: OS: Linux 5.15 Debian GNU/Linux 11 (bullseye) 11 (bullseye) CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12700K Binaries: Node: 20.9.0 - /tmp/xfs-426f4fe7/node Yarn: 4.3.0 - /tmp/xfs-426f4fe7/yarn npm: 10.1.0 - ~/.nvs/default/bin/npm pnpm: 9.1.1 - /usr/local/share/npm-global/bin/pnpm

Additional context

No response

dbaeumer avatar Jun 12 '24 07:06 dbaeumer

The problem seems to be a dynamic import in worker.js in the package eslint-plugin-prettier. I tried a few fixes and I could only get it to work using require. Dynamic import seems to lock up the node process.

await import('prettier') /fail await import('prettier/index.cjs') /fail (await import('prettier')).default /fail const prettier = require('prettier') /works

shermify avatar Jun 13 '24 01:06 shermify

Observe: the promise never resolves. You can't even terminate NodeJS using Ctrl+C. You have to kill it.

I don't repro that; is something missing? 🤔

Screenshot 2024-06-13 at 10 47 14

Outside of that, did you try with a newer Node release? If the whole process ends up locked it could be a Node.js bug.

arcanis avatar Jun 13 '24 08:06 arcanis

It works with node 18.18.2 (https://github.com/microsoft/vscode-eslint/issues/1856#issuecomment-2162347554) I've tested with 18.20.0; 20.9.0; 20.14.0 (LTS) and i can still reproduce it. if i set nodeLinker: node-modules the promise gets resolved, doesn't matter which node version it is in this case. Doesn't that mean it's yarn pnp related?

I'm not sure if these debug logs are helpful but this is what i did as well (https://github.com/microsoft/vscode-eslint/issues/1856#issuecomment-2160957041)

hashjona avatar Jun 13 '24 11:06 hashjona

I tested this again and the steps provided makes it hang for me. I tested this under Ubuntu 22.04 in case this makes a difference.

dbaeumer avatar Jun 13 '24 13:06 dbaeumer

Hangs for me under Windows as well.

dbaeumer avatar Jun 13 '24 13:06 dbaeumer

@arcanis is there anything I can capture that might help you to understand what is going on?

dbaeumer avatar Jun 13 '24 13:06 dbaeumer

if i set nodeLinker: node-modules the promise gets resolved, doesn't matter which node version it is in this case. Doesn't that mean it's yarn pnp related?

Yes and no. The Node.js ESM loader implementation is quite fluctuating, so bugs sometimes appear and get fixed on later versions (for instance some 22.x versions introduce a deadlock with some other loaders). As a result problems are very rarely about PnP itself, but more the APIs we're given to integrate with ESM in Node.

is there anything I can capture that might help you to understand what is going on?

I managed to repro - I was running yarn node (which translates into node --require ./.pnp.cjs --loader ./.pnp.loader.mjs), but in your repro it's just a good old node. It shouldn't make a difference, but perhaps Node regressed and is somehow registering the loaders twice even when they have the same path? Investigating.

arcanis avatar Jun 13 '24 13:06 arcanis

The relevant error here is visible when running yarn unplug eslint-plugin-prettier inside the repository. After that, the REPL starts to properly reject the promise with an ERR_MODULE_NOT_FOUND not found.

Digging into the source code, I see that this error is thrown by a worker thread spawned just so eslint-plugin-prettier can synchronously and conditionally require prettier. The SDK doesn't currently inject the --require and --loader flag in a way that would work through nested workers, so that explains the issue. I'll look what we can do.

As for the deadlock, I have no idea - given that it works after running yarn unplug (ie after materializing the eslint-plugin-prettier files on disk, rather than by loading them from their zip archive) I suspect something in the Node.js error handling code is trying to access the files on disk and doesn't support memory modules, which seems a bug to me.

arcanis avatar Jun 14 '24 12:06 arcanis

Do you know why, when it's unplugged, the dynamic import still doesn't seem to work but it does work when you change it to require? I would guess that it still needs a module loader in scope in both cases. Also, if you change it to require and eslint-plugin-prettier is still "plugged", you still get the deadlock. So something about unplugging it allows you to access the commonjs loader? Or possibly prettier (unplugged) can only dynamic import other unplugged modules? Apologies if I'm missing something obvious here, this is a bit outside my wheelhouse :)

shermify avatar Jun 14 '24 19:06 shermify

Do you know why, when it's unplugged, the dynamic import still doesn't seem to work but it does work when you change it to require?

Because the Node.js ESM loader pipeline is still broken 😞

Digging into the issue, it seems the --loader flag that SyncKit adds is ignored by the new Worker constructor, which seems to be the subject of https://github.com/nodejs/node/issues/47747. That's what causes the ERR_MODULE_NOT_FOUND error when dynamically importing prettier, which in turns causes a deadlock probably because of a faulty error handling code somewhere in the Node.js internals.

I opened an issue on SyncKit (https://github.com/un-ts/synckit/issues/174) to suggest a workaround.

arcanis avatar Jun 17 '24 08:06 arcanis

sorry if this is just noise, but I believe I'm seeing a similar issue when comsiconfig attempts to dynamic import a stylelint plugin:

https://github.com/stylelint/vscode-stylelint/issues/464#issuecomment-2174603745

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/herockk/Workspaces/example/.yarn/__virtual__/stylelint-prettier-virtual-4b73a1c895/3/.yarn/berry/cache/stylelint-prettier-npm-5.0.0-f95c073012-10c0.zip/node_modules/stylelint-prettier/recommended.js' imported from /Users/herockk/Workspaces/example/.yarn/__virtual__/cosmiconfig-virtual-a7403f58a3/3/.yarn/berry/cache/cosmiconfig-npm-9.0.0-47d78cf275-10c0.zip/node_modules/cosmiconfig/dist/loaders.js
    at new NodeError (node:internal/errors:406:5)
    at finalizeResolution (node:internal/modules/esm/resolve:233:11)
    at moduleResolve (node:internal/modules/esm/resolve:852:10)
    at defaultResolve (node:internal/modules/esm/resolve:1050:11)
    at nextResolve (node:internal/modules/esm/hooks:833:28)
    at resolve$1 (file:///Users/herockk/Workspaces/example/.pnp.loader.mjs:1976:12)
    at nextResolve (node:internal/modules/esm/hooks:833:28)
    at Hooks.resolve (node:internal/modules/esm/hooks:278:30)
    at MessagePort.handleMessage (node:internal/modules/esm/worker:168:24)
    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:807:20)

The module loads when stylelint-prettier is unplugged.

kherock avatar Jun 26 '24 20:06 kherock

Hi @arcanis, Any updates on this? It seems the https://github.com/un-ts/synckit/issues/174 issue has been fixed

agarwalvaibhav0211 avatar Jul 15 '24 09:07 agarwalvaibhav0211

@agarwalvaibhav0211 yes it has been fixed with latest release of synckit (https://github.com/un-ts/synckit/releases/tag/v0.9.1) which is a dependency of eslint-plugin-prettier. Just make sure you have [email protected] in your packages. If reinstalling eslint-plugin-prettier package doesn't fetch latest synckit then you can enforce it through yarnresolutions like this:

"resolutions": {
  "synckit": "^0.9.1"
}

fmal avatar Jul 17 '24 08:07 fmal

[email protected] has bumped synckit to 0.9.1

hanyufoodles avatar Jul 18 '24 12:07 hanyufoodles

Not sure where the true problem lies, but after getting everything else working with pnp, seems like I'm going to have to throw in the towel and go back to node-modules linker anyways.

I'm on the following versions

  • node: 22.14.0
  • yarn: 4.6.0
  • eslint-plugin-prettier: 5.2.3
  • eslint-config-prettier: 10.0.1
  • synckit: 0.9.2 (tried 0.9.1) as well

I've backed down to [email protected] & [email protected] to no avail

I can't even run the eslint command directly with yarn lint in my scripts. I'm not sure how over in vscode gh issue others got it working (many comments keep conflating it with vscode as well)... yarn unplug eslint-plugin-prettier doesn't work either

It's allegedly both fixed in the node issue & from synckit but no luck for me

marcebdev avatar Feb 22 '25 00:02 marcebdev

This is still an issue as of 0.11.0.

Repro:

  • check out https://github.com/tpict/vscode-eslint-pnp-flat-repro
  • run yarn lint and observe various lint errors
  • open one of the failing files in VSCode
  • check ESLint output; observe no squiggly lines even though the library & config are successfully loaded
[Info  - 12:24:13 PM] ESLint server is starting.
[Info  - 12:24:14 PM] ESLint server running in node v20.18.3
[Info  - 12:24:14 PM] ESLint server is running.
[Info  - 12:24:15 PM] ESLint library loaded from: /Users/tpict/Projects/eslint-example/.yarn/sdks/eslint/lib/api.js
loaded the config!

Then update the nodeLinker setting:

diff --git a/.yarnrc.yml b/.yarnrc.yml
index a398e49..91b1101 100644
--- a/.yarnrc.yml
+++ b/.yarnrc.yml
@@ -2,4 +2,4 @@ compressionLevel: mixed
 
 enableGlobalCache: false
 
-nodeLinker: pnp
+nodeLinker: node-modules

and run yarn again, restart VSCode, observe squiggly lines in failing files.

tpict avatar May 14 '25 14:05 tpict

-nodeLinker: pnp +nodeLinker: node-modules

Switching from Plug'n'Play to node-modules is not a solution, it might be considered a workaround but, it is going backwards to legacy methodology, not forwards.

node-modules has it's own set of annoying issues which PnP solves. https://yarnpkg.com/features/pnp

Fixing the root compatibility issue, which only appears only in the newest version of Yarn (it was working before), is a real solution.

sheldonmaschmeyer avatar Jun 02 '25 15:06 sheldonmaschmeyer

@agarwalvaibhav0211 yes it has been fixed with latest release of synckit (https://github.com/un-ts/synckit/releases/tag/v0.9.1) which is a dependency of eslint-plugin-prettier. Just make sure you have [email protected] in your packages. If reinstalling eslint-plugin-prettier package doesn't fetch latest synckit then you can enforce it through yarnresolutions like this:

"resolutions": { "synckit": "^0.9.1" }

This solution, using { "synckit": "^0.11.8" }, does not work. I still have to disable eslint-plugin-prettier

SheldonWBM avatar Jun 02 '25 16:06 SheldonWBM

Fixing the root compatibility issue, which only appears only in the newest version of Yarn (it was working before), is a real solution.

@sheldonmaschmeyer right, it wasn’t posed as a solution, but as a demonstration that the fault is with the pnp linker

tpict avatar Jun 02 '25 16:06 tpict

I was able to fix this issue in my project by downgrading eslint-plugin-prettier to v5.3.1 from v5.4.0 (also affected is v5.4.1): https://github.com/retrixe/IveBot/commit/4550fecf2a96c52606e91a411a4d8b51bbea71f5

This suggests to me the bug was introduced (reintroduced?) by some behavioural change caused by https://github.com/prettier/eslint-plugin-prettier/commit/59a0cae5f27801d7e00f257c6be059a848b32fbe (the only commit in v5.4.0)

retrixe avatar Jun 07 '25 10:06 retrixe

Bumping this issue. @retrixe suggestion to "downgrade to v5.3.1" solves the reocurrence of the bug.

Martizs avatar Jul 20 '25 10:07 Martizs

Wanted to chime in that I'm encountering this issue as well. My environment details are below (if there's any other information that's necessary, I can grab it):

I was originally encountering the issue with [email protected].

Workarounds which got the eslint extension working again:

  • Downgrading to [email protected] (as suggested previously in this thread)
  • Switching the linker mode to pnpm (which did not require downgrading the eslint-plugin-prettier package version)

bskinner avatar Aug 05 '25 20:08 bskinner

too have a problem

leshasmlesha avatar Oct 04 '25 16:10 leshasmlesha

Okay I finally took some time to get to the bottom of this

There is a bug in synckit support for PnP ESM, which causes the worker thread to fail and hangs the main thread indefinitely.

I have opened a PR over at synckit: https://github.com/un-ts/synckit/pull/255. Applying that as a patch to synckit should fix this.

clemyan avatar Oct 28 '25 14:10 clemyan