graphql / vscode-graphql

MIGRATED: VSCode GraphQL extension (autocompletion, go-to definition, syntax highlighting)
https://marketplace.visualstudio.com/items?itemName=Prisma.vscode-graphql
MIT License
555 stars 71 forks source link

Fix: Hello, please minute of attention, https://github.com/graphql/graphiql/pull/1715 was not applied #320

Closed orsenkucher closed 2 years ago

orsenkucher commented 3 years ago

Hello, please bump graphql-language-service-server dependency from ^2.5.6 -> ^2.5.7 or later or update .lock files Because, as my investigation has shown current dependency setup does not contain changes from https://github.com/graphql/graphiql/pull/1715

Thus I don't have Go to Definition navigation working, the URI of file resource to open is malformed for windows.

Steps to reproduce:

  1. clone repo
  2. run npm install
  3. look at node_modules\graphql-language-service-server\dist\MessageProcessor.js\MessageProcessor\handleDefinitionRequest and at ln: 438 your should see
    return {
    uri: res.path.indexOf('file:///') === 0
        ? res.path
        : `file://${path.resolve(res.path)}`,
    range: defRange,
    };

    If you run from Debug -> Extension now, code navigation won't work on windows.

  4. now delete the yarn.lock, package-lock.json and node_modules
  5. run npm install again
  6. look up the same file, at ln: 440 you will see
    return {
    uri: res.path,
    range: defRange,
    };

    Which is now correct and everything works as expected 🥰

orsenkucher commented 3 years ago

Ok, so I've downloaded .tgz from npm graphql-language-service-server of version 2.5.6 and it doesn't contain needed fix

orsenkucher commented 3 years ago

Published https://marketplace.visualstudio.com/items?itemName=orsenkucher.vscode-graphql And it works on Windows 😱🤯😎🥳🤪

ferm10n commented 3 years ago

Creating a PR might be a good idea to get the maintainers to check this out. I'm curious why you added codemirror and change the typescript dependency though https://github.com/orsenkucher/vscode-graphql/commit/06fbd5fec23be82a5622bc1e315e564f95550f00#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

acao commented 3 years ago

We just need a PR to update our language server to use our latest LSP release! This repo will soon be merged with graphiql repo to make this part easier. If someone wants to create a PR just to update the language server to our latest version that would be rad

acao commented 2 years ago

this should be released now! let me know if there are issues

NamanShergill commented 2 years ago

this should be released now! let me know if there are issues

I am still having this issue, I cannot use go to definition as it just says unable to resolve reference

acao commented 2 years ago

Are the files that contain fragments listed in documents?

NamanShergill commented 2 years ago

Are the files that contain fragments listed in documents?

Yes, VS Code says it's unable to resolve the resource and lists the exact path of the file. unknown

NamanShergill commented 2 years ago

Sorry for some of the path being drawn over as this is the only image I have as I'm not on my PC, but that's the exact path the file is in.