Support queries for multiple schemas in the same file #3411

frandiox · 2023-08-24T08:51:59Z

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 · 2023-08-24T08:52:02Z

⚠️ No Changeset found

Latest commit: 92f2c7b

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 · 2023-08-24T08:52:03Z

The committers listed above are authorized under a signed CLA.

✅ login: frandiox / name: Fran Dios (935ae8a, b20c7ed, d09f46d, ecd29f8, ffca1f7, 196d830, 5148707, 5dcd471, 12acf3e, 6b9582c, 63e82ba, 92f2c7b, bbc8a6e, b956407, 80a3541)

frandiox · 2023-08-24T08:53:08Z

packages/graphql-language-service-server/src/MessageProcessor.ts

@@ -288,6 +288,39 @@ export class MessageProcessor {
    );
  }

+  async _getDiagnosticsForAllFileProjects(


This method is extracted from the code below to avoid repetition, and it's adapted to run for every project that matches the file.

frandiox · 2023-08-24T08:54:35Z

packages/graphql-language-service-server/src/findGraphQLTags.ts

-      const hasGraphQLPrefix =
-        node.quasis[0].value.raw.startsWith('#graphql\n');
+      const hasGraphQLPrefix = node.quasis[0].value.raw.startsWith('#graphql');


This makes the matching similar to the syntax highlight here, allowing #graphql:suffix.

frandiox · 2023-08-24T09:00:18Z

packages/graphql-language-service-server/src/GraphQLLanguageService.ts

-    if (config) {
-      return config;
+  getAllProjectsForFile(uri: Uri) {
+    const filePath = uri.startsWith('file:') ? fileURLToPath(uri) : uri;


Passing an Uri that starts with the file:// protocol doesn't work properly in grahiQLConfig.getProjectForFile. Internally that calls minimatch and it always returns false.

yes there was a breaking change in vscode some years ago, and there are a few places that we didn't handle this, thanks for the bugfix! I will come back around look at the implementations because they should all be passing a file url

acao · 2023-09-26T20:49:04Z

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 · 2023-09-26T20:57:42Z

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).

❗ 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%)`	⬆️
...hql-language-service-server/src/findGraphQLTags.ts	`82.71% <100.00%> (-1.90%)`	⬇️
...guage-service-server/src/GraphQLLanguageService.ts	`45.45% <82.35%> (+3.65%)`	⬆️
...ql-language-service-server/src/MessageProcessor.ts	`46.97% <84.84%> (+1.31%)`	⬆️

... and 7 files with indirect coverage changes

packages/graphql-language-service-server/src/GraphQLLanguageService.ts

benjaminsehl · 2023-11-14T03:11:48Z

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

… into fd-gql-multiple-schema

frandiox · 2023-12-11T14:13:55Z

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

acao · 2023-12-11T23:26:46Z

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 · 2023-12-14T12:31:01Z

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 · 2024-01-07T16:21:25Z

@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 · 2024-01-07T16:22:33Z

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

frandiox · 2024-01-08T19:23:28Z

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?

TallTed

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

packages/graphql-language-service-server/README.md

packages/graphql-language-service-server/src/findGraphQLTags.ts

packages/vscode-graphql/README.md

frandiox · 2024-01-11T09:47:40Z

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

acao · 2024-01-24T20:10:59Z

packages/graphql-language-service-server/src/GraphQLLanguageService.ts

@@ -111,7 +149,7 @@ export class GraphQLLanguageService {
    // Perform syntax diagnostics first, as this doesn't require
    // schema/fragment definitions, even the project configuration.
    let documentHasExtensions = false;
-    const projectConfig = this.getConfigForURI(uri);
+    const projectConfig = this.getProjectForQuery(document, uri);


can you rename this to getProjectForDocument ? just so people don't assume this only works with queries, as it should also work with SDL

Updated! I've also resolved the new conflicts but fyi, there's a problem with cspell on commit because it finds unknown words in .vscode/settings.json.

frandiox added 2 commits August 23, 2023 18:12

feat: support queries for multiple schemas in the same file

ecd29f8

Test queries for multiple schemas in the same file

b20c7ed

frandiox commented Aug 24, 2023

View reviewed changes

frandiox changed the title ~~feat: support queries for multiple schemas in the same file~~ Support queries for multiple schemas in the same file Aug 24, 2023

frandiox commented Aug 24, 2023

View reviewed changes

frandiox added 2 commits September 26, 2023 22:51

feat: support queries for multiple schemas in the same file

ffca1f7

Test queries for multiple schemas in the same file

bbc8a6e

acao force-pushed the fd-gql-multiple-schema branch from b20c7ed to bbc8a6e Compare September 26, 2023 20:51

frandiox commented Sep 27, 2023

View reviewed changes

packages/graphql-language-service-server/src/GraphQLLanguageService.ts Outdated Show resolved Hide resolved

frandiox added 3 commits December 11, 2023 14:51

Merge branch 'fd-gql-multiple-schema' of github.com:frandiox/graphiql…

196d830

… into fd-gql-multiple-schema

Fix uri in non-annotated cases

5dcd471

Merge remote-tracking branch 'origin/main' into fd-gql-multiple-schema

5148707

michenly mentioned this pull request Dec 14, 2023

Use Customer Account API in templates/skeleton & template/demo-store Shopify/hydrogen#1576

Merged

16 tasks

frandiox added 2 commits January 8, 2024 19:28

Merge branch 'main' into fd-gql-multiple-schema

63e82ba

Add docs to readmes

12acf3e

TallTed suggested changes Jan 8, 2024

View reviewed changes

frandiox mentioned this pull request Jan 15, 2024

Support annotation suffixes for codegen Shopify/hydrogen#1635

Draft

acao reviewed Jan 24, 2024

View reviewed changes

frandiox and others added 6 commits January 25, 2024 16:35

Rename function

935ae8a

Fix type in related test

6b9582c

Update doc with partial feedback

b956407

Merge remote-tracking branch 'origin/main' into fd-gql-multiple-schema

d09f46d

Merge branch 'main' into fd-gql-multiple-schema

80a3541

Merge branch 'main' into fd-gql-multiple-schema

92f2c7b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support queries for multiple schemas in the same file #3411

Support queries for multiple schemas in the same file #3411

frandiox commented Aug 24, 2023

changeset-bot bot commented Aug 24, 2023 •

edited

linux-foundation-easycla bot commented Aug 24, 2023 •

edited

frandiox Aug 24, 2023

frandiox Aug 24, 2023

frandiox Aug 24, 2023

acao Sep 26, 2023

acao commented Sep 26, 2023

codecov bot commented Sep 26, 2023 •

edited

benjaminsehl commented Nov 14, 2023

frandiox commented Dec 11, 2023 •

edited

acao commented Dec 11, 2023

frandiox commented Dec 14, 2023

acao commented Jan 7, 2024

acao commented Jan 7, 2024

frandiox commented Jan 8, 2024

TallTed left a comment

frandiox commented Jan 11, 2024 •

edited

acao Jan 24, 2024

frandiox Jan 25, 2024

Support queries for multiple schemas in the same file #3411

Are you sure you want to change the base?

Support queries for multiple schemas in the same file #3411

Conversation

frandiox commented Aug 24, 2023

changeset-bot bot commented Aug 24, 2023 • edited

⚠️ No Changeset found

linux-foundation-easycla bot commented Aug 24, 2023 • edited

frandiox Aug 24, 2023

Choose a reason for hiding this comment

frandiox Aug 24, 2023

Choose a reason for hiding this comment

frandiox Aug 24, 2023

Choose a reason for hiding this comment

acao Sep 26, 2023

Choose a reason for hiding this comment

acao commented Sep 26, 2023

codecov bot commented Sep 26, 2023 • edited

Codecov Report

benjaminsehl commented Nov 14, 2023

frandiox commented Dec 11, 2023 • edited

acao commented Dec 11, 2023

frandiox commented Dec 14, 2023

acao commented Jan 7, 2024

acao commented Jan 7, 2024

frandiox commented Jan 8, 2024

TallTed left a comment

Choose a reason for hiding this comment

frandiox commented Jan 11, 2024 • edited

acao Jan 24, 2024

Choose a reason for hiding this comment

frandiox Jan 25, 2024

Choose a reason for hiding this comment

changeset-bot bot commented Aug 24, 2023 •

edited

linux-foundation-easycla bot commented Aug 24, 2023 •

edited

codecov bot commented Sep 26, 2023 •

edited

frandiox commented Dec 11, 2023 •

edited

frandiox commented Jan 11, 2024 •

edited