Use monaco vscode api

[CGNonofr]

I'm open to any feedback!

It relies on https://github.com/CodinGame/monaco-vscode-api

[CGNonofr]

@kaisalmen I think it's ready!

[kaisalmen]

@CGNonofr wow, that is a huge change. I just briefly looked over the external repo and the changes here and see if I can build everything. That works. 👍 I need to look at it in more detail and test it myself (examples)

So, you basically build a vscode extension (external repo) that exposes the vscode api and services, but binds it to existing monaco-editor functionality or let's users implement missing things if required? Some additional words could be helpful. 😊

Do you want to move the content external repo in here as a sub-package once we think it is ready?

[CGNonofr]

So, you basically build a vscode extension (external repo) that exposes the vscode api and services, but binds it to existing monaco-editor functionality or let's users implement missing things if required? Some additional words could be helpful. blush

It's not a vscode extension! It's an implementation of the vscode API.
The VSCode api is composed of:

• A lot of classes and tools, which are exported the same way as in vscode
• Some features that are supported by monaco (language feature registrations) which are just forwarded to it (with some transformations)
• Some features that are not supported by monaco, and in such case:
  • If it's needed or can be useful for the LanguageClient: we let the user implement it as they wish
  • If it's some advanced features (debug, tests...) it just throws an error when you try to use it.

It's pretty similar to what the vscode-compatibility was doing here, the main differences are that it's not lying on typescript types and it doesn't use any hack. It uses the actual vscode code, import everything needed for it and then treeshake out what is not needed. It also tries to reuse the code already present in monaco.

Do you want to move the content external repo in here as a sub-package once we think it is ready?

I don't think it would make a lot of sense to put everything in a big monorepo, the other project can be used for something else that this library (you can run vscode extension code on monaco with it)

Btw, I'll probably improve the documentation

[kaisalmen]

Thanks for the clarifications. I misunderstood some things and did not look closely enough at the new repo yesterday.

It's pretty similar to what the vscode-compatibility was doing here, the main differences are that it's not lying on typescript types and it doesn't use any hack. It uses the actual vscode code, import everything needed for it and then treeshake out what is not needed. It also tries to reuse the code already present in monaco.

I think the overall approach is better. 👍 It will also ease maintenance of the lib and make integration easier for users, I think.

I don't think it would make a lot of sense to put everything in a big monorepo

Somewhat true, but it also remove 90% of the code from this repo. Having one repo doesn't hinder us from releasing different npm packages at different speeds and it doesn't prevent anyone from using a specific npm package.

the other project can be used for something else that this library (you can run vscode extension code on monaco with it)

Do you already have a proof of concept for that? Do you plan on using this at Codingame?

[CGNonofr]

It will also ease maintenance

True, there is still some potential issues (see the repo issues) I need to address. Also we need some CI on it (I can't just put semantic-release as we'll probably want the lib version to match the vscode api version)

True, but it also remove 90% of the code from this repo.

True, is it an issue? this repo can focus on the language client tweaks, examples and documentations. Also, it will stay the "visible" part for any user, the other lib is just a dependency.

Do you already have a proof of concept for that? Do you plan on using this at Codingame?

We already use almost all declarative parts of vscode extensions at CG in monaco-editor-wrapper (everything in the package.json: grammars, languages, configurations, snippets, themes, semantic tokens...). For some of them, some code is required to make the language client work at 100% (for instance, the jdt.ls LSP doesn't use the protocol for inlayHint and requires some client code, so we don't have the feature right now)

[kaisalmen]

I would like to merge all other PRs and make a 1.1.0 release and then do this one with a next release (even a 2.0.0)?

[CGNonofr]

I would like to merge all other PRs and make a 1.1.0 release and then do this one with a next release (even a 2.0.0)?

Sure! It's a big change, let's release a version with the small fixes first.

I'll probably try to resolve the few issues on monaco-vscode-api in the next days.

This PR was mainly to gather some feedback but it's not urgent to merge it, we better well test it first. We can probably release a *-dev version though

[kaisalmen]

I looked at the new lib a bit more closely to understand some of the magic (e.g. rollup part).

So, you did not really extract the vscode-compatibility, but you created something new that from the perspective of monaco-languageclient behaves similar. That wasn't clear to me at first.

What @codingame/monaco-vscode-api does is not only to expose the vscode API and redirects calls to Monaco editor, but it really embeds the required parts of vscode into the library.

For testing purposes I created a new esm bundle of monaco-languageclient locally with vite and this works fine (it mostly integrates the monaco imports of the new libs and the vscode-languageclient deps).

The node client examples works fine, btw.

But, shouldn't we keep the MonacoToProtocolConverter and ProtocolToMonacoConverter here? With your approach this is no longer needed, but the two existing converters open the possibility to directly connect monaco to the language server protocol. As we have seen yesterday there are people using it (issue 375) and you could build solutions not requiring vscode.

[CGNonofr]

So, you did not really extract the vscode-compatibility, but you created something new that from the perspective of monaco-languageclient behaves similar.

I'm not sure to understand what you mean, it doesn't behave similar to vscode-compatibility, as vscode-compatibility expects language-server-protocol objects (due to the bypassConvertion in the languageClient)

What @codingame/monaco-vscode-api does is not only to expose the vscode API and redirects calls to Monaco editor, but it really embeds the required parts of vscode into the library.

That's right, it exports all requires classes and tools as well

But, shouldn't we keep the MonacoToProtocolConverter and ProtocolToMonacoConverter here? With your approach this is no longer needed, but the two existing converters open the possibility to directly connect monaco to the language server protocol. As we have seen yesterday there are people using it (issue 375) and you could build solutions not requiring vscode.

I'm afraid that if the code is no longer used by the library, it won't be well maintained. But why not! what about making another library out of it though?

[kaisalmen]

I'm not sure to understand what you mean, it doesn't behave similar to vscode-compatibility, as vscode-compatibility expects language-server-protocol objects (due to the bypassConvertion in the languageClient)

Sorry, that was confusing. monaco-languageclient is now basically a vscode languageclient, because you wrap the complete treeshaked vscode API in the new lib and have monaco-editor directly glued into it.

[CGNonofr]

Ok, I updated to the just released vscode 1.68 and I think I did solve the issues I had (still not perfect but I don't have any idea on how to improve it)

Are you ok to publish a 2.0.0-dev version?

[kaisalmen]

@CGNonofr Are you ok to publish a 2.0.0-dev version?

Will do. Btw, you can do it yourself if you like. You should have all the permissions.

[kaisalmen]

Here you go:
https://www.npmjs.com/package/monaco-languageclient/v/2.0.0-dev.0

I pushed a small update on this branch (version changes)

[CGNonofr]

Btw, we're now able to release an esm version of this lib, should we do it in another PR?

[kaisalmen]

Btw, we're now able to release an esm version of this lib

Yes, this is really great. 👍

should we do it in another PR?

Let's do it directly here. I can quickly release a new dev version when you need one. You can directly increase the version number, btw.

Can you revert the deletion of monaco-converter.ts in this branch (Otherwise we invalidate fix #375)? Questions is if we should keep the browser example to use it.

One more thing: Could you add a note to README of https://github.com/CodinGame/monaco-vscode-api that this repo exists to some part exists because of the lessons you learned here. I know you basically rewrote it/took a different approach, but a link/hint would be fair, I think.

[CGNonofr]

Let's do it directly here. I can quickly release a new dev version when you need one. You can directly increase the version number, btw.

I'll do it! I have to fix another small issue as well with double re-export

Can you revert the deletion of monaco-converter.ts in this branch (Otherwise we invalidate fix #375)? Questions is if we should keep the browser example to use it.

I can! But I think that's a shame to have to maintain it while there is another way that doesn't require any code from this library. Will still do it, we will decide later I guess.

One more thing: Could you add a note to README of https://github.com/CodinGame/monaco-vscode-api that this repo exists to some part exists because of the lessons you learned here. I know you basically rewrote it/took a different approach, but a link/hint would be fair, I think.

You're completely right, I'll try something! I'll give you the permissions on the repo as well if you want?

[kaisalmen]

I can! But I think that's a shame to have to maintain it while there is another way that doesn't require any code from this library. Will still do it, we will decide later I guess.

I am not sure about this currently. We could have a two step approach: Keep it mid-term (mark deprecate) and then remove it. Then we should have two browser examples (unchanged old one and the adjusted one from this PR) for people to know how to adapt (just a quick idea)

I'll give you the permissions on the repo as well if you want?

That's not required right now. I could become a regular contributor at some point 😎

[CGNonofr]

@kaisalmen I've rewritten the history with all the requested changes, can you please check & release the version? :)

[kaisalmen]

Done! I pushed an update and also fixed some exports because vite complained CloseAction and ErrorAction aren't properly exported. Must this export:

export * from 'vscode-languageclient/lib/common/client';

be moved from monaco-language-client.ts to index.ts

[CGNonofr]

Did you push your changes?

[kaisalmen]

Now, sorry forgot, but the release is available

[CGNonofr]

A new block was added in the https://github.com/CodinGame/monaco-vscode-api README, is it ok for you?

[kaisalmen]

A new block was added in the https://github.com/CodinGame/monaco-vscode-api README, is it ok for you?

Very good, thank you 👍

[kaisalmen]

@CGNonofr we should merge this to the main, I think. This makes handling other PRs easier as well.

We can put out further 2.0.0 dev or beta releases, so people could play around before making the final 2.0.0 release.

[kaisalmen]

LGTM