search

graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.06k stars 1.72k forks source link

(graphql-language-service-server) Cannot find definition with neovim builtin LSP client #1977

Closed bryclee closed 2 years ago

bryclee commented 3 years ago

graphql-language-service-server does not with with the builtin neovim LSP client. From some debugging, it seems like the main difference between the builtin LSP client and coc.nvim, another popular neovim LSP client which works for my use cases, is that the initial value for version is 0, while the other LSP client sends 1.

Because of this, the below logic doesn't store the document in the cache, and it can't be used for later commands like definition requests.

https://github.com/graphql/graphiql/blob/72bff0e7db46fb53293efc990dc64d2c06401459/packages/graphql-language-service-server/src/MessageProcessor.ts#L1063

I didn't see anything in the spec that says the version must start at 1, so it seems like this should be fixed in the language server side. Adding a check to compare against undefined instead is enough to make the language server work for this scenario.

Happy to submit this as a PR - I don't have experience with LSP clients, so I wanted to get confirmation before submitting the change.

acao commented 3 years ago

Thanks for this deep dive! A PR would be more than welcome for this. This is an embarrassing oversight on my part I believe. I wonder if the unit tests are testing this properly as well?

bryclee commented 2 years ago

Verified this is fixed with #2055, thanks!