Support multiple handlers of JSON-RPC Notifications

[mhintzke]

I am trying to build an application that utilizes monaco editor to support writing PowerShell in the browser. We have a requirement to disallow saving of a file when there are script diagnostic errors present. This means I need my application to know when the Diagnostics change which sent me down a path to use the IConnection.onDiagnostics(...) method to handle when the diagnostics are updated.

However, when I did this in my application code, all of a sudden the monaco editor stopped displaying the errors and warnings in the editor itself. So I dove into the code and found this:

vscode-languageserver-node/jsonrpc/src/main.ts

Line 1004 in 9e12146

notificationHandlers[type.method] = { type, handler };

This line of code is overwriting the handler when a new handler is set, effectively only allowing a single handler to be used for a given notification type. So clearly, when I try to handle diagnostic updates in my application code I am removing the handler that is set here:

vscode-languageserver-node/client/src/client.ts

Line 2906 in 153fa63

connection.onDiagnostics(params => this.handleDiagnostics(params));

Is there something in the JSON-RPC spec that disallows multiple handlers to be used? This seems like something very simple to introduce if it is allowed by the spec. Is there some other way I could be handling this that is already supported? I guess I'm surprised that no one else has needed this kind of functionality before.

Without this change, I will have to introduce much more code to "poll" for the diagnostics during each Angular change detection cycle which is definitely less elegant or performant.

[mhintzke]

After trying to figure out how to do this in an alternative fashion, I realized that in a newer version of the vscode-languageclient there is a middleware options you can add to the LanguageServiceOptions which allows you to hook into the handler of Diagnostics:

#322

The above PR outlines the exact issue I am facing. I will attempt to update to a newer version which should allow me to get this functionality. Previously we were unable to upgarde the package due to other depenedencies have conflicts, but looks like we will have to figure it out.

I would still like to address the larger concern of Notification in general not allowing multiple handlers and would like to understand why we cannot support that natively with JSON RPC

[rcjsuen]

This request sounds similar to #174.

[mhintzke]

Thanks, this is similar to the issue, but not identical. I did comment over there to try to resurrect the issue as its been over 2 years and no signs of fixes have been found.

I'm not against fixing it myself, I just wanted to know if there was some good reason for not supporting this behavior.

[dbaeumer]

Closing in favour of #174