You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thanks for this — GraphQL variable and input-object support closes #1211 and is a real gap in the input package. It merges cleanly with master, but a few things need fixing before it can land:
New fixture breaks CI for every language.test/inputs/graphql/github-variables.graphql doesn't match the harness's naming scheme: graphQLSchemaFilename in test/fixtures.ts (lines 1064-1072) requires the basename to match /(.*\D)\d+$/ so it can derive the shared .gqlschema file. github-variables has no trailing digit, so it throws "GraphQL test filename does not correspond to naming schema" for every GraphQL fixture that runs. Please rename it (and its .1.json) to github11.graphql / github11.1.json (11, since github10.graphql is already used by this PR). As a result, no language has actually round-tripped the new variables/input-object code in CI yet — worth double-checking output once the fixture runs.
makeVariablesType panics on variable types it doesn't recognize (packages/quicktype-graphql-input/src/index.ts). It only handles NonNull > Named and NonNull > List > Named. A variable like $ids: [ID!] (list of non-null) or a nested list hits listItemType.kind !== VariableKind.NAMED and panics with "Named type not found for list variable...". Since master today ignores query variables entirely, this is a new failure mode for previously-fine queries. VariableKind only has NAMED/LIST/NON_NULL, so a small recursive walk over those three node kinds instead of the fixed pattern match should handle all shapes.
makeIRTypeFromInputObject silently emits the null primitive for input objects nested deeper than 3 levels (the _inputObjectDepth > 3 check, with the // TODO: Support objects with depth > 3 and recursive references comment). This is easy to hit with recursive filter-style input types. At minimum this should panic with a clear message rather than silently producing a wrong type; full support would mean memoizing input-object types by name so recursion terminates without an arbitrary depth cutoff (happy to have that be a follow-up PR).
Once the fixture is renamed, the variables-type walk is generalized, and the depth-3 case panics instead of going silent, this looks mergeable to me. Let us know if you'd like help landing any of these.
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
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.
Description
Implements #1212
Related Issue
Fixes #1211