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

Configurable LSP transport #3819

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robertoaloi
Copy link

@robertoaloi robertoaloi commented Dec 6, 2024

Currently, the VS Code GraphQL extension enforces IPC as the communication transport between the language client and the server. This is only one of the 4 communication channels recommended by the LSP specification.

Both the client and the server support stdio as a communication channel, so this change makes it configurable. This can be convenient in those environments where IPC communication may not be available.

The default transport is unchanged.

A setting is added to allow users to quickly change the communication channel:

Screenshot 2024-12-06 at 16 05 51

Tested via the VS Code LSP Extension Run functionality in VS Code:

  • Added packages/graphql-language-service-server/src/__tests__ to the workspace
  • Changed the setting to stdio
  • Opened the test.graphql file
  • Got auto-completion
  • GraphQL Language Server Output looks correct

Copy link

changeset-bot bot commented Dec 6, 2024

🦋 Changeset detected

Latest commit: c1e6bf2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
vscode-graphql Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

linux-foundation-easycla bot commented Dec 6, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@acao
Copy link
Member

acao commented Dec 6, 2024

good idea! originally vscode only supported IPC, wasn't aware that they support this. or other LSP users outside of vscode use these transports every day so they should indeed be ready to go

@robertoaloi
Copy link
Author

@acao Great to hear. Anything missing or can this be merged, then?

@acao
Copy link
Member

acao commented Dec 11, 2024

@robertoaloi I still need to confirm the bundled extension works. since you tested with Extension Run, that bypasses the esbuild and vsix bundle factors, though it should be fine.

you can test it yourself by running vsce:package in vscode graphql and right clicking the .vsix and chose "install extension", vaugely similar to how testing browser plugins works

@robertoaloi
Copy link
Author

@acao

$ yarn run vsce:package
yarn run v1.22.22
warning [email protected]: The engine "vscode" appears to be invalid.
$ vsce package --yarn
Executing prepublish script 'yarn run vscode:prepublish'...
warning [email protected]: The engine "vscode" appears to be invalid.
$ npm run compile -- --minify

> [email protected] compile
> node esbuild --minify

successfully bundled vscode-graphql 🚀
 DONE  Packaged: [REDACTED]/graphql/graphiql/packages/vscode-graphql/vscode-graphql-0.12.1.vsix (127 files, 51.92MB)
 INFO
The latest version of @vscode/vsce is 3.2.1 and you have 2.29.0.
Update it now: npm install -g @vscode/vsce
✨  Done in 11.82s.

Installed the .VSIX and everything seems to be working fine. Just a typo I'm going to fix now.

@acao
Copy link
Member

acao commented Dec 13, 2024

good eye, I didn't even see the typo! you tested the extension when switching the modes and the server restarted appropriately, yes?

this should be good for now, we should add the socket transport as well but I can add/test that later

@robertoaloi
Copy link
Author

I reloaded the window with both settings and verified the server started in both cases and provided auto-completion.

@robertoaloi
Copy link
Author

@acao We are good to go from my side, if you are happy.

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

Successfully merging this pull request may close these issues.

2 participants