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

📎 LSP: support for Workspaces #1573

Closed
ematipico opened this issue Jan 16, 2024 · 20 comments · Fixed by #2607
Closed

📎 LSP: support for Workspaces #1573

ematipico opened this issue Jan 16, 2024 · 20 comments · Fixed by #2607
Labels
A-LSP Area: language server protocol Fund S-Enhancement Status: Improve an existing feature S-Help-wanted Status: you're familiar with the code base and want to help the project

Comments

@ematipico
Copy link
Member

ematipico commented Jan 16, 2024

Description

At the moment, our LSP doesn't support workspaces.

To achieve that, the LSP needs to add a few features that it doesn't support at the moment:

  • register itself for other events:
  • keep track of multiple configurations, one for each workspace folder, and apply this configuration based on the file the user opens. This could be challenging because our Workspace isn't designed to hold multiple settings, so the solution might lie in the LSP itself and having a data structure that saves this data and updates the Workspace settings before handling a file. This should be enough, but we need to verify the performance implication of swapping settings every time we change a file.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@ematipico ematipico added S-Help-wanted Status: you're familiar with the code base and want to help the project A-LSP Area: language server protocol S-Enhancement Status: Improve an existing feature labels Jan 16, 2024
@unional
Copy link

unional commented Feb 20, 2024

Adding my two cents here.

The projects within a workspace (vscode workspace) can be owned by different teams and they have different configuration.
Also the file/folder structure can be very different (thus the files.include and files.ignore).

So switching setting on the fly before handing a file may not be the best approach.

It might be better if there can be one instance of LSP per project and the vscode extension talks to these instances.

@ematipico
Copy link
Member Author

You're suggesting a solution at the client level (VSCode), not the server level (LSP). I don't think that's what we want to do, because other clients (neovim, sublime, etc.) would not benefit from it, and it's not fair to favour only VSCode.

So switching setting on the fly before handing a file may not be the best approach.

I'm not sure. The configurations can be computed when workspace/workspaceFolders is called; then, we just need to swap the configuration when the workspace folder changes; the LSP will signal this with workspace/didChangeWorkspaceFolders.

@unional
Copy link

unional commented Feb 22, 2024

Hi, I'm not sure how other client work so don't know if that is correct, but does other clients has the same workspace concept as in VSCode? Bigger IDE such as Visual Studio or IntelliJ has similar concepts, don't know if neovim, sublime, etc. has similar things or not.

I don't think it is about "fairness". It's about complexity and separation of concerns. The workspace in VSCode can be different in Visual Studio IntelliJ, neovim, etc. So adding the support on the server side might make it more complicate than necessary.

Another consideration is performance and distribution. If the workspace consist of projects/repos with very different sizes, some extra large and some are tiny, is one LSP instance the best approach or it is better to separate them so that one will not brought down the others? How about during file rename or bulk change? On vscode, doing a find and replace on hundreds of files brought it to a halt by the Prettier extension. Biome extension also have that problem but it is a bit better.

Just want to bring these points up so that you can drive the design towards a good direction.

@NeoN0x
Copy link

NeoN0x commented Mar 4, 2024

Hi all,

