Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for "flattening" fragment-spreads #121

Merged
merged 2 commits into from
Sep 30, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ When releasing a new version:
### New features:

- genqlient's types are now safe to JSON-marshal, which can be useful for putting them in a cache, for example. See the [docs](FAQ.md#-let-me-json-marshal-my-response-objects) for details.
- The new `flatten` option in the `# @genqlient` directive allows for a simpler form of type-sharing using fragment spreads. See the [docs](FAQ.md#-shared-types-between-different-parts-of-the-query) for details.

### Bug fixes:

Expand Down
22 changes: 18 additions & 4 deletions docs/FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,9 @@ type GetMonopolyPlayersGameWinnerUser struct {
// (others similarly)
```

But maybe you wanted to be able to pass all those users to a shared function (defined in your code), say `FormatUser(user ???) string`. That's no good; you need to put three different types as the `???`. genqlient has two ways to deal with this.
But maybe you wanted to be able to pass all those users to a shared function (defined in your code), say `FormatUser(user ???) string`. That's no good; you need to put three different types as the `???`. genqlient has several ways to deal with this.

One option -- the GraphQL Way, perhaps -- is to use fragments. You'd write your query like:
**Fragments:** One option -- the GraphQL Way, perhaps -- is to use fragments. You'd write your query like:

```graphql
fragment MonopolyUser on User {
Expand Down Expand Up @@ -319,7 +319,21 @@ query GetMonopolyPlayers {

and you can even spread the fragment into interface types. It also avoids having to list the fields several times.

Alternately, if you always want exactly the same fields, you can use the simpler but more restrictive genqlient option `typename`:
**Fragments, flattened:** the field `Winner`, above, has type `GetMonopolyPlayersGameWinnerUser` which just wraps `MonopolyUser`. If we don't want to add any other fields, that's unnecessary! Instead, we could do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "Winner" => "winner"

Also, the "winner" field directly above this section does have another field "winCount", though I understand this is referencing the query above that.

```
query GetMonopolyPlayers {
game {
# @genqlient(flatten: true)
winner {
...MonopolyUser
}
# (etc.)
}
}
```
and genqlient will skip the indirection and give the field `Winner` type `MonopolyUser` directly. This is often much more convenient if you put all the fields in the fragment, like the first query did. See the [options documentation](genqlient_directive.graphql) for more details.

**Type names:** Finally, if you always want exactly the same fields, you can use the simpler but more restrictive genqlient option `typename`:

```graphql
query GetMonopolyPlayers {
Expand Down Expand Up @@ -351,7 +365,7 @@ type User struct {

In this case, genqlient will validate that each type given the name `User` has the exact same fields; see the [full documentation](genqlient_directive.graphql) for details.

Note that it's also possible to use the `bindings` option (see [`genqlient.yaml` documentation](genqlient.yaml)) for a similar purpose, but this is not recommended as it typically requires more work for less gain.
**Bindings:** It's also possible to use the `bindings` option (see [`genqlient.yaml` documentation](genqlient.yaml)) for a similar purpose, but this is not recommended as it typically requires more work for less gain.

### … documentation on the output types?

Expand Down
36 changes: 35 additions & 1 deletion docs/genqlient_directive.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,40 @@ directive genqlient(
# fragment, you'll have to remove this option, and the types will change.
struct: Boolean

# If set, this field's selection must contain a single fragment-spread; we'll
# use the type of that fragment-spread as the type of the field.
#
# For example, given a query like
# query MyQuery {
# myField {
# ...MyFragment
# }
# }
# by default genqlient will generate these types:
# type MyQueryResponse struct {
# MyField MyQueryMyFieldMyType
# }
# type MyQueryMyFieldMyType struct {
# MyFragment
# }
# If we instead do:
# query MyQuery {
# # @genqlient(flatten: true)
# myField {
# ...MyFragment
# }
# }
# genqlient will simplify things:
# type MyQueryResponse struct {
# MyField MyFragment
# }
#
# This is only applicable to fields whose selection is a single
# fragment-spread, such that the field-type implements the fragment-type
# (i.e. we can't do this if MyFragment is on one implementation of the type
# of MyField; what if we got back the other type?).
flatten: Boolean

# If set, this argument or field will use the given Go type instead of a
# genqlient-generated type.
#
Expand Down Expand Up @@ -128,7 +162,7 @@ directive genqlient(
# if your type-name conflicts with an autogenerated one (again, unless they
# request the exact same fields). They must even have the fields in the
# same order. Fragments are often easier to use (see the discussion of
# code-sharing in FAQ.md).
# code-sharing in FAQ.md, and the "flatten" option above).
#
# Note that unlike most directives, if applied to the entire operation,
# typename affects the overall response type, rather than being propagated
Expand Down
34 changes: 34 additions & 0 deletions generate/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ func (g *generator) convertOperation(
return nil, err
}

// It's not common to use a fragment-spread for the whole query, but you
// can if you want two queries to return the same type!
if queryOptions.GetFlatten() {
i, err := validateFlattenOption(baseType, operation.SelectionSet, operation.Position)
if err == nil {
return fields[i].GoType, nil
}
}

goType := &goStructType{
GoName: name,
descriptionInfo: descriptionInfo{
Expand Down Expand Up @@ -340,6 +349,17 @@ func (g *generator) convertDefinition(
if err != nil {
return nil, err
}
if options.GetFlatten() {
// As with struct, flatten only applies if valid, important if you
// applied it to the whole query.
// TODO(benkraft): This is a slightly fragile way to do this;
// figure out a good way to do it before/while constructing the
// fields, rather than after.
i, err := validateFlattenOption(def, selectionSet, pos)
if err == nil {
return fields[i].GoType, nil
}
}

goType := &goStructType{
GoName: name,
Expand Down Expand Up @@ -406,6 +426,14 @@ func (g *generator) convertDefinition(
if err != nil {
return nil, err
}
// Flatten can only flatten if there is only one field (plus perhaps
// __typename), and it's shared.
if options.GetFlatten() {
i, err := validateFlattenOption(def, selectionSet, pos)
if err == nil {
return sharedFields[i].GoType, nil
}
}

implementationTypes := g.schema.GetPossibleTypes(def)
goType := &goInterfaceType{
Expand Down Expand Up @@ -705,6 +733,12 @@ func (g *generator) convertNamedFragment(fragment *ast.FragmentDefinition) (goTy
if err != nil {
return nil, err
}
if directive.GetFlatten() {
i, err := validateFlattenOption(typ, fragment.SelectionSet, fragment.Position)
if err == nil {
return fields[i].GoType, nil
}
}

switch typ.Kind {
case ast.Object:
Expand Down
83 changes: 79 additions & 4 deletions generate/genqlient_directive.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type genqlientDirective struct {
Omitempty *bool
Pointer *bool
Struct *bool
Flatten *bool
Bind string
TypeName string
}
Expand All @@ -28,6 +29,7 @@ func newGenqlientDirective(pos *ast.Position) *genqlientDirective {
func (dir *genqlientDirective) GetOmitempty() bool { return dir.Omitempty != nil && *dir.Omitempty }
func (dir *genqlientDirective) GetPointer() bool { return dir.Pointer != nil && *dir.Pointer }
func (dir *genqlientDirective) GetStruct() bool { return dir.Struct != nil && *dir.Struct }
func (dir *genqlientDirective) GetFlatten() bool { return dir.Flatten != nil && *dir.Flatten }

func setBool(optionName string, dst **bool, v *ast.Value, pos *ast.Position) error {
if *dst != nil {
Expand Down Expand Up @@ -85,6 +87,8 @@ func (dir *genqlientDirective) add(graphQLDirective *ast.Directive, pos *ast.Pos
err = setBool("pointer", &dir.Pointer, arg.Value, pos)
case "struct":
err = setBool("struct", &dir.Struct, arg.Value, pos)
case "flatten":
err = setBool("flatten", &dir.Flatten, arg.Value, pos)
case "bind":
err = setString("bind", &dir.Bind, arg.Value, pos)
case "typename":
Expand Down Expand Up @@ -116,7 +120,13 @@ func (dir *genqlientDirective) validate(node interface{}, schema *ast.Schema) er
}

if dir.Struct != nil {
return errorf(dir.pos, "struct is only applicable to fields")
return errorf(dir.pos, "struct is only applicable to fields, not frragment-definitions")
}

if dir.Flatten != nil {
if _, err := validateFlattenOption(node.Definition, node.SelectionSet, dir.pos); err != nil {
return err
}
}

// Like operations, anything else will just apply to the entire
Expand All @@ -128,22 +138,32 @@ func (dir *genqlientDirective) validate(node interface{}, schema *ast.Schema) er
}

if dir.Struct != nil {
return errorf(dir.pos, "struct is only applicable to fields")
return errorf(dir.pos, "struct is only applicable to fields, not variable-definitions")
}

if dir.Flatten != nil {
return errorf(dir.pos, "flatten is only applicable to fields, not variable-definitions")
}

return nil
case *ast.Field:
if dir.Omitempty != nil {
return errorf(dir.pos, "omitempty is not applicable to fields")
return errorf(dir.pos, "omitempty is not applicable to variables, not fields")
}

typ := schema.Types[node.Definition.Type.Name()]
if dir.Struct != nil {
typ := schema.Types[node.Definition.Type.Name()]
if err := validateStructOption(typ, node.SelectionSet, dir.pos); err != nil {
return err
}
}

if dir.Flatten != nil {
if _, err := validateFlattenOption(typ, node.SelectionSet, dir.pos); err != nil {
return err
}
}

return nil
default:
return errorf(dir.pos, "invalid @genqlient directive location: %T", node)
Expand Down Expand Up @@ -178,6 +198,58 @@ func validateStructOption(
return nil
}

func validateFlattenOption(
typ *ast.Definition,
selectionSet ast.SelectionSet,
pos *ast.Position,
) (index int, err error) {
index = -1
if len(selectionSet) == 0 {
return -1, errorf(pos, "flatten is not allowed for leaf fields")
}

for i, selection := range selectionSet {
switch selection := selection.(type) {
case *ast.Field:
// If the field is auto-added __typename, ignore it for flattening
// purposes.
if selection.Name == "__typename" && selection.Position == nil {
continue
}
// Type-wise, it's no harder to implement flatten for fields, but
// it requires new logic in UnmarshalJSON. We can add that if it
// proves useful relative to its complexity.
return -1, errorf(pos, "flatten is not yet supported for fields (only fragment spreads)")

case *ast.InlineFragment:
// Inline fragments aren't allowed. In principle there's nothing
// stopping us from allowing them (under the same type-match
// conditions as fragment spreads), but there's little value to it.
return -1, errorf(pos, "flatten is not allowed for selections with inline fragments")

case *ast.FragmentSpread:
if index != -1 {
return -1, errorf(pos, "flatten is not allowed for fields with multiple selections")
} else if !fragmentMatches(typ, selection.Definition.Definition) {
// We don't let you flatten
// field { # type: FieldType
// ...Fragment # type: FragmentType
// }
// unless FragmentType implements FieldType, because otherwise
// what do we do if we get back a type that doesn't implement
// FragmentType?
return -1, errorf(pos,
"flatten is not allowed for fields with fragment-spreads "+
"unless the field-type implements the fragment-type; "+
"field-type %s does not implement fragment-type %s",
typ.Name, selection.Definition.Definition.Name)
}
index = i
}
}
return index, nil
}

// merge joins the directive applied to this node (the argument) and the one
// applied to the entire operation (the receiver) and returns a new
// directive-object representing the options to apply to this node (where in
Expand All @@ -193,6 +265,9 @@ func (dir *genqlientDirective) merge(other *genqlientDirective) *genqlientDirect
if other.Struct != nil {
retval.Struct = other.Struct
}
if other.Flatten != nil {
retval.Flatten = other.Flatten
}
if other.Bind != "" {
retval.Bind = other.Bind
}
Expand Down
6 changes: 6 additions & 0 deletions generate/testdata/errors/FlattenField.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
query FlattenField {
# @genqlient(flatten: true)
t {
f
}
}
2 changes: 2 additions & 0 deletions generate/testdata/errors/FlattenField.schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
type Query { t: T }
type T { f: String }
7 changes: 7 additions & 0 deletions generate/testdata/errors/FlattenImplementation.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fragment F on T { f }
query FlattenImplementation {
# @genqlient(flatten: true)
i {
...F
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type Query { i: I }
interface I { f: String }
type T implements I { f: String }
42 changes: 42 additions & 0 deletions generate/testdata/queries/Flatten.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# @genqlient(flatten: true)
fragment QueryFragment on Query {
...InnerQueryFragment
}

fragment InnerQueryFragment on Query {
# @genqlient(flatten: true)
randomVideo {
...VideoFields
}
# @genqlient(flatten: true)
randomItem {
...ContentFields
}
# @genqlient(flatten: true)
otherVideo: randomVideo {
...ContentFields
}
}

fragment VideoFields on Video {
id
parent {
# @genqlient(flatten: true)
videoChildren {
...ChildVideoFields
}
}
}

fragment ChildVideoFields on Video {
id name
}

fragment ContentFields on Content {
name url
}

# @genqlient(flatten: true)
query ComplexNamedFragments {
...QueryFragment
}
2 changes: 2 additions & 0 deletions generate/testdata/queries/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ type Topic implements Content {
parent: Topic
url: String!
children: [Content!]!
videoChildren: [Video!]!
schoolGrade: String
}

Expand Down Expand Up @@ -162,6 +163,7 @@ type Query {
root: Topic!
randomItem: Content!
randomLeaf: LeafContent!
randomVideo: Video!
convert(dt: DateTime!, tz: String): DateTime!
maybeConvert(dt: DateTime, tz: String): DateTime
getJunk: Junk
Expand Down
Loading