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

No language features for .graphql files since v0.5.0 (only colorization) #2671

Closed
MariaSolOs opened this issue Aug 14, 2022 · 20 comments · Fixed by #2760
Closed

No language features for .graphql files since v0.5.0 (only colorization) #2671

MariaSolOs opened this issue Aug 14, 2022 · 20 comments · Fixed by #2760
Labels

Comments

@MariaSolOs
Copy link
Contributor

MariaSolOs commented Aug 14, 2022

I've been trying to make the latest versions of this extension to work but it seems like completion (and most other language features) broke since v0.5.0. Note that everything works fine in v0.4.15.

The project I'm currently working on is here, in case you want to check out my setup. But as a summary, my graphql.config.json file looks like this:

{
    "schema": "./graphql-server/introspection.json",
    "documents": "./graphql-server/**/*.graphql"
}

And I'm using codegen with the following configuration:

schema: './graphql-server/schema.ts'
documents: './graphql-server/**/*.graphql'
generates:
  ./graphql-server/introspection.json:
    plugins:
      - introspection
    config:
      minify: true
  ./graphql-server/sdk.ts:
    plugins:
      - add:
          content: '// @ts-nocheck Ignore this file, it is just codegen output'
      - typescript
      - typescript-operations
      - typescript-graphql-request
      - plugin-typescript-swr
    config:
      skipTypename: true
  ./graphql-server/resolvers-types.ts:
    plugins:
      - add:
          content: '// @ts-nocheck Ignore this file, it is just codegen output'
      - typescript
      - typescript-resolvers

The logs only show the following:

8/14/2022, 2:55:11 PM [3] (pid: 7005) graphql-language-service-usage-logs: {"type":"usage","messageType":"initialize"}
@MariaSolOs
Copy link
Contributor Author

MariaSolOs commented Aug 15, 2022

About why I use an introspection output as my schema: It's been the only way I've been able to make this extension work. I explained this approach here.

@acao
Copy link
Member

acao commented Aug 15, 2022

No highlighting even?! I’m sorry! I hope to test these cases next weekend and get back to you soon

@MariaSolOs
Copy link
Contributor Author

No highlighting even?! I’m sorry! I hope to test these cases next weekend and get back to you soon

No need to be sorry! Thanks for the quick reply ❤️
Let me know if there's any more information you would like me to provide to ease your investigation!

@MariaSolOs
Copy link
Contributor Author

@acao Any updates on this? I can try taking a look :)
Are there any special steps I should follow if I only want to work with the VS Code extension part of this repo?

@acao acao changed the title No language features since v0.5.0 (only colorization) No language features for .graphql files since v0.5.0 (only colorization) Aug 24, 2022
@acao
Copy link
Member

acao commented Aug 24, 2022

@MariaSolOs very sorry, trying to get to it, some things came up and I haven’t been able to do as much oss as I’ve liked.

the issue is somewhere in graphql-language-service-server. It could be in parseDocument, perhaps some logic was changed and disabled handling graphql files directly? You can clone the repo, run yarn and yarn build —watch while editing the server, and then keep restarting vscode graphql debugger on each server change. I should have these instructions in DEVELOPMENT.md but not sure if i do. PRs are welcome if you have luck!

@MariaSolOs
Copy link
Contributor Author

MariaSolOs commented Aug 25, 2022

No worries @acao! I can try taking a look ;)

So there's no task for F5-debugging the extension? Never mind

@MariaSolOs
Copy link
Contributor Author

Okay so I tried debugging the extension but my breakpoints won't bind anything outside the vscode-graphql package, which makes it hard to understand what's going on in the server :/

@acao
Copy link
Member

acao commented Aug 25, 2022

@MariaSolOs now that I'm actually looking more closely - the unit tests for the language server only include configs like these:

testMultipleIncludes:
    schema: __schema__/StarWarsSchema.graphql
    documents:
      - __queries__/*.graphql
      - __fragments__/*.graphql

is it that only queries and fragments but no SDL .graphql files are working? or are all types of .graphql files not working?
perhaps you can expand on this test graphql config file in the language server tests, and add a test to MessageProcessor-test.ts to test projects with something that reflects your case if you like?

this may be easier than the path I showed you, which is more ideal for manually confirming functionally that what you see working in unit tests is working in vscode

@acao
Copy link
Member

acao commented Aug 25, 2022

Okay so I tried debugging the extension but my breakpoints won't bind anything outside the vscode-graphql package

my bad, i forgot to mention that the debugger for the vscode graphql start only attaches to the client main process for vscode, and not the language server process. we are just missing I think a seperate debugger we can run alongside the vscode graphql debugger, on an alternative port, I'm sure it's in the vscode extension docs somewhere.

unfortunately, the best way we currently have to debug output when testing the language server with vscode or other clients is with the LSP output channel, in vscode or whichever client. in MessageProcessor it is this._logger, by default only this._logger.error() and this._logger.info() is displayed I think :/. I always forget this part, and it is a really cumbersome anti-pattern for developing with the LSP server that we need to improve on

another option is to execute the graphql-language-service-cli directly for completion output. just noticing how vauge the readme is but I think you could even do this:

packages/graphql-language-service-cli/bin/graphql.js validate -c graphql.config.js path/to/file.graphql

but i wouldn't know to pass --inspect there, perhaps you have to replace the #!/usr/bin/env node with #!/usr/bin/env node --inspect ?

@MariaSolOs
Copy link
Contributor Author

@acao The current version of the extension is not working for me on all .grapqhl files + TypeScript files using gql (which is how my schema is defined).

I will try expanding the tests to reflect the setup that I have.

As for debugging the server, it seems like the server already has a debug configuration that should allow me to use the Chrome Dev Tools for debugging the process running the server. I'll check that.
That being said, I agree that it would make it easier for contributors if there was a debugging VS Code setup for the server package :)

@acao
Copy link
Member

acao commented Aug 25, 2022

@MariaSolOs oh that’s how to enable that yes, with the debug extension setting

@MariaSolOs
Copy link
Contributor Author

Okay so I have an update: After debugging for a bit today I discovered that the problem is not parseDocument but in getTokenAtPosition. The returned token is not correct, resulting in whack completions that don't apply to the cursor's position.

As an example, I was comparing the behaviour in v0.4.15 versus the current one with the following mutation:

mutation createSnippet(
    $name: String!, 
    $description: String!
) {
    createSnippet(
        name: $name, 
        description: $description
    ) {
        id
        | <- cursor triggering completions
    }
}

In v0.4.15, the kind of this token would be Field, but now it is Document.

@MariaSolOs
Copy link
Contributor Author

I've also identified #2557 to be the breaking PR.

@acao
Copy link
Member

acao commented Aug 26, 2022

Thank you @MariaSolOs ! This makes a lot of sense!

hopefully i can take a closer look after work today

@acao
Copy link
Member

acao commented Aug 26, 2022

@MariaSolOs i have a PR to get rid of the completion insert text anyways, it doesn’t factor in required arguments so it’s not very useful

@acao
Copy link
Member

acao commented Aug 26, 2022

also for the record, I know this bug exists, because there are duplicates of this issue (they are hard to find because there is no easy error/language to refer to!) I will need to find in this repo for when we finally close this. It's just a matter of the mountain of priorities and PRs already lined up for the LSP, but it would be annoying to ship all of that and still this regression. hopefully I'm very productive this weekend and can get to this! haha

thank you so much for going above and beyond to look at it. in an ideal world you wouldn't have needed to go through this trouble with debugging, but that has been insightful for both this and other issues with the language server/extension DX

@MariaSolOs
Copy link
Contributor Author

No worries at all @acao! I'm glad I was able to help and I completely understand that there are other problems with higher priorities that need to be addressed first.

@MariaSolOs i have a PR to get rid of the completion insert text anyways, it doesn’t factor in required arguments so it’s not very useful
So are completions going to be removed entirely? That's kind of sad because I do find them extremely helpful (when they do take into account the schema and the context of the completion of course).

@acao acao added the bug label Aug 28, 2022
@acao
Copy link
Member

acao commented Aug 28, 2022

@MariaSolOs another clue, i suspect maybe logic I introduced with cacheProjectFiles() in messageprocessor could have caused some issues here

@MariaSolOs
Copy link
Contributor Author

MariaSolOs commented Aug 28, 2022

@MariaSolOs another clue, i suspect maybe logic I introduced with cacheProjectFiles() in messageprocessor could have caused some issues here

@acao So I'm not sure if it is a caching issue per se, but I think I have a fix :) #2720

This was referenced Sep 7, 2022
@acao acao closed this as completed in #2760 Sep 7, 2022
@acao
Copy link
Member

acao commented Sep 7, 2022

@MariaSolOs thank you so much for the extensive effort you put into finding this fix! 0.7.5 should be the fix version for this in vscode-graphql

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants