bf266d874dfeba49d676bcc8334f60574cbc0b2e
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ben KraftandGitHub
|
8f9d1cf792 |
Add support for interfaces, part 4: getter methods (#57)
## Summary:
Right now, if you make a query like `{ myInterface { field } }`, you
have to type-switch on all the possible implementations of `myInterface`
to get at `field`. Now, we generate getter-methods (e.g. `GetField`),
to make that access easier. Of course this only applies to shared
fields (which for now are the only ones, but once we support fragments
will no longer be).
This also includes a small change to the way we generate type-names for
interfaces: we no longer include the name of the concrete type in the
interface we propagate forward, so we generate
`MyInterfaceMyFieldMyType`, not `MyInterfaceMyImplMyFieldMyType`, in the
case where you have an interface `MyInterface` implemented by `MyImpl`
(and maybe other types) with field `myField: MyType`. This is necessary
so the getter method returns a well-defined type, and also probably
convenient for calling code. It will have to get a little bit more
complicated once we support fragments, where you could have two
implementing types with identically-named fields of different types, but
I think it'll be easiest to figure out how to deal with that when
implementing fragments.
While I was in the area, I added to the interface doc-comment a list of
the implementations. (In GraphQL, we're guaranteed to know them all
assuming our schema is up to date.)
Issue: https://github.com/Khan/genqlient/issues/8
## Test plan:
make check
Author: benjaminjkraft
Reviewers: benjaminjkraft, 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/57
|
||
|
Ben KraftandGitHub
|
1e87553788 |
Add support for interfaces, part 2: list-of-interface (#54)
## 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 |
||
|
Ben KraftandGitHub
|
4c38cb7759 |
Add support for interfaces, part 1: the simplest cases (#52)
## 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 |
||
|
Ben KraftandGitHub
|
8815d0991c |
Big refactor to separate operation-traversal from code-generation (#51)
## Summary: I've felt for a while that types.go is way too confusing, and as I started to implement some of the more complex cases of generating interface-types, the cracks were really starting to show. Luckily, I also finally realized how to fix it: we need to separate the process of traversing the GraphQL operation and schema to decide what types to generate from the process of actually generating those types. This requires an extra set of intermediate data structures, but I think it makes things quite a lot easier to understand -- and, importantly, it means that the code-generation doesn't need to go in the order we traverse the query/schema. So in this commit, I did that huge refactor. It's probably best to just review types.go and traverse.go as if they were new; the old code was quite hard to understand and the new code will hopefully make a lot more sense. (And to that end, review comments about what could be organized better or needs more documentation are very much in order, even for code that is mostly unchanged.) This does introduce one bug, sort of, which is that rather than generating broken code for list-of-interface fields, we generate no code at all. (A TODO in unmarshal.go describes why.) I'll fix this when I add support for those fields. (It's all behind the AllowBrokenFeatures flag, anyway.) Otherwise, the only changes to generated code are that a few methods are ordered differently, because we now generate the implements-interface methods with the interface, rather than the implementations, as it's much simpler that way. (In GraphQL, unlike Go, we know the list of all possible implementations of each interface, so this is possible.) ## Test plan: golangci-lint run ./... && go test ./... Author: benjaminjkraft Reviewers: dnerdy, benjaminjkraft, aberkan, csilvers, MiguelCastillo Required Reviewers: Approved by: dnerdy Checks: ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Test (1.13), ✅ 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/51 |