-
Notifications
You must be signed in to change notification settings - Fork 114
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
In this commit I add support for inline fragments (`... on MyType { fields }`) to genqlient. This will make interfaces a lot more useful! In future commits I'll add named fragments, for which we'll generate slightly different types, as discussed in DESIGN.md. In general, implementing the flattening approach described in DESIGN.md was... surprisingly easy. All we have to do is recurse on applicable fragments when generating our selection-set. The refactor to selection-set handling this encouraged was, I think, quite beneficial. It did reveal two tricky pre-existing issues. One issue is that GraphQL allows for duplicate selections, as long as they match. (In practice, this is only useful in the context of fragments, although GraphQL allows it even without.) I decided to handle the simple case (duplicate leaf fields; we just deduplicate) but leave to the future the complex cases where we need to merge different sub-selections (now #64). For now we just forbid that; we can see how much it comes up. The other issue is that we are generating type-names incorrectly for interface types; I had intended to do `MyInterfaceMyFieldMyType` for shared fields and `MyImplMyFieldMyType` for non-shared ones, but instead I did `MyFieldMyType`, which is inconsistent already and can result in conflicts in the presence of fragments. I'm going to fix this in a separate commit, though, because it's going to require some refactoring and is irrelevant to the main logic of this commit; I left some TODOs in the tests related to this. Issue: #8 Test plan: make check Reviewers: marksandstrom, adam, miguel
- Loading branch information
1 parent
e99dced
commit ae5c33d
Showing
19 changed files
with
2,267 additions
and
66 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package errors | ||
|
||
const _ = `# @genqlient | ||
query { | ||
myField { | ||
subField { subSubField1 subSubField2 } | ||
... on OnePossibleConcreteType { | ||
subField { subSubField3 subSubField4 } | ||
} | ||
} | ||
} | ||
` |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
query { | ||
myField { | ||
subField { subSubField1 subSubField2 } | ||
... on OnePossibleConcreteType { | ||
subField { subSubField3 subSubField4 } | ||
} | ||
} | ||
} |
18 changes: 18 additions & 0 deletions
18
generate/testdata/errors/ConflictingSelections.schema.graphql
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
type Query { | ||
myField: InterfaceType | ||
} | ||
|
||
interface InterfaceType { | ||
subField: SubFieldType | ||
} | ||
|
||
type OnePossibleConcreteType implements InterfaceType { | ||
subField: SubFieldType | ||
} | ||
|
||
type SubFieldType { | ||
subSubField1: String! | ||
subSubField2: String! | ||
subSubField3: String! | ||
subSubField4: String! | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
# We test all the spread cases from DESIGN.md, see there for more context on | ||
# each, as well as various other nonsense. But for abstract-in-abstract | ||
# spreads, we can't test cases (4b) and (4c), where I implements J or vice | ||
# versa, because gqlparser doesn't support interfaces that implement other | ||
# interfaces yet. | ||
query ComplexInlineFragments { | ||
root { | ||
id | ||
... on Topic { schoolGrade } # (1) object spread in object scope | ||
... on Content { name } # (3) abstract spread in object scope | ||
} | ||
randomItem { | ||
id | ||
... on Article { text } # (2) object spread in abstract scope | ||
... on Content { name } # (4a) abstract spread in abstract scope, I == J | ||
... on HasDuration { duration } # (4d) abstract spread in abstract scope, neither implements the other | ||
} | ||
repeatedStuff: randomItem { | ||
id | ||
id | ||
url | ||
otherId: id | ||
... on Article { | ||
name | ||
text | ||
otherName: name | ||
} | ||
... on Content { | ||
id | ||
name | ||
otherName: name | ||
} | ||
... on HasDuration { duration } | ||
} | ||
conflictingStuff: randomItem { | ||
# These two have different types! Naming gets complicated. Note GraphQL | ||
# says [1] that you can only have such naming conflicts when the fields are | ||
# both on object-typed spreads (reasonable, so they can never collide) and | ||
# they are of "shapes that can be merged", e.g. both nullable objects, | ||
# which seems very strange to me but is the most interesting case for us | ||
# anyway (since where we could have trouble is naming the result types). | ||
# [1] https://spec.graphql.org/draft/#SameResponseShape() | ||
# TODO(benkraft): This actually generates the wrong thing right now (the | ||
# two thumbnail types get the same name, and one clobbers the other). Fix | ||
# in a follow-up commit. | ||
... on Article { thumbnail { id thumbnailUrl } } | ||
... on Video { thumbnail { id timestampSec } } | ||
} | ||
nestedStuff: randomItem { | ||
... on Topic { | ||
children { | ||
id | ||
... on Article { | ||
text | ||
parent { | ||
... on Content { | ||
name | ||
parent { | ||
... on Topic { | ||
children { id name } | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
query SimpleInlineFragment { | ||
randomItem { | ||
id | ||
name | ||
... on Article { text } | ||
... on Video { duration } | ||
} | ||
} |
Oops, something went wrong.