Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Old vscode-languageserver-protocol version #11628

Closed
perrinjerome opened this issue Sep 2, 2022 · 2 comments
Closed

Old vscode-languageserver-protocol version #11628

perrinjerome opened this issue Sep 2, 2022 · 2 comments

Comments

@perrinjerome
Copy link
Contributor

Bug Description:

Since #8868 ( two years ago ) vscode-languageserver-protocol is pinned to an old version.

"vscode-languageserver-protocol": "~3.15.3",

I tried to update the version to the current version in perrinjerome@fb602c0 and the test seems to pass, except the yarn license:check test:

ERROR: Found results that aren't part of the baseline!
X npm/npmjs/-/lru-cache/7.14.0, ISC
X npm/npmjs/-/marked/4.1.0, MIT
X npm/npmjs/-/vscode-jsonrpc/8.0.2, MIT
X npm/npmjs/@sinonjs/text-encoding/0.7.2, Apache-2.0 OR Unlicense OR (Apache-2.0 AND Unlicense)

( from https://github.com/perrinjerome/theia/runs/8148442408?check_suite_focus=true#step:5:174 )
which might be expected at this point because in that commit I regenerated the yarn.lock by removing it and re-running yarn install.

@paul-marechal @vince-fugnitto do you think there is still a reason to keep this an old version these days ?

For the background, I'm interested in the fix from microsoft/vscode-languageserver-node#787 but generally speaking I guess it's better to update if the version is compatible.

If it helps, I can start a pull request with perrinjerome@fb602c0 and we'll discuss the problem with the licence checks there.

Steps to Reproduce:

  1. install theia
  2. it will use an old version of vscode-languageserver-protocol because of the version pin

Additional Information

  • Operating System: Chrome OS
  • Theia Version: current master ( 9e5db77 )
@paul-marechal
Copy link
Member

Correct me if I'm wrong, but I don't think we really rely on vscode-languageserver-protocol anymore, nor vscode-languageserver-node?

Theia doesn't directly talk LSP anymore, instead each plugin you have installed for language support should embed its own version of those packages which do the conversion between LSP and the VS Code API.

It would be interesting to see if upgrading our version of this package could be done safely, but I believe it might not be the cause of the issue you linked (#787). I'm mostly wary of transitive upgrades to json-rpc, there was a subtle change from 5 to 6 that required care.

@perrinjerome
Copy link
Contributor Author

Thanks for the feedback @paul-marechal I studied this a bit more, I was misunderstanding. To have microsoft/vscode-languageserver-node#787 fixed, what matters is that the extension embeds vscode-languageclient to a version >= 8.0.0, because as you said this code runs in the extension. I also understood that theia does not yet support extensions using vscode-languageclient >= 8.0.0 because #10993 , so it's not yet possible to get the fix.

For sure, for that issue, updating vscode-languageserver-protocol version used by theia like I was suggesting won't have any impact at all. I also opened this issue because I thought that theia might be using an old version vscode-languageserver-protocol for reasons that are not true anymore, but there's probably no problem in using that old version for now, so I would say let's close this issue. I'm clicking on close, I hope you agree.

Thank you again, for your comment, it opened my eyes :)

@perrinjerome perrinjerome closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants