Add Separate Export for LSP Services

[montymxb]

Closes #1166 by providing a separate langium/lsp export for LSP-related services for Langium.

Note, this is a breaking change planned for introduction later in v3.0.0.

Changes (not exhaustive):

• Splits lsp related services into lsp-services & langium-lsp-module
• Furrther split some existing services into default & lsp variants (configuration provider, langium document factory, workspace manager, to name a few)
• Exposed related types like LangiumServicesWithLSP and LangiumSharedServicesWithLSP to make it pretty clear how to extend existing applications with the new type signatures
• Updates existing example languages to incorporate the new LSP module (in conjunction with the default one) to work correctly
• More or less leaves everything in 'grammar' that is tied to /lsp alone
• TextDocument(s) were pulled out into the LSP, but we may want to put this back, I'm open for thoughts here
• updates impacted tests
• inject can inject more

Open for refactors and changes based on feedback, and can go from there.

createServicesForGrammarWithLSP is marked with a TODO that is a effectively an RFC. Looking for feedback to see if I should generalize the approach a bit more to work with createServicesForGrammar, or leave it as is.

[spoenemann]

We need to eliminate the internal dependencies to the lsp subfolder. This is only possible by splitting default-module.ts.

[spoenemann]

Thanks for this first spike!

I'm thinking about a different approach to structure the services and modules:

• Introduce LangiumCoreServices and LangiumLspServices for the split, incl. respective shared services and default modules
• Keep LangiumServices and LangiumSharedServices intact, i.e. the union of core and LSP (implying that these need to be defined in the lsp subfolder)

Rationale:

• Reduce the impact of the change in terms of breaking existing code (just change the import)
• Splitting core and LSP services is not something that many users will do, but rather a special feature for those using Langium as a library in the browser
• We should keep things simple for the most common case, but make the more special cases possible

@msujew WDYT?

[montymxb]

@spoenemann I'm on board with the core vs. lsp services 👍 . I was also wondering if we wanted to do something with an explicit 'core' set of services, I like that.

In addition retaining the structure of LangiumServices and LangiumSharedServices as being lsp-complete makes sense, given existing consumers of these services will generally expect them to be complete. I can roll that change in later this week.

[montymxb]

Also renamed the DefaultServiceRegistry to CoreServiceRegistry, still not 100% sure on this given the 'core' registry is also used in the lsp + core case -- just with a different interface.

[montymxb]

Just a note, there's a single test failure after the last rebase on top of the new ./generate export work. I'm not able to pin it down at this time, but the CI is a bit misleading here. There appears to be a separate error leading the reported failure regarding vite being unable to resolve some functions (or typeinfo), for example:

TypeError: vite_ssr_import_0.createDefaultSharedModule is not a function

Several other 'langium' imports in the domainmodel example seem to have a similar issue, but strangely not others like requirements or statemachine, which look like they should trigger the same issue. The plan is to keep looking into this more next week to resolve.

[sailingKieler]

Very good progress @montymxb!
I have a few detail remarks/wishes and one major thing: The workspace initialization should not got into the LSP related part.

I think special attention need to be paid to the stuff in promise-util.ts that need to be separated, too, doesn't it?

[msujew]

@montymxb #1171 has been merged, feel free to rebase :)

[montymxb]

CI looks good sans lint. Local linting & gitpod linting looks good despite this, so this action triggered lint is still outstanding for now.

[msujew]

Great, you've made good progress on this. We needsome better abstraction layers for some of these services, but otherwise this looks quite good.

[montymxb]

Thanks for the feedback @msujew . I'm catching up on some things right now, and then should be coming back to reply & update in a day or two.

[spoenemann]

Seeing the amount of additional complexity that is necessary to separate LSP from Core, I am no longer convinced that the benefits of the separation justify its costs.

What's the use case for the separation? Using Langium in a browser app to parse and process text (without editor support), with a minimal bundle size. That's a rather specific case.

Can we offer an alternative solution for that use case? For example, a recommended bundling configuration where imports from vscode-languageserver are replaced by some mocks?

Let's discuss this in January when everyone's back from vacations.

Edit: I resolved my detail comments below – there's no need to address them before we clarify how to proceed in general.

[sailingKieler]

Excellent work @montymxb!!

I have some remarks to simplify some things, and I agree with Miro, that the LSP services should expect LangiumServices instead of LangiumCoreServices, but in general I'm very in favor of this contribution!

[montymxb]

Thanks for the feedback @sailingKieler . Added some notes back to what you've mentioned.

[sailingKieler]

Hey @msujew,

I added another commit contributing packages/langium/src/utils/cancellation.ts just containing
export * from 'vscode-jsonrpc/lib/common/cancellation.js'; and updating all related imports.

I added it to utils/index.ts as a named export similar to the AstUtils to not pollute Langium's namespace further.

Technically, the results will be of the same size as with copying the code.
How likely is it that vscode-jsonrpc will also get node16-exports in near future? (like vscode-languageserver-types has, for example). Should I add an export of vscode-jsonrpc/lib/common/event.js, too?
What do you think about that?

[sidharthv96]

@msujew yes, this is very crucial for us at Mermaid.

This is our current bundle breakdown.

And this is the breakdown of our parser package.

VSCode related packages make up around 1/3rd of the bundle.
With the ~175Kb reduction from #1171, and ~200Kb from this PR, it'll cut the size by more than 50%, which is huge for mermaid. 🚀

mermaid-js/mermaid#4734 (comment)

[sailingKieler]

I did another refinement, now there's no import ... from 'vscode-jsonrpc'; in the code base anymore, except for
export * from 'vscode-jsonrpc/lib/common/cancellation.js'; in utils/cancellation.ts, as mentioned above.

I also double-checked on exporting Event (see above), but that doesn't bring any value IMO, so I discarded that.

I have one more commit in the pipeline replacing all type imports of vscode-languageserver by vscode-languageserver-types or vscode-languageserver-protocol, but that would be too much for this PR I think.

[msujew]

@sidharthv96 Thanks for the info! Good to know that we're on the right track on this.

[msujew]

@sailingKieler

How likely is it that vscode-jsonrpc will also get node16-exports in near future? (like vscode-languageserver-types has, for example). Should I add an export of vscode-jsonrpc/lib/common/event.js, too?

Good question - no idea. I assume at the same time that vscode switches over to ESM, which might take a while. I think the new approach is just fine.

[Lotes]

Why was InitializableService reintroduced? Is it not better to register an event listener? Adding super types to make this happen is a solution. But I would prefer avoiding changes on the type hierarchy, since is already doable with listeners.

Or do I miss something?

[sailingKieler]

Why was InitializableService reintroduced? Is it not better to register an event listener? Adding super types to make this happen is a solution. But I would prefer avoiding changes on the type hierarchy, since is already doable with listeners.

Or do I miss something?

As you can see in https://github.com/eclipse-langium/langium/pull/1258/files#diff-9ef9cc03a159c7e21b4350faa99ddd28f97e4f27c934a51508e14531e4f4987bL49-L58 the DefaultWorkspaceManager (a core service) accessed the LanguageServer -- an LSP service --, which is to be avoided.
Same was the case for the DefaultConfigurationProvider. Moreover, the latter relied on API of vscode-languageserver that is also to be avoided.

[sidharthv96]

Are there any migration docs in the works, that I can refer to and try to make changes in our parser?
Or an example of how we can get from a string to AST without LSP. I think the current examples rely on LSP.

[sailingKieler]

Are there any migration docs in the works, that I can refer to and try to make changes in our parser? Or an example of how we can get from a string to AST without LSP. I think the current examples rely on LSP.

@sidharthv96 the changes of this will mainly require you to update/refactor imports within your custom code, and to re-generate the generated code with the langium cli.

In addition, to take advantage of this restructuring, you have to rework the preparations in your langium config module, precisely replace the calls of createDefault[Shared]Module by createDefault[Shared]CoreModule, and adapt the imports accordingly. That's basically it wrt. to this PR.

However, the changes of #1320 will most likely induce more manual migration effort.
For sure, there will be some migration guide.

[spoenemann]

This is now in a really good shape, good work! 💪

The poll results don't suggest that this effort is needed: at the time of writing, none of the 26 participants selected "Parsing only". However, we have statements from @sidharthv96 (#1258 (comment)) and @jindong-zhannng (#1346 (comment)) confirming that there are at least some applications where bundle size is crucial. I see this as a sufficient sign that we can proceed, provided that the changes are not too disruptive for other users.

I think we found good solutions to avoid raising the overall complexity for "normal" Langium users. We still need to write a migration guide, but in most cases it's just about updating imports (which is necessary for other v3 changes, too).

@sailingKieler please have a look at my questions and remarks below. In addition, I found imports from 'vscode-languageserver' in the following source files, which should be changed to type imports from 'vscode-languageserver-types':

• ast-utils.ts
• cst-utils.ts
• jsdoc.ts

[sailingKieler]

I now limited the re-export of vscode-languageserver-textdocument to TextDocument, as duplicate definitions are accessible via import ... from 'vscode-languageserver-protocol' for all other definitions exported by vscode-languageserver-textdocument.

[sailingKieler]

I now replaced the constants like DiagnosticSeverity.Information from vscode-languageserver-types by their values, and left an info on their original definition.

[msujew]

Not a big fan of this - users that don't want to include vscode-languageserver-types in their build, are now only allowed to use the numbers themselves (or have to redeclare the enum themselves). We should offer the enum ourselves.

[sailingKieler]

Actually this peace of code is only used to convert the Langium notion of severity being defined as 'error' | 'warning' | 'info' | 'hint' into the LSP notion while composing LSP diagnostics, right?
Our code is just converting the values according to the protocol definition, and therefore I don't see why Langium should export copies of those definitions.
Do you have concrete use cases in mind? Why should users care about those constants?

[msujew]

Ah, right I forgot. Alright, that's fine then 👍

[spoenemann]

Let's ship it! 🚀

[spoenemann]

Looking at this again, I'd rather remove this interface: we already have onInitialize and onInitialized events in the LanguageServer service, which are the recommended ways to hook into the LSP lifecycle. The interface is only used by two core framework classes which require to invert the dependency. The interface is not necessary because LanguageServer hard-codes the calls to these classes anyway.

[Yokozuna59]

Hi there, thanks for your work!

I'm currently facing an issue regarding the ServiceRegistry#getServices, see the comment below.

[Yokozuna59]

How can I access the LSP services if this method only returns LangiumCoreServices?

I'm currently trying to run two language servers in the same document, one of which is the Langium language server and the other not.
The Langium language server is the default one, and if the user typed some specific text, it would use the other language server suggestions.

Anyway, the approach I'm taking is to create two modes; according to mode, the language server would use either the language server or the other language server methods to suggest, but now I can't use the language server since I can't directly access the LSP services.

Here is an example:

export class LangiumMode implements LanguageMode {
  private getServices({ uri }: TextDocument): LangiumServices /* error in 3.0.0 */ { 
    return this.shared.ServiceRegistry.getServices(URI.parse(uri));
  }

  public doComplete(
    document: TextDocument,
    params: CompletionParams,
    cancelToken?: CancellationToken | undefined,
  ): MaybePromise<CompletionList | undefined> {
    return this.getServices(document).lsp.CompletionProvider?.getCompletion(
      this.textDocumentToLangiumDocument(document),
      params,
      cancelToken,
    );
  }
  
  // ...
}

[cdietrich]

for me it worked to downcast

[sailingKieler]

@Yokozuna59 A quick answer: Look #1166 (comment) and below.

[Yokozuna59]

I see, thanks @cdietrich @sailingKieler.