fix(eslint): detect Yarn PnP in function form cmd

[Frederick888]

---

name: Pull Request
about: Submit a pull request
title: fix(eslint): detect Yarn PnP in function form cmd

c86804a3 fix(eslint): detect Yarn PnP in function form cmd

Problem:
In the new lsp/eslint.lua, the PnP detection in before_init did not work
at all as before_init cannot modify config.cmd [1].

Solution:
The code is moved to the new cmd function, which detects PnP by checking
.pnp.[c]js and prepends cmd with 'yarn exec' so that Node.js can load a
PnP project's ESLint library zip file for ESLint LS.

In the old lua/lspconfig/configs/eslint.lua, on_new_config still works,
though since [2] the new :Lsp* commands no longer utilise the old files.

Note since the cmd function does not have access to either bufnr or the
resolved root_dir [3], it only looks for .pnp.[c]js in current working
directory.

[1] https://github.com/neovim/neovim/blob/93925fe0/runtime/lua/vim/lsp/client.lua#L456-L466
[2] e4d1c8b
[3] https://github.com/neovim/neovim/blob/93925fe0/runtime/lua/vim/lsp.lua#L558

---

Closes #3858

Closes #3889

[justinmk]

Note since the cmd function does not have access to either bufnr or the resolved root_dir [3], it only looks for .pnp.[c]js in current working directory.

That is a confusing thing to mention. Even if cmd had access to bufnr/root-dir, it is starting the LS. Obviously, you wouldn't want to start a new LS for every buffer, so what good would it do for cmd to know the bufnr?

I still would like to confirm my question from #3889 (comment) : is this decision required as a CLI arg? Then if so, it makes sense to define cmd as you have done here. Please mention that in a one-line comment. And that also means there is no reason to mention that "cmd does not know bufnr", because that's irrelevant.

[justinmk]

Obviously, you wouldn't want to start a new LS for every buffer, so what good would it do for cmd to know the bufnr?

I think you may want to define a reuse_client function. Then cmd can use nvim_get_current_buf() instead of CWD ?

[Frederick888]

That is a confusing thing to mention. Even if cmd had access to bufnr/root-dir, it is starting the LS. Obviously, you wouldn't want to start a new LS for every buffer, so what good would it do for cmd to know the bufnr?

I agree it's not quite intuitive. Basically having bufnr/root_dir allows it to check if root_dir/.pnp.[c]js exists, so that users can e.g. open a file from any sub-directory in a project.

If you take my playground project in #3889, with this PR + the new vim.lsp.enable('eslint'), nvim -u repro.lua packages/shared/src/index.ts works while cd packages/shared/src; nvim -u ../../../repro.lua ./index.ts still has the 'Unable to find ESLint library' issue. (On the other hand with the old require('lspconfig').eslint.setup({}), both can work.)

I still would like to confirm my question from #3889 (comment) : is this decision required as a CLI arg?

Yes, somewhat. It's not an arg of the eslint LS itself but some args of node that yarn exec adds.

# still in my playground project
$ node
Welcome to Node.js v24.2.0.
Type ".help" for more information.
> process.env.NODE_OPTIONS
undefined

$ yarn exec node
Welcome to Node.js v24.2.0.
Type ".help" for more information.
> process.env.NODE_OPTIONS
'--require /home/frederick/Programming/Others/playground/.pnp.cjs --experimental-loader file:///home/frederick/Programming/Others/playground/.pnp.loader.mjs'

AFAIU yarn exec instructs node to load the .pnp.[c]js file upon start, which monkey patches require/import to allow loading dependency zip files, which replaces node_modules/ in Yarn PnP. So eslint LS itself probably just always calls const eslintLib = require('eslint') or something, but without these additional node args, node only looks for node_modules/, which is nowhere to be found in a Yarn PnP project.

I think you may want to define a reuse_client function. Then cmd can use nvim_get_current_buf() instead of CWD ?

I'm not familiar with this function but after having a glace at the docs, I assume it's something that gets called every time a new buffer of a related filetype (.js / .ts) is opened. So I assume you were talking about that in this fun(client: vim.lsp.Client, config: vim.lsp.ClientConfig): boolean, I can modify config: vim.lsp.ClientConfig and return false once (be careful of >1 existing eslint LS client), and here config.cmd modifications are actually effective since it happens before Client.create(config)?

If I'm not mistaken, unless reuse_client gets called when starting the first client too with (nil, config), it only helps when opening new files? Plus if a user does something like nvim package.json packages/shared/src/index.ts then :tab all, nvim_get_current_buf() may not return the buffer we want?

[justinmk]

I still would like to confirm my question from #3889 (comment) : is this decision required as a CLI arg?

Yes, somewhat. It's not an arg of the eslint LS itself but some args of node that yarn exec adds.

Nevertheless, a one-line comment should mention that.

unless reuse_client gets called when starting the first client too with (nil, config), it only helps when opening new files?

reuse_client is not used for the first client because cmd of course will always be invoked for the first client. Thereafter, reuse_client decides whether to re-invoke cmd.

[Frederick888]

Nevertheless, a one-line comment should mention that.

Done.

unless reuse_client gets called when starting the first client too with (nil, config), it only helps when opening new files?

reuse_client is not used for the first client because cmd of course will always be invoked for the first client. Thereafter, reuse_client decides whether to re-invoke cmd.

Then I think really it'd be nice if something simpler can come out of neovim/neovim#32287 that allows constructing cmd according to root_dir. Then the default behaviour of reuse_client, i.e. reuse if root_dir is the same, should just work nicely too.

[justinmk]

it'd be nice if something simpler can come out of neovim/neovim#32287 that allows constructing cmd according to root_dir. Then the default behaviour of reuse_client, i.e. reuse if root_dir is the same, should just work nicely too.

can you show a concrete example

meanwhile, why not use reuse_client in this config? Otherwise the CWD gets saved in cmd for all future buffers.