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

Proposal: More LSP friendly package names #1404

Closed
acao opened this issue Mar 6, 2020 · 5 comments · Fixed by #1459 or #1521
Closed

Proposal: More LSP friendly package names #1404

acao opened this issue Mar 6, 2020 · 5 comments · Fixed by #1459 or #1521

Comments

@acao
Copy link
Member

acao commented Mar 6, 2020

We're working to further LSP-ify our language services and server in preparation for monaco and an improved LSP interface and experience (see: https://github.com/graphql/graphiql/project/6).

(Edit: Revised, as npm will not allow us to publish both graphql-language-service and graphql-languageservice)

Overall, we need graphql-language-service to refer to a runtime agnostic service, and for the CLI to become another package with an explicit name, such as graphql-language-service-cli

Deprecating the binary

To transition users from the graphql bin included in graphql-language-service, the new graphql-language-service package will include a bin temporarily that just stderrs to install the new cli package, as I've seen webpack do.

CLI Usage Notes

image

The GraphQL Language Service has taken off in terms of usage, however by comparison to our other packages in the monorepo it's still the least installed. That said, it's often installed as a global bin, so usage will be different.

Also, to note, I've seen a few implementations of it that bundle the packaged bin manually with whatever IDE plugin that uses it, as their actual IDE plugin code is written in python, lua, etc.

So, a significant amount of users could be impacted by this.

Renaming the bin

Another consideration is renaming the bin itself. graphql bin path is being used by graphql-cli, which we may end up using/reccomending for scaffolding plugins. After working with a wide range of IDE extension developers (not just vscode devs), LSP is very descriptive.

So, I reccomend changing the bin as such:

graphql => graphql-lsp

@rebornix
Copy link
Contributor

rebornix commented Mar 6, 2020

consider a full document persistence layer in the language service, alongside the cacheing

Maybe you are considering doing that: once having a document cache in the language server side, you may want to turn on document incremental sync mode to reduce the communication payload.

Besides, if you don't want to implement a document store yourself, you can use the VS Code one https://github.com/microsoft/vscode-textbuffer .

@acao
Copy link
Member Author

acao commented Mar 6, 2020

oh brilliant, even better

currently we cache things like directives, definitions, etc, so whole files seems the logical next step, so that each implementation doesn't need to worry about that

@acao
Copy link
Member Author

acao commented Mar 6, 2020

also from @rebornix, for posterity's sake:

I like the idea of aligning the name LAN-languageservice and Monaco-graphql. I was confused the very first time when I tried graphql ls code as I thought it's platform (browser/node) agnostic.

@acao
Copy link
Member Author

acao commented Mar 6, 2020

@rebornix had this same experience with a few other devs who were LSP familiar, good to know this matters from the vscode side

@acao
Copy link
Member Author

acao commented May 29, 2020

done!

@acao acao closed this as completed May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants