Ensure diagnostics get regenerated on schema save

[yasamoka]

First of all, thank you for all of this great work!

I just installed the GraphQL.vscode-graphql extension yesterday and noticed that when I update the GraphQL schema and save it, diagnostics for queries / mutations do not get updated. For example, say I have a mutation using a field that no longer exists in the new schema; diagnostics would not show up for that mutation until I switch to the file containing the mutation and edit it. I assume this is the issue in #2357.

I was also thinking that it may be a good idea to separate the cached documents according to project since with the current way cached documents are stored, there seems to be no way to differentiate between documents belonging to different projects.

I am not very familiar with the overall structure of the project or the API as I just learned about this extension yesterday, so any advice regarding potentially better caching behavior, or any existing implementation details, would be appreciated.

Thank you again!

[changeset-bot]

⚠️ No Changeset found

Latest commit: 010fd32

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: yasamoka / name: Ramzi Sabra (8da9516, 57e72c7, 7e15297, a557677, 010fd32)

[yasamoka]

Just realized that I should run the diagnostics only once after any schema invalidation has happened. I can push this change once the initial change has been confirmed to be useful and practical.

[yasamoka]

I was also thinking that it may be a good idea to separate the cached documents according to project since with the current way cached documents are stored, there seems to be no way to differentiate between documents belonging to
different projects.

Found the getProjectForFile() function so I used that to filter by project.

Just realized that I should run the diagnostics only once after any schema invalidation has happened. I can push this change once the initial change has been confirmed to be useful and practical.

Fixed that as well.

[acao]

awesome stuff! thank you!

[codecov]

Codecov Report

Merging #3301 (010fd32) into main (045329e) will decrease coverage by 0.27%.
The diff coverage is 23.80%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3301      +/-   ##
==========================================
- Coverage   56.40%   56.13%   -0.27%     
==========================================
  Files         109      109              
  Lines        5159     5198      +39     
  Branches     1407     1415       +8     
==========================================
+ Hits         2910     2918       +8     
- Misses       1825     1853      +28     
- Partials      424      427       +3     

Impacted Files	Coverage Δ
...ql-language-service-server/src/MessageProcessor.ts	44.06% <23.80%> (-2.17%)	⬇️

[yasamoka]

awesome stuff! thank you!

Thanks for the kind words!

I actually have an incoming pull request that revamps diagnostics entirely. It's already in my fork ("diagnostics" branch) but I still need to fix a test and add a change that is currently local.

[acao]

@yasamoka awesome! I was in the midst of doing something similar this week when discovering some of these issues (for example, the unnecessary re-parsing of the document!) while working on a branch that adds end to end tests for vscode-graphql. the more tests you can add the better. i plan on overhauling the unit tests in language server to make it easier to test multi-file configs as well.

just make sure that it works for both file based schemas and remote schemas, as the schema save event only happens for users with HTTP schemas, so you need to be able to detect when incoming file changes are schema related as well. detecting schema changes when schema is schema: ["schema/**.ts", "anotherfile/types.ts", "@directive something"] also needs to be possible. my approach in my local branch was to generate a hash for the schema based on the printed SDL output to be able to detect whether it has changed. the local branch also re-runs diagnostics for the entire document cache instead of just the current file

I also have a major rewrite of the language server coming, so be prepared to wait as long as a month for all of this to get merged. To follow the pattern of the rewrite, move as much functionality as you can to GraphQLCache. we will be moving the document cache to GraphQLCache for example.

[acao]

See #2609 for some additional prior art,

As I did with the new GraphiQL 2 architecture, I've created a draft proposal I will be working on here

#3314

[yasamoka]

Thank you for pointing me in the right direction. I am still very new to this extension so I might need a bit of guidance. I'm wrapping up a project where this extension was a big boost to keeping everything sane so maybe in a week or so, I should be done with that. Do you think it's a good idea to keep the pull requests small and incremental? In other words, do I push what I already have regarding revamping diagnostics then we base further work on top of that? Let me know.

[acao]

@yasamoka I'm elated to hear it was helpful for you! happy to help guide you. can you and your colleauges leave a review on the vscode marketplace because our reviews have plummeted lately

yes, incremental is better, and I plan to carry out the re-write as such as well.

if you can add unit test coverage for the code you add here, that would be super helpful! codecov highlights which lines are missing coverage in the file diff

[acao]

for some reason, this only seems to work for me if the file tab is closed and re-opened. in another PR I have it working so that the diagnostics reload immediately on schema file change, so that the files highlight immediately. maybe it's just a merge conflict issue but I can't tell