Skip to content

Commit

Permalink
Include __typename on interface-typed fragments
Browse files Browse the repository at this point in the history 
We ask for `__typename` on interface-typed fields so that we know which
type to use when unmarshaling.  We never strictly need this for
fragments, because the context into which they're spread must either
have `__typename` or must be of concrete type.  But a few cases have
come up where it would be somewhat convenient: first, if you're using
genqlient's types for mocking (not really supported right now, but
possible); and second, for consistency if we add an option to "flatten"
fragments (#30), which would effectively drop the `__typename` field
from the structs otherwise.  Neither is a deeply compelling reason, I
feel, but it's not too hard to do so we may as well.

Test plan:
make check

Reviewers: marksandstrom, steve, jvoll, adam, miguel, mahtab
  • Loading branch information
@benjaminjkraft
benjaminjkraft committed Sep 23, 2021
1 parent 5995653 commit bdc8720
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 39 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ When releasing a new version:

### New features:

- genqlient now adds `__typename` to fragments of interface type, in addition to fields of interface type. This isn't necessary for any of genqlient's operation, but may be convenient to some callers.

### Bug fixes:

- The `omitempty` option now works correctly for struct- and map-typed variables, matching `encoding/json`, which is to say it never omits structs, and omits empty maps. (#43)
Expand Down
81 changes: 49 additions & 32 deletions generate/generate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,39 +159,13 @@ func (g *generator) usedFragments(op *ast.OperationDefinition) ast.FragmentDefin
// when unmarshaling.
func (g *generator) preprocessQueryDocument(doc *ast.QueryDocument) {
var observers validator.Events
// We want to ensure that everywhere you ask for some list of fields (a
// selection-set) from an interface (or union) type, you ask for its
// __typename field. There are four places we might find a selection-set:
// at the toplevel of a query, on a field, or in an inline or named
// fragment. The toplevel of a query must be an object type, so we don't
// need to consider that. And fragments must (if used at all) be spread
// into some parent selection-set, so we'll add __typename there (if
// needed). Note this does mean abstract-typed fragments spread into
// object-typed scope will *not* have access to `__typename`, but they
// indeed don't need it, since we do know the type in that context.
// TODO(benkraft): We should omit __typename if you asked for
// `# @genqlient(struct: true)`.
observers.OnField(func(_ *validator.Walker, field *ast.Field) {
// We are interested in a field from the query like
// field { subField ... }
// where the schema looks like
// type ... { # or interface/union
// field: FieldType # or [FieldType!]! etc.
// }
// interface FieldType { # or union
// subField: ...
// }
// If FieldType is an interface/union, and none of the subFields is
// __typename, we want to change the query to
// field { __typename subField ... }

fieldType := g.schema.Types[field.Definition.Type.Name()]
if fieldType.Kind != ast.Interface && fieldType.Kind != ast.Union {
handleSelection := func(typ *ast.Definition, selectionSet *ast.SelectionSet) {
if typ.Kind != ast.Interface && typ.Kind != ast.Union {
return // a concrete type
}

hasTypename := false
for _, selection := range field.SelectionSet {
for _, selection := range *selectionSet {
// Check if we already selected __typename. We ignore fragments,
// because we want __typename as a toplevel field.
subField, ok := selection.(*ast.Field)
Expand All @@ -201,7 +175,7 @@ func (g *generator) preprocessQueryDocument(doc *ast.QueryDocument) {
}
if !hasTypename {
// Ok, we need to add the field!
field.SelectionSet = append(ast.SelectionSet{
*selectionSet = append(ast.SelectionSet{
&ast.Field{
Alias: "__typename", Name: "__typename",
// Fake definition for the magic field __typename cribbed
Expand All @@ -216,10 +190,53 @@ func (g *generator) preprocessQueryDocument(doc *ast.QueryDocument) {
},
// Definition of the object that contains this field, i.e.
// FieldType.
ObjectDefinition: fieldType,
ObjectDefinition: typ,
},
}, field.SelectionSet...)
}, *selectionSet...)
}
}

// We want to ensure that everywhere you ask for some list of fields (a
// selection-set) from an interface (or union) type, you ask for its
// __typename field. There are four places we might find a selection-set:
// at the toplevel of a query, on a field, or in an inline or named
// fragment. The toplevel of a query must be an object type, so we don't
// need to consider that. We don't strictly need __typename on fragments
// -- they are always spread into either a concrete context, or a
// selection-set that has typename. But in practice for named fragments
// it's convenient for various reasons (mocking, and simplifying things
// when we "inline" single-selection types), and it's not hard, so we do it
// anyway.
// TODO(benkraft): We could omit __typename if you asked for
// `# @genqlient(struct: true)`.
observers.OnField(func(_ *validator.Walker, field *ast.Field) {
// We are interested in a field from the query like
// field { subField ... }
// where the schema looks like
// type ... { # or interface/union
// field: FieldType # or [FieldType!]! etc.
// }
// interface FieldType { # or union
// subField: ...
// }
// If FieldType is an interface/union, and none of the subFields is
// __typename, we want to change the query to
// field { __typename subField ... }
handleSelection(
g.schema.Types[field.Definition.Type.Name()],
&field.SelectionSet)
})
observers.OnFragment(func(_ *validator.Walker, fragment *ast.FragmentDefinition) {
// We are interested in a fragment like
// fragment F on Type { subField ... }
// where the schema looks like
// interface Type { # or union
// subField: ...
// }
// If FieldType is an interface/union, and none of the subFields is
// __typename, we want to change the fragment to
// fragment F on Type { __typename subField ... }
handleSelection(fragment.Definition, &fragment.SelectionSet)
})
validator.Walk(g.schema, doc, &observers)
}
Expand Down
27 changes: 21 additions & 6 deletions ...ata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ...a/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"operations": [
{
"operationName": "ComplexNamedFragments",
"query": "\nquery ComplexNamedFragments {\n\t... on Query {\n\t\t... QueryFragment\n\t}\n}\nfragment QueryFragment on Query {\n\t... InnerQueryFragment\n}\nfragment InnerQueryFragment on Query {\n\trandomItem {\n\t\t__typename\n\t\tid\n\t\tname\n\t\t... VideoFields\n\t\t... ContentFields\n\t}\n\trandomLeaf {\n\t\t__typename\n\t\t... VideoFields\n\t\t... MoreVideoFields\n\t\t... ContentFields\n\t}\n\totherLeaf: randomLeaf {\n\t\t__typename\n\t\t... on Video {\n\t\t\t... MoreVideoFields\n\t\t\t... ContentFields\n\t\t}\n\t\t... ContentFields\n\t}\n}\nfragment VideoFields on Video {\n\tid\n\tname\n\turl\n\tduration\n\tthumbnail {\n\t\tid\n\t}\n\t... ContentFields\n}\nfragment ContentFields on Content {\n\tname\n\turl\n}\nfragment MoreVideoFields on Video {\n\tid\n\tparent {\n\t\tname\n\t\turl\n\t\t... ContentFields\n\t\tchildren {\n\t\t\t__typename\n\t\t\t... VideoFields\n\t\t}\n\t}\n}\n",
"query": "\nquery ComplexNamedFragments {\n\t... on Query {\n\t\t... QueryFragment\n\t}\n}\nfragment QueryFragment on Query {\n\t... InnerQueryFragment\n}\nfragment InnerQueryFragment on Query {\n\trandomItem {\n\t\t__typename\n\t\tid\n\t\tname\n\t\t... VideoFields\n\t\t... ContentFields\n\t}\n\trandomLeaf {\n\t\t__typename\n\t\t... VideoFields\n\t\t... MoreVideoFields\n\t\t... ContentFields\n\t}\n\totherLeaf: randomLeaf {\n\t\t__typename\n\t\t... on Video {\n\t\t\t... MoreVideoFields\n\t\t\t... ContentFields\n\t\t}\n\t\t... ContentFields\n\t}\n}\nfragment VideoFields on Video {\n\tid\n\tname\n\turl\n\tduration\n\tthumbnail {\n\t\tid\n\t}\n\t... ContentFields\n}\nfragment ContentFields on Content {\n\t__typename\n\tname\n\turl\n}\nfragment MoreVideoFields on Video {\n\tid\n\tparent {\n\t\tname\n\t\turl\n\t\t... ContentFields\n\t\tchildren {\n\t\t\t__typename\n\t\t\t... VideoFields\n\t\t}\n\t}\n}\n",
"sourceLocation": "testdata/queries/ComplexNamedFragments.graphql"
}
]
Expand Down

0 comments on commit bdc8720

Please sign in to comment.