Support queries for multiple schemas in the same file

[frandiox]

Hi 👋 First of all, thanks so much for the active maintenance of these packages!

While working on shopify/hydrogen I noticed that the GraphQL Language Service Server so far only supports 1 schema per file. However, we are introducing multiple GraphQL APIs and it would be nicer for DX to combine queries in the same file instead of separating them in different files.

This is an attempt to implement that. I was just trying to see if it was possible at all to open an issue, but it actually works so I'm just opening a PR instead 😅 -- feel free to discuss this feature completely!

The idea is that you need a config like the following with 2 projects pointing to the same files. Then, you can pass an option to set a "gql annotation suffix" in extensions.languageService.gqlTagOptions.annotationSuffix:

Then, in a given file that matches the pattern for both projects, you can choose to which GraphQL API/schema each of the queries points to with a modifier of the GraphQL annotation: #graphql:<suffix>

---

This whole thing also works with GraphQL Codegen by using documentTransforms. Also, noticed that syntax highlights already works due to how the match happens in that package (doesn't end in \n).

This feature at the moment only works when using the in-query comment #graphql. It doesn't work with gql tag or the leading comment /* GraphQL */ because there's no access to that information where it's needed (maybe that could be changed? Unsure). Another option is to just read a different comment inside the query, like #project:storefront and make it independent from the #graphql comment.

Thanks for checking it!

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 92f2c7bc7254c3d233ded8a9bfaa580cf51bf78a

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[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: frandiox / name: Fran Dios (935ae8a2cf5c7406d3e8f2c3f70acb0f3fcf1c54, b20c7edd02f7eee3d0f34047007e9d9751cc3bbf, d09f46d78d07ff3d8126232e8f4be38ff15618d1, ecd29f88eadd0ff38ee4ae5c40a64514ec4228c9, ffca1f75bce2afd147a4b1477956834b777989f1, 196d8307933e476ce9f7e22c66d713df7a735575, 5148707c7336e6f93dd9309be61f7a5186c2382e, 5dcd471cec3624e5582fa81eca1eaf6375de81ac, 12acf3e0c4c84c5920576c6e20de9c00ec876fb0, 6b9582cfeae0145cbe051c06020e9821a5f64c84, 63e82ba262d084b78635efd4c9b89ed4dad59038, 92f2c7bc7254c3d233ded8a9bfaa580cf51bf78a, bbc8a6e4668f1f6e839c64b1beb830bfe387afe4, b956407a9621b61128bc29321e68420199b3f758, 80a354134c151348e74ac6877585be76c2aefd57)

[acao]

never thought to implement this because graphql-config didn't seem to support it, but here we are and it's a great idea. as you may have noticed, isolated projects end up accidentally sharing schema as a bug in many contexts, so I'm glad you've added projects to the test mock as I'm planning to fix that as well.

I will focus on a few bugfixes this week, but with my current day job workload, I can't promise I'll have time to fully review this until next week. The lack of test coverage makes things precarious and there are a number of manual scenarios I like to check, but it seems you've already fixed a few known bugs yourself! And added coverage, thanks for that!

CC @dotansimha @Urigo

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 84.61538% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 56.01%. Comparing base (29e99a8) to head (12acf3e).

:exclamation: Current head 12acf3e differs from pull request most recent head 92f2c7b. Consider uploading reports for the commit 92f2c7b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3411      +/-   ##
==========================================
+ Coverage   55.33%   56.01%   +0.67%     
==========================================
  Files         115      114       -1     
  Lines        5349     5311      -38     
  Branches     1450     1441       -9     
==========================================
+ Hits         2960     2975      +15     
+ Misses       1963     1909      -54     
- Partials      426      427       +1     

Files	Coverage Δ
...raphql-language-service-server/src/GraphQLCache.ts	50.88% <100.00%> (+2.12%)	:arrow_up:
...hql-language-service-server/src/findGraphQLTags.ts	82.71% <100.00%> (-1.90%)	:arrow_down:
...guage-service-server/src/GraphQLLanguageService.ts	45.45% <82.35%> (+3.65%)	:arrow_up:
...ql-language-service-server/src/MessageProcessor.ts	46.97% <84.84%> (+1.31%)	:arrow_up:

... and 7 files with indirect coverage changes

[benjaminsehl]

Any chance you could take a look at this @acao? 🙏

[frandiox]

I've merged main branch and resolved conflicts. Tests are still passing ✅

[acao]

unfortunately this impacts a lot of behavior for which there is no test coverage, so an exhaustive set of manual tests will be necessary.

If it makes sense to introduce this after review with the spec committees, I should be able to get to it in the coming months. I hope to present it at the January WG, or perhaps you can present it at the next graphql working group? There is some high priority stability & test work that needs to come before major changes, as requested by the TSC and other stakeholders.

Though the LSP is only at about 50% coverage, even 100% coverage won't account for all the scenarios we need to cover before we can make major changes.

[frandiox]

Understandable! It would be great if you could mention this at the January WG 🙌 In the meantime, is there any public list of use cases that are normally tested manually? That would help us and the community to contribute with more unit/e2e test cases whenever possible.

[acao]

@frandiox the readme for the language server and the graphql config website document many combinations of test cases we don't test for

also if you can update the PR, i refactored tag parsing and improved types in the language server some. I think we can get this improvement out there sooner than later!

[acao]

oh and, can you add documention of this feature to the language server and vscode-graphql readmes?

[frandiox]

also if you can update the PR, i refactored tag parsing and improved types in the language server some. I think we can get this improvement out there sooner than later! oh and, can you add documention of this feature to the language server and vscode-graphql readmes?

Done! Let me know if you prefer docs in some other way.

Also, are we OK with the suggested languageService.gqlTagOptions.annotationSuffix public API? Or prefer to rename it / unnest it?

[frandiox]

A little human-facing language clarification. If it seems I've gotten any wrong, that's a good sign more clarification is needed. :-)

Hi 👋 Thanks for the suggestions. It looks like most of the changes are not related to this PR, though. Should we move it to a different PR? Not sure how strict things should be with PRs in this repo cc @acao

[frandiox]

@acao Hi! I see there was a major refactoring in main branch, and that there's another PR on the way with more changes.

Do you think it makes sense to update this PR now and resolve conflicts or wait until new notice?

[TallTed]

[@TallTed] A little human-facing language clarification. If it seems I've gotten any wrong, that's a good sign more clarification is needed. :-)

[@frandiox] Hi 👋 Thanks for the suggestions. It looks like most of the changes are not related to this PR, though. Should we move it to a different PR? Not sure how strict things should be with PRs in this repo cc @acao

GitHub's change request interface includes an "add commit to batch" or similar button for you to use. If you don't use that button (i.e., you apply the change manually), it's difficult to be sure that change requests have been fully applied, and people like me have to review the current PR character-by-character.

As it stands, it appears that you've rejected all the change requests that are now "marked resolved". There's no argument in place, saying why some change request has been rejected, so there's no argument I can make for applying them, nor any way for me to revise my request.

If you do have good reason to reject any request, please state it inline, or use that change request to start a new issue, such that the work I've already done here is not wasted, and all interested parties can weigh in.

(As to whether my change requests are relevant to this PR -- I generally confine my suggestions to the lines changed by any given PR, though sometimes they extend to a nearby line, which is displayed in GitHub's "Files changed" pane. My change requests are generally human-focused and non-normative. There shouldn't be a problem with including such changes in any PR, even if totally unrelated. However, if you feel that any given change request does deserve to be in a separate PR, you can always use that change request as the start of a new issue, as described earlier.)