feat: improve monorepo support for Typescript and ESLint LSP

[fuegoio]

Hello 👋

Monorepos (or "workspaces") in Typescript are more and more popular and the associated tooling is evolving to improve the developer experience in such setup. Especially, the typescript-language-server and the vscode-eslint-language-server now supports monorepos, removing the need to spawn a different server for each package of a workspace.

This was my main issue and why I iterated on this: with a few packages as the servers need to load every other package to work (the typescript-language-server, even if spawned multiple times with different root_dir, will load in memory other packages to resolve the types), the amount of memory used grows exponentially.

But in fact, those servers support monorepos: they support multiple configurations in subpackages and will load the correct one to process a buffer. The ESLint server even supports loading multiple ESLint binaries (and therefore versions), while keeping one instance of the server.

Solution

To enable this, instead of only relying on the configuration files as root_markers, we need to instead look at the root of the package (in case of a single project) / monorepo. To me, the best way to get this is to look at the Lock files created by node package managers:

• package-lock.json: Npm
• yarn.lock: Yarn
• pnpm-lock.yaml: Pnpm
• bun.lockb: Bun

We still need to look at configuration files to enable the conditionnaly attachment of the LSP for a buffer (for ESLint, we want to attach the LSP only if there are ESLint configuration files) in case of LSP that operates on files that are "generic" (like typescript or javascript).

To do that, I replace the root_markers that were the configuration files by a root_dir function that superseds them. It will both:

• look for a configuration file upward to check if the LSP needs to be attached
• look for the root of the "project" via the lock files to specify the root_dir of the LSP

Prior experimentations

I've tried to play with the reuse_client quite a lot, trying to understand if we need to spawn a new server or not looking at the Typescript / ESLint binary that was loaded, but in fact it's way easier to just have a better root_dir that is the true root of the project for the LSP server: in case of those two servers, the root of the package / monorepo.

I also tried to use the current directory opened as the root_dir, but it's less powerful on nvim compared to VSCode as we navigate more inside folders using terminal commands and then open vim.

I think this method also removes the need from a project-local config (which could be quite useful anyway for ESLint flat config setting which auto-detection is a bit unreliable / compute heavy) as this should work normally accross all different setups.

Fixes #3910.

[igorlfs]

Hello,

To me, the best way to get this is to look at the Lock files created by node package managers

If this solution is desired, it probably makes sense to also follow this rule for other LSPs in JS ecosystem (e.g., vtsls)

[justinmk]

This looks like a nice change, thanks.

before_init is usually sus but I didn't study the whole config so can't say for sure.

[fuegoio]

Thanks! I'm gonna take your feedbacks and also apply this on the vtsls LSP (I was not aware of it).

[igorlfs]

and also apply this on the vtsls LSP (I was not aware of it).

I mentioned vtsls only as an example, but it might be worth it to investigate other servers (there are plenty that rely on the package.json). I know that the svelte LSP also has support for monorepos (related #3491), can you also include it here?

[antonk52]

I agree that picking workplace root is a good call. I think we should still check if an eslintrc file or its variation is present before enabling the language server. The current implementation can potentially introduce an issue in a project where you use a particular linter/formatter. If in your config you have two linters/formatters enabled by locating a lock file, both of them both will get attached to a buffer and the user may see potentially conflicting diagnostics or formatting suggestions

To clarify we already do this for this language server in on_init, just highlighting that if we decide to do this for others we should still check for the presence of their config files

[justinmk]

I think we should still check if an eslintrc file or its variation is present before enabling the language server

Ok, waiting on that then. And my other comments. @fuegoio do you plan to finish this

[fuegoio]

I mentioned vtsls only as an example, but it might be worth it to investigate other servers (there are plenty that rely on the package.json). I know that the svelte LSP also has support for monorepos (related #3491), can you also include it here?

This technique only works for LSPs that are supporting "monorepos", ie that are supporting multiple configurations of the library (like multiple tsconfig.json or eslint.config.ts), this is why I only modified those ones. I think we need to go from there one server at a time where we can test that it's working well in both setup: single project ond multiple ones.

For Svelte I added it as well, but I'm not 100% sure it works, could you try it? I'm not sure how it behaves if there are multiple "configurations" or multiple versions? I tried to add that to each documentation of LSP to warn users to not use multiple versions if it's not well supported.

[igorlfs]

For Svelte I added it as well, but I'm not 100% sure it works, could you try it?

It's working fine, from what I could test. In both mono and regular repos (haven't tested different versions, though). However, I couldn't find any mentions in the documentation stating that the server has full support for monorepos.

[fuegoio]

I agree that picking workplace root is a good call. I think we should still check if an eslintrc file or its variation is present before enabling the language server. The current implementation can potentially introduce an issue in a project where you use a particular linter/formatter. If in your config you have two linters/formatters enabled by locating a lock file, both of them both will get attached to a buffer and the user may see potentially conflicting diagnostics or formatting suggestions

To clarify we already do this for this language server in on_init, just highlighting that if we decide to do this for others we should still check for the presence of their config files

Thanks a lot for this comment, I totally agree that I forgot about this. To avoid this, I propose to go in the root_dir direction instead to determine both:

• the root dir of the project
• if the buffer needs this LSP to start or not

I uniformized this technique in those LSPs with success:

• ts_ls
• vtsls
• eslint
• tsgo
• biome

This is needed in fact for every LSP that interacts on files that are not specific to this LSP, like javascript or typescript files. For svelte, it is not needed as a svelte file will need the LSP to run, and there is no configuration to indicate this.

[justinmk]

Is the PR description still an accurate Problem/Solution explanation?

[fuegoio]

Is the PR description still an accurate Problem/Solution explanation?

Yes clearly! I just added a small part on the fact that we keep the configuration files as important root_markers, not for the root_dir purpose but for attaching or not the LSP.

In fact, from my first proposal, this is the only logic that changes, and it's clearly for the best as removing it was more a bug.

[fuegoio]

Thanks for merging @justinmk! I just realized that the new implementation for ESLint to find the configuration type was not great in fact, leading to a big perfomance issue. I fixed that by going back to globpath here: #4004