Support fragment arguments on GraphQL 17 - #429
Draft
JoviDeCroock wants to merge 1 commit into
Draft
Conversation
Parses every document with `experimentalFragmentArguments` and validates the parsed document instead of letting graphql-language-service re-parse it without that option, so `fragment Fields($size: Int!) on Product` and `...Fields(size: $size)` stop being reported as syntax errors. Older GraphQL versions ignore the option. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: d0d3425 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stops fragment arguments from being reported as syntax errors when the project is on GraphQL 17:
What changed
src/graphql/parse.tswrapsgraphql'sparsewithexperimentalFragmentArguments, the option GraphQL 17 gates this syntax behind. Every call site that parsed documents —ast/index.ts,autoComplete.ts,checkImports.ts,definition.ts,diagnostics.ts,fieldUsage.ts— now goes through it. GraphQL 15 and 16 ignore unknown parse options, so nothing changes there: the syntax keeps reporting a syntax error, because those versions genuinely can't represent it.getDiagnostics, which parses the document itself without those options.getDocumentDiagnosticsreproduces what it does — concatenating the printed external fragments, then validating — but parses with our options and hands the AST tovalidateQuery. GraphQL 17'sspecifiedRulesalready validate fragment arguments (required, unknown, and mis-typed arguments), so no custom rules are needed. If the document genuinely doesn't parse, it falls back togetDiagnosticsfor the ranged syntax error it produces.graphql'sprintrather than@0no-co/graphql.web's, so the AST is printed by the same version that parsed it. Printing a GraphQL 17 fragment definition with graphql.web 1.3 silently drops its argument definitions, which would then surface as bogus validation errors.Not covered here
Persisted-document hashing (
persisted.ts) parses and prints with@0no-co/graphql.web, which can't represent fragment arguments until 0no-co/graphql.web#83 lands. Routing it throughgraphqlinstead isn't an option — the hash is derived from graphql.web's printed output, so switching printers would invalidate every existing persisted hash. Left as a follow-up: bump the dependency once that release is out. This is not a regression; that path never handled the syntax.Tests
test/unit/fragmentArguments.test.tspins both halves of the contract: thatPARSE_OPTIONSis what unlocks the syntax on GraphQL 17 (agraphql17dev alias is added for this), and that the option is inert on the GraphQL 16 the repo tests against — ordinary documents still parse, and a fragment-arguments document still produces exactly one syntax diagnostic through the fallback rather than crashing.The end-to-end "no diagnostics on GraphQL 17" case isn't covered in CI. The plugin resolves
graphqlfrompackages/graphqlsp/node_modules, which is 16 for every fixture project, and aliasing it in Vitest doesn't reachgraphql-language-service, which resolves its own copy and then rejects the schema as coming "from another module or realm". Getting real coverage means running the suite (or a fixture project) against GraphQL 17, which felt like a separate change. Verified by hand in the meantime.Full unit suite passes (21). All e2e files pass except
unused-fiedsandmulti-schema-tada, which fail identically onmainin this environment and vary run to run (2–5 failures for the same code), so they look load-related rather than caused by this change.🤖 Generated with Claude Code