TypeFox / monaco-languageclient

Repo hosts npm packages for monaco-languageclient, vscode-ws-jsonrpc, monaco-editor-wrapper, @typefox/monaco-editor-react and monaco-languageclient-examples
https://www.npmjs.com/package/monaco-languageclient
MIT License
1.04k stars 178 forks source link

Is monaco-languageclient still needed in the future? #543

Closed kaisalmen closed 5 months ago

kaisalmen commented 11 months ago

This is not a question about if we want to continue to maintain it or not? The question is whether monaco-languageclient is still required as library on its own!?

In 2023 the actual code of the lib was reduced to a simple BasicLanguageClient extension and most of the current lib code is about how to configure monaco-vscode-api services. With recent discussion in https://github.com/CodinGame/monaco-vscode-api/pull/191 it could even make sense to move away from the current vscode services config approach as it draws in many dependencies (even if conditional) actually not needed by the end-users. This would reduce the code base even further. This and other things (e.g. how is the lib used) made me wonder if the library is still needed on its own or if it should be integrated elsewhere?

One potential idea is to fuse the code of this lib with monaco-editor-wrapper as the wrapper already makes usage/integration of monaco-vscode-api + monaco-editor + monaco-languageclient simpler.

Another idea is to move this code to monaco-vsode-api and create a sub-package for the client.

In any case examples should stay here.

What are your thoughts?

CGNonofr commented 11 months ago

The vscode-ws-jsonrpc is still required for the integration of monaco in the browser with a remote language server. it needs to be somewhere and it's probably too specific to be integrated in monaco-vscode-api

I see monaco-vscode-api as a very complex toolbox. One usage of it it to integrate a languageclient with monaco-editor. A lot of users (most of them?) just want that feature and asking them to understand everything is probably too much.

I really do think having this repo to guide/help users and make sure this use-case works great is a good thing.

Not mentioning that an user wanting to integrate a language client won't probably easily find monaco-vscode-api on google

kaisalmen commented 11 months ago

The vscode-ws-jsonrpc is still required for the integration of monaco in the browser with a remote language server.

Yes, vscode-ws-jsonrpc should stay as is. This is just about monaco-languageclient. What I realize the last weeks/month is that actually monaco-languageclient in itself doesn't serve a purpose any longer. Actually the service init is a util that is actually not required to be in the lib and then only one little BaseLanguageClient extension remains.

I see monaco-vscode-api as a very complex toolbox. One usage of it it to integrate a languageclient with monaco-editor. A lot of users (most of them?) just want that feature and asking them to understand everything is probably too much.

I really do think having this repo to guide/help users and make sure this use-case works great is a good thing.

I agree with both statements.

Not mentioning that an user wanting to integrate a language client won't probably easily find monaco-vscode-api on google

Likely true.

Extra thoughts: It could make sense to move monaco-editor-wrapper (depends on momnaco-languageclient and vscode-ws-jsonrpc) into this repo as it makes setting up monaco-editor + languageclient and talking to a language server (via web worker or websocket) quite simple, IMO. We currently don't use the languageclient on its own. It is always via the wrapper, because less code/unified configuration. Then the lib could stay/evolve or be fused together. My overall aim is less maintenance work (one set of examples, mono-repo) + benefit for the end-users.

RSS1102 commented 10 months ago

I haven't learned about monaco vscode apis, but if I only want the editor to support the functionality of a certain language (language/worker), is usingmonaco vscode apis too complicated?

dkattan commented 10 months ago

It could make sense to move monaco-editor-wrapper (depends on momnaco-languageclient and vscode-ws-jsonrpc) into this repo as it makes setting up monaco-editor + languageclient and talking to a language server (via web worker or websocket) quite simple, IMO. We currently don't use the languageclient on its own. It is always via the wrapper, because less code/unified configuration. Then the lib could stay/evolve or be fused together. My overall aim is less maintenance work (one set of examples, mono-repo) + benefit for the end-users.

I really like this idea.

I haven't learned about monaco vscode apis, but if I only want the editor to support the functionality of a certain language (language/worker), is usingmonaco vscode apis too complicated?

I think as long as we had an sample that does this (which I'm pretty sure we do in the monaco-editor-wrapper project) then this would be a non-issue.

IMO anyone who is using monaco-languageclient will eventually want other features from VS Code, it is very much a "If you give a mouse a cookie" situation. Showing the migration path from just getting it wired up to the language server, then to a debug server, then loading popular extensions would be extremely helpful.

ls-infra commented 8 months ago

Hello, @kaisalmen , @CGNonofr . Thanks for the excellent monaco-languageclient. It is really helpful.

I see an opportunity to make the code more configurable to use different LSP servers more efficiently.

In https://github.com/TypeFox/monaco-languageclient/blob/main/packages/examples/src/common/server-commons.ts#L24,

I need to change the parameters passed into the createServerProcess to make it work with the https://github.com/GroovyLanguageServer/groovy-language-server . It will be great if we can reuse this code as part of the core instead of needing to copy-paste the /examples folder code in the users' repo and modify it.

Maybe we can move files under the /examples folder to a package like language-server-runner. Then, other developer users can just install and use this package via passing config object like

export interface LanguageServerRunConfig {

 serverName: string;

 serverPath: string;

 runCommand: string; // 'node', 'java' , etc. 

 runCommandArgs: string[];

 spawnOptions?: cp.SpawnOptions;

}

Maybe using https://nx.dev/ will be an excellent option to make a mono repo for https://github.com/TypeFox/monaco-languageclient so that we do not need lots of separated repos like https://github.com/TypeFox/monaco-languageclient-ng-example for demo or related feature. In general, Monorepo makes dependency management easier. (FYI: https://monorepo.tools/)

Using nx, you can put the demos into the /apps folder, and the reusable lower-level core code will be in the /libs folder.

For example:

/apps/angular-example calls the /libs/xxx-language-server

/apps/vue-example calls the /libs/xxx-language-server

/apps/react-example calls the /libs/xxx-language-server

More importantly , I see some dependencies between https://github.com/CodinGame and https://github.com/TypeFox, which depend on each other. Maybe it is a good idea to create a new Github Organization like “LanguageServerInfrastructure” by consensus from both sides.

Then, both TypeFox and CodinGame repos will depend on repos inside the new Organization, for example, the shared core LanguageServerInfrastructure/monaco-vscode-api instead of depending on each other. This structure may benefit collaboration and maintenance as open source projects for the community.

I hope it helps.

Thank you very much,

kaisalmen commented 7 months ago

Hi @adrian2024lsp please find some answers below:

I need to change the parameters passed into the createServerProcess to make it work with the https://github.com/GroovyLanguageServer/groovy-language-server . It will be great if we can reuse this code as part of the core instead of needing to copy-paste the /examples folder code in the users' repo and modify it.

The examples are available as separate npm package: https://www.npmjs.com/package/monaco-languageclient You can use the code without copying it around. If you like to improve the code, please feel welcome to open up an enhancement PR.

Regarding https://github.com/TypeFox/monaco-languageclient-ng-example , it was separated on purpose in the past, because it polluted this npm workspace with many unwanted dependencies and angular does not integrate well into the build workflow. Btw, it uses the examples npm package mentioned above.

Maybe using https://nx.dev/ will be an excellent option to make a mono repo

This is already an npm workspace. So far this is sufficient and gives enough flexibility. nx could be an option for something new and bigger. Local code and lib dependencies are handle nicely by npm and typescript config. Just fire up npm run watch when in need to change the client lib (see getting-started).

More importantly , I see some dependencies between https://github.com/CodinGame and https://github.com/TypeFox, which depend on each other. Maybe it is a good idea to create a new Github Organization like “LanguageServerInfrastructure” by consensus from both sides.

It is good the way it is. Even with a shared org there will be interdependent npm packages. @CGNonofr intentionally started a new repo to move the vscode api out of this repo and evolve it into those awesome package(s). We agreed on that together. Is see no need to change this. 👍

kaisalmen commented 7 months ago

@CGNonofr Some thoughts that should help to move this issue forward.

Instead of moving https://github.com/TypeFox/monaco-languageclient/blob/main/packages/examples/src/common/language-client-runner.ts we should bring monaco-editor-wrapper in here. I mentioned this idea already. It has all the things we need already and more. Developing the packages separately made sense, but nowadays the separation creates maintenance effort. Especially, the examples have a huge overlap and could be merged.

What we miss is small language server bootstrap library. language-server-runner could evolve into something like that.

monaco-languageclient and monaco-editor-wrapper should be kept separate. I came to the conclusion that fusing them makes no sense. But, having them here in one repo will be helpful. The seperate npm package/wrapper extension @typefox/monaco-editor-react should be moved along.

The angular example should stay separate or stay disconnected from the workspace as angular brings in so many dependencies. It could become one further verification example.

ls-infra commented 7 months ago

@kaisalmen yes , i agree , we should leverage the npm workspace to put related packages into one monorepo if possible, only making new separated repo as the last resort for special case like the angular one .

btw, i want to know a bit more how angular pollutes the npm workspace. are angular's specific dependencies and devDependencies not contained within the packages/angular/package.json only ?

kaisalmen commented 7 months ago

btw, i want to know a bit more how angular pollutes the npm workspace. are angular's specific dependencies and devDependencies not contained within the packages/angular/package.json only ?

@ls-infra The packages themselves are not polluted as the dependency definitions in the respective package.json are self-standing, but the main node_modules folder is huge. Using pnpm could also be considered, because it manages large number of dependencies more effectively.