Adding my simple use case here (I do not understand deeply LSP, sorry if I does not bring something useful, I'm a simple user)

I believe I'm facing a related issue here.

I got a team workspace like the following

a-project
- eslint.json
b-project
- biome.json

Currently in the b-project, everything is fine related to biome using CLI. However, the configuration is not applied on the b-project with the vscode extension, and it is not usable at all. After some tests it appears that the "workspace root folder" here is the a-project (alphabetically ordered ?).

As I workaround, I copy/paste biome.json in the a-project, and then the vscode extension is happy and apply my rules (having to pay attention to not commit this file)

In order to have a better alternative, would it be a good idea to define the source biome.json path in the extension settings ? (could also be a profile/workspace setting making it usable in different workspace as well, defaulted to the root one so behave normally if not set).

PS: eslint does not have this strange behaviour

Thanks

@Sec-ant
Copy link
Member

Sec-ant commented Apr 2, 2024

@unional

but does other clients has the same workspace concept as in VSCode? Bigger IDE such as Visual Studio or IntelliJ has similar concepts, don't know if neovim, sublime, etc. has similar things or not.

Yes the workspace is a concept of the Language Server Protocol (LSP), so it will be supported in any editor that follows and implements the spec. It's not a VSCode-specific thing.

Another consideration is performance and distribution. If the workspace consist of projects/repos with very different sizes, some extra large and some are tiny, is one LSP instance the best approach or it is better to separate them so that one will not brought down the others?

From the VSCode Wiki, single language server is the recommended way to implement the multi-root workspaces support:

Single language server or server per folder?

The first decision the author of a language server must make is whether a single server can handle all the folders in a multi-root folder setup, or whether you want to run a server per root folder. Using a single server is the recommended way. Sharing a single server is more resource-friendly than running multiple servers.

But I think what you said in this topic is not about multi-root workspaces, but multiple projects (or package manager workspaces) in a single-root LSP workspace. That should fall into the category of monorepo support which can be discussed in #2228.

is one LSP instance the best approach or it is better to separate them so that one will not brought down the others?

I think one LSP server instance doesn't necessarily mean one will bring down the others. We can preserve an internal structure where one LSP server has more than one LSP workspace instances, and each workspace instance can also have more than one projects. And on each editor fileOpen/fileChange event, we will decide the project or workspace the file belongs, and apply the settings accordingly. In this way, biome configurations can also be set in a project-level. With a proper cache strategy, this shouldn't be a problem.

The reason why Biome needs to understand things in a workspace-level instead of just a project-level is answered in #2228 (comment). Actually, we already have module resolution involved in the extends field of the configuration, and it wouldn't be possible to extend the configuration from another package in the same workspace in a monorepo setup if Biome cannot do workspace-level analysis.

Just to be clear, the "project" I mentioned above is a one-to-one correspondence with the "package.json" files. And I think its a reasonable minmal granularity for the Biome configuration.

@unional
Copy link

unional commented Apr 2, 2024

Thanks for the detailed explanations. 🍻

But I think what you said in this topic is not about multi-root workspaces, but multiple projects (or package manager workspaces) in a single-root LSP workspace. That should fall into the category of monorepo support which can be discussed in #2228.

I'm talking about multi-root workspaces, not monorepo. i.e. in VS Code, it's the folders in the xxx.code-workspace file.

I concur that one LSP server with multiple LSP workspace instances is the way to go. Each root/workspace should have their own biome.json and each LSP workspace instance should be isolated from each other.

@wojtekmaj
Copy link

I'm adding a $50 bounty to this ticket. I absolutely adore Biome, but this thing is affecting my workflow very much and it's the only thing preventing me from using Biome in many, many repositories.

Bounty will be granted, via GitHub Sponsors, to whoever makes Biome's linting and formatting experience in VSCode on par with ESLint/Prettier, i.e. fully respecting biome.json files at the roots of multi root workspaces, as well lower down the tree.

@ematipico
Copy link
Member Author

Thank you, @wojtekmaj. Would you consider funding the issue via Polar instead? We added a badge to the issue now.

@matamatanot
Copy link
Contributor

I accessed here from a link in the official documentation. Is the support already finished? If so, I want to update the documentation. If it has not been addressed, it may be a good to reopen this issue or make the link to another issue.

@ematipico
Copy link
Member Author

The PR that implemented the feature is in main, the next release will have this feature. We will update the documentation once we are close to the release.

@ematipico
Copy link
Member Author

You can test LSP workspaces using this nightly: https://github.com/biomejs/biome/releases/tag/cli%2Fv1.7.4-nightly.125f34b

@jpenna
Copy link

jpenna commented Jun 10, 2024

I installed the pre-release version of the plugin (v2024.5.251958 (pre-release)), but it isn't using the inner folder's biome.json file yet. Was this nightly version supposed to fix it?
My JSON file is in frontend/biome.json.

@kris-ellery
Copy link

kris-ellery commented Jun 10, 2024

Hi there! Is there a working dummy monorepo with VSCode workspaces and Biome integration? I have upgraded to v1.8.1 and haven't had any success with formatting on file save via VSCode.

I have a single biome.json at the root of a monorepo, and the check/lint/format commands work great via CLI. Format on file save via VSCode w/out workspaces works as expected. However, as soon as I open the workspaces, the format on save stops working.

Any help would be greatly appreciated!

My .vscode/settings.json config:

{
  "editor.defaultFormatter": "biomejs.biome",
  "editor.formatOnSave": true,
  "editor.codeActionsOnSave": {
    "source.organizeImports.biome": "explicit",
    "source.fixAll.biome": "explicit",
    "quickfix.biome": "explicit"
  }
}

My biome.json config:

{
  "$schema": "https://biomejs.dev/schemas/1.8.1/schema.json",
  "organizeImports": {
    "enabled": true
  },
  "vcs": {
    "clientKind": "git",
    "defaultBranch": "main",
    "enabled": true,
    "useIgnoreFile": true
  },
  "linter": {
    "enabled": true,
    "rules": {
      "recommended": true,
      // ...
    }
  },
  "formatter": {
    "enabled": true,
    // ...
  },
  "javascript": {
    "formatter": {
      // ...
    }
  }
}

@ematipico
Copy link
Member Author

Workspaces need to be supported by the clients too.

There's an issue you can track for VSCode: biomejs/biome-vscode#201

@kris-ellery
Copy link

@ematipico thanks for the update! I truly appreciate all the work you and your team are putting into Biome. I donated to the project and to this issue. Hope it helps!

@reckek
Copy link

reckek commented Jul 1, 2024

Is development paused now? I see that there have been no new commits in the release branch (biomejs/biome-vscode#201) for a month.

@AdaptCharm
Copy link

Any update on the progress? @ematipico

@ematipico
Copy link
Member Author

Any update on the progress? @ematipico

This was shipped months ago. Now any LSP client that supports LSP workspaces will work out of the box.

@stanlrt
Copy link

stanlrt commented Sep 18, 2024

@ematipico
Copy link
Member Author

It will be biomejs/website#974

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LSP Area: language server protocol Fund S-Enhancement Status: Improve an existing feature S-Help-wanted Status: you're familiar with the code base and want to help the project
Projects
None yet
Development

Successfully merging a pull request may close this issue.