vscode-java icon indicating copy to clipboard operation
vscode-java copied to clipboard

Update language-client to 8.0.0-next.4

Open CsCherrYY opened this issue 3 years ago • 2 comments

This PR is a part of #2376, to reduce the complexity of that PR. Type Hierarchy support has been introduced since language-client 8.0.0-next.4 so we should update this dependency, the language-client version requires vscode 1.61.0 for compiling, so I update this dependency as well.

  • for notification part: see https://github.com/microsoft/vscode-languageserver-node#3170-next-protocol-800-next-json-rpc-800-next-client-and-800-next-server, now all sendNotification methods will return a promise.

This update is incompatible with current Theia. see: https://github.com/eclipse-theia/theia/issues/10993

CsCherrYY avatar Mar 24 '22 02:03 CsCherrYY

I ran into a problem updating to the language client to 8.0.1 in vscode-xml, and I suspect it'll be the same here. The extension will break on Theia.

Note: Node 14 needs to be used as with 12, I ran into SyntaxError: Unexpected Token '?'. (nullish coalescing issue). Afterwards I ran into :

2022-05-26T13:34:36.203Z root INFO [hosted-plugin: 9987] PLUGIN_HOST(9987): PluginManagerExtImpl/loadPlugin(/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension)
2022-05-26T13:34:36.257Z root ERROR [hosted-plugin: 9987] Activating extension 'XML' failed: TypeError: Class extends value undefined is not a constructor or null
    at Object.1365 (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:447499)
    at n (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:655557)
    at Object.71 (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:340748)
    at n (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:655557)
    at Object.4384 (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:307797)
    at n (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:655557)
    at Object.2850 (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:481672)
    at n (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:655557)
    at Object.4060 (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:251645)
    at n (/home/rgrunber/.theia/extensions/vscode-xml-0.21.0.vsix/extension/dist/extension.js:2:655557)

extension.js:2:447499 points to class ProtocolTypeHierarchyItem extends code.TypeHierarchyItem extension.js:2:251645 points to const vscode_languageclient_1 = __webpack_require__(48);

If I'm understanding how the __webpack_require__ works, const vscode_languageclient_1 is in 47, which gets called at the very beginning by const xmlExtensionApiImplementation_1 = __webpack_require__(47); . This is from xmlExtensionApiImplementation.ts which originates in the vscode-xml sources.

So the act of referencing the language client causes TypeHierarchyItem to be referenced, which Theia does not support

CC'ing @fbricon , @testforstephen in case I'm missing an obvious way to work around this.

rgrunber avatar May 26 '22 14:05 rgrunber

@rgrunber Yeah I had opened an issue here: https://github.com/eclipse-theia/theia/issues/10993, it seems Theia should support the related types (e.g., TypeHierarchyItem).

CsCherrYY avatar May 27 '22 01:05 CsCherrYY

We should re-investigate this soon. Once Theia releases, I'm hoping this will work. It seems they recently started supporting the Type Hierarchy API.

https://eclipse-theia.github.io/vscode-theia-comparator/status.html

rgrunber avatar Oct 25 '22 13:10 rgrunber

it seems that Theia has released a new version 1.31.0 which supports TypeHierarchyItem: https://github.com/eclipse-theia/theia/blob/master/CHANGELOG.md#breaking_changes_1.31.0

I will update the PR these days.

CsCherrYY avatar Oct 28 '22 02:10 CsCherrYY

I may have been a bit too optimistic. We will still need to wait for the end of the year to be able to make this change, but at least it will happen eventually :(

rgrunber avatar Oct 28 '22 14:10 rgrunber

@rgrunber is there anything I miss about this change?

One concern is that we seem not to have a way to define the required Theia version in package.json, since only the newest Theia supports this API.

CsCherrYY avatar Oct 31 '22 01:10 CsCherrYY

https://github.com/redhat-developer/vscode-java/pull/2377/commits/6842d5553e44ece50087a28baba5e7fc6eaf6974 updates the language client to newest 8.0.2. Comparing it with the former 8.0.0-next 4, the changes can be found also in https://github.com/microsoft/vscode-languageserver-node#3170-protocol-800-json-rpc-800-client-and-800-server:

  • removeal of onReady()
  • change of ErrorHandler

cc @rgrunber

CsCherrYY avatar Jan 30 '23 08:01 CsCherrYY

I tried this out, and I get an error notification when the syntax server is shut down. Is there a way to avoid this?

datho7561 avatar Feb 06 '23 21:02 datho7561

I tried this out, and I get an error notification when the syntax server is shut down. Is there a way to avoid this?

In the current version, when switching from syntax server to the standard server, we can also see these error messages in the syntax server output channel: image

And the behavior is introduced in https://github.com/microsoft/vscode-languageserver-node/issues/824, which forces the error message shown in the pop up. So I think the current way to solve this is to know why there is an error when syntax server stops.

CsCherrYY avatar Feb 07 '23 07:02 CsCherrYY

maybe https://github.com/eclipse/eclipse.jdt.ls/pull/1790 is related. @rgrunber could you provide any comments?

CsCherrYY avatar Feb 07 '23 08:02 CsCherrYY

@datho7561 It seems that if we disable the client capability shouldLanguageServerExitOnShutdown, the error notification will not appear and the syntax server can shutdown successfully. Could you help verify it?

image

@rgrunber If it's true, I guess language client 8+ has fixed what we patch in https://github.com/eclipse/eclipse.jdt.ls/pull/1790.

CsCherrYY avatar Feb 13 '23 03:02 CsCherrYY

... if we disable the client capability shouldLanguageServerExitOnShutdown, the error notification will not appear and the syntax server can shutdown successfully

Yes, i think this is the right fix. This is what we did in https://github.com/redhat-developer/vscode-xml/commit/825823c6e7199052d338166d91dca13d434bf9a6 for vscode-xml

datho7561 avatar Feb 13 '23 15:02 datho7561

BTW, dose the new version of language client has native support of inlay hint? If yes, maybe we can remove https://github.com/redhat-developer/vscode-java/blob/master/src/inlayHintsProvider.ts and its related code.

jdneo avatar Feb 16 '23 06:02 jdneo

@jdneo I guess the anwser is yes. See: https://github.com/microsoft/vscode-languageserver-node#3170-protocol-800-json-rpc-800-client-and-800-server inlay hint support is in the list here. I also find the related types in the language-client library.

CsCherrYY avatar Feb 17 '23 01:02 CsCherrYY

it looks like vscode-languageclient version 8.1.0 is available, I'll take a look if we can directly update to this version.

CsCherrYY avatar Feb 17 '23 01:02 CsCherrYY

The build still passes. Is this safe ?

@rgrunber Probably we should upgrade version of typescript. See https://github.com/microsoft/TypeScript/issues/1778#issuecomment-913202933

Eskibear avatar Feb 22 '23 02:02 Eskibear

Ok, then let's go ahead and update typescript here if that will fix it.

rgrunber avatar Feb 22 '23 13:02 rgrunber

@rgrunber it looks like that issue got resolved after updating typescript and node :)

CsCherrYY avatar Feb 23 '23 02:02 CsCherrYY