## Summary:
We guarantee that we never return a nil response, so you can safely do
```
resp, err := myQuery(...)
return resp.Field.SubField, err
```
And furthermore, if the error was a GraphQL error, `resp` may even be
nonzero; other, non-failing fields may be set. (This depends on the
server, of course.) But we weren't testing either of those. Now we do.
## Test plan:
make check
Author: benjaminjkraft
Reviewers: jvoll, aberkan, dnerdy, MiguelCastillo, mahtabsabet
Required Reviewers:
Approved By: jvoll
Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint
Pull Request URL: https://github.com/Khan/genqlient/pull/83
## Summary:
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: https://github.com/Khan/genqlient/issues/8
## Test plan:
make check
Author: benjaminjkraft
Reviewers: dnerdy, aberkan, MiguelCastillo
Required Reviewers:
Approved by: dnerdy
Checks: ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13), ✅ Lint, ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/65
## Summary:
In this commit I remove one of the limitations of our support for
interfaces, from #52, by adding support for list-of-interface fields.
This was surprisingly complex! The issue is that, as before, it's the
containing type that has to do all the glue work -- and it's that glue
work that is complicated by list-of-interface fields.
All in all, it's not that much new code, and by far the hard part is
just 20 lines in the UnmarshalJSON template (which come with almost
twice as many lines of comments to explain them). It may be easiest to
start by reading some of the generated code, and then read the template.
I also added support for such fields with `pointer: true` specified,
such that the type is `[][]...[]*MyInterface`, although I don't know why
you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`;
that would require a way to specify it (see #16) but also add some extra
complexity (as we'd have to actually walk the type-unwrap chain
properly, instead of just counting the number of slices and whether
there's a pointer).
Issue: https://github.com/Khan/genqlient/issues/8
## Test plan:
make check
Author: benjaminjkraft
Reviewers: dnerdy, benjaminjkraft, aberkan, csilvers, MiguelCastillo
Required Reviewers:
Approved by: dnerdy
Checks: ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13), ⌛ Lint, ⌛ Lint, ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13)
Pull request URL: https://github.com/Khan/genqlient/pull/54
## Summary:
In this commit I begin the journey to add the long-awaited support for
interfaces (part of #8). Well, it's not the beginning: I already had
some half-written broken code around. But it's the first fully
functional support, and especially, the first *tested* support; it's
probably best to review the nontrivially-changed code as if it were new.
Conceptually, the code so far is pretty simple: we generate an interface
type, and the implementations. (That code is in fact mostly unchanged.)
The complexity comes in because encoding/json doesn't know how to
unmarshal that. So we have to add an UnmarshalJSON method, which
actually has to be on the types with interface-type fields, that knows
how. I factored it into two methods, such that that UnmarshalJSON
method is just glue, and then there's a separate function, corresponding
to each interface-type, that actually does all the work. (If only one
could just write it as an actual method!) The method uses the same
trick suggested to me by a few others in another context to deserialize
all but one field, then handle that field specially, which is discussed
in the code.
This still has some limitations, which will be lifted in future commits:
- it doesn't allow for list-of-interface fields
- it requires that you manually ask for `__typename`
- it doesn't support fragments, i.e. you can only query for interface
fields, not concrete-type-specific ones
But it works, even in integration tests, which is progress!
As a part of this, I added a proper config option for the "allow broken
features" flag, since I need to be able to set it from the integration
tests which are in a separate package (and actually shell out via `go
generate`). I also renamed what was to be the first case
(InterfaceNoFragments), and replaced it with a further-simplified
version (avoiding list-of-interface fields.
[1] https://github.com/benjaminjkraft/notes/blob/master/go-json-interfaces.md
Issue: https://github.com/Khan/genqlient/issues/8
## Test plan:
make tesc
Author: benjaminjkraft
Reviewers: dnerdy, benjaminjkraft, aberkan, csilvers, MiguelCastillo
Required Reviewers:
Approved by: dnerdy
Checks: ⌛ Test (1.17), ✅ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13), ✅ Lint, ✅ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/52
## Summary:
We have lots of tests covering codegen, but not a lot that actually run
the code. For things where all we do is generate types, that's (mostly)
fine (especially now that we actually build the code), but as we
generate more nontrivial non-type code we need to actually run it.
So I wrote some integration tests that spin up a little gqlgen
server, and make calls to it; we can add more over time especially as
the JSON marshalling logic gets complex (to support fragments).
They're more work to write than the snapshot tests, but of course they
can test a lot more.
In addition to gqlgen, I pulled in testify assert/require, because I
really wanted to be able to use assert.Equal and such for these. I
didn't bother converting existing tests, although I assume they will
become useful elsewhere in time. Both gqlgen and testify are of course
only used in tests.
Fixes#21 and #24.
Issue: https://github.com/Khan/genqlient/issues/21
Issue: https://github.com/Khan/genqlient/issues/24
## Test plan:
make check
Author: benjaminjkraft
Reviewers: aberkan, dnerdy, benjaminjkraft, csilvers, MiguelCastillo
Required Reviewers:
Approved by: aberkan, dnerdy
Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ Lint
Pull request URL: https://github.com/Khan/genqlient/pull/50
