Commit Graph
21 Commits
Author SHA1 Message Date
Ben KraftandGitHub 222d6f191a Document and improve support for binding non-scalars to a specific type (#69)
## Summary:
We had this setting called "scalars", which said: bind this GraphQL type
to this Go type, rather than the one you would normally use.  It's
called that because it's most useful for custom scalars, where "the one
you would normally use" is "error: unknown scalar".  But nothing ever
stopped you from using it for a non-scalar type.  I was planning on
removing this functionality, because it's sort of a rough edge, but a
discussion with Craig found some good use cases, so instead, in this
commit, I document it better and add some slightly nicer ways to specify
it.

Specifically, here are a few potential non-scalar use cases:
- bind a GraphQL enum to a nonstandard type (or even `string`)
- bind an input type to some type that has exactly the fields you want;
  this acts as a sort of workaround for issues #14 and #44
- bind an object type to your own struct, so as to add methods to it
  (this is the use case Craig raised)
- bind an object type to your own struct, so as to share it between
  multiple queries (I believe named fragments will address this case
  better, but it doesn't hurt to have options)
- bind a GraphQL list type to a non-slice type in Go (presumably one
  with an UnmarshalJSON method), or any other different structure
The latter three cases still have the sharp edge I was originally
worried about, which is that nothing guarantees that the fields you
request in the query are the ones the type expects to get.  But I think
it's worth having the option, with appropriate disclaimers.

The main change to help support that better is that you can now specify
the type inline in the query, as an alternative to specifying it in the
config file; this means you might map a given object to a given struct,
but only in some cases, and when you do you have a chance to look at the
list of fields you're requesting.

Additionally, I renamed the config field from "scalars" to "bindings"
(but mentioned it in a few places where you might go looking for how to
map scalars, most importantly the error message you get for an unknown
(custom) scalar).  While I was making a breaking change, I also changed
it to be a `map[string]<struct>` instead of a `map[string]string`,
because I expect to add more fields soon, e.g. to handle issue #38.

Finally, since the feature is now intended/documented, I added some
tests, although it's honestly quite simple on the genqlient side.

## Test plan:
make tesc


Author: benjaminjkraft

Reviewers: csilvers, aberkan, dnerdy, MiguelCastillo

Required Reviewers: 

Approved by: csilvers

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/69
2021-08-27 15:57:10 -07:00
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
2021-08-25 12:02:18 -07:00
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
2021-08-25 11:57:24 -07:00
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
2021-08-25 11:51:26 -07:00
Ben KraftandGitHub 700392315a Enable golangci-lint (#49)
## Summary:
It would be nice to have some linting beyond `go vet`!  Now we do.  I
started by copying the config from Khan/webapp.  I did remove a couple
of staticcheck checks that I didn't feel were useful.  (Note also that
exportloopref is the replacement for scopelint in newer golangci-lint.)

Included are all the needed lint fixes; most are stylistic but the
changes in the example are a (minor) bugfix.

Fixes #22.
Issue: https://github.com/Khan/genqlient/issues/22

## 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/49
2021-08-20 10:39:12 -07:00
Ben Kraft dfc9f02efc Actually build the generated code in tests
This revealed one bug already (a redux of #43)!  And will hopefully help
more as unmarshalers get more complicated (thanks to
interfaces/fragments).
2021-06-03 15:48:05 -07:00
Ben Kraft 1f442da041 Switch to cupaloy for snapshots
Slightly uglier filenames, but less code and free diffing!  Approved in
ADR-466 for Khan use.  Fixes #23.
2021-06-03 12:08:56 -07:00
Ben Kraft 3c11f07e62 Make test ID type actually exist
This way I can start to test that the code compiles.
2021-06-03 11:36:45 -07:00
Ben Kraft 2272ad8a12 allow map[string]interface{}, and other such things, as scalars 2021-04-21 11:32:49 -07:00
Ben Kraft 2c087c7821 big overhaul of error-formatting, to get positions more consistently
fixes #2
2021-04-15 17:01:46 -07:00
Ben Kraft f8f81dad7b snapshots for errors 2021-04-15 15:42:05 -07:00
Ben Kraft ddf4b03350 allow interface{} as a scalar 2021-04-15 11:50:52 -07:00
Ben Kraft e597cac74c add option to force using a pointer 2021-04-12 16:01:21 -07:00
Ben Kraft b4e8316c6a add support for custom scalars -- mainly adding proper import machinery 2021-04-08 13:07:31 -07:00
Ben Kraft 334c186944 error if you try to use features that aren't done (fragments/interfaces) 2021-04-02 16:19:50 -07:00
Ben Kraft 9fe8f09e8c more miscellaneous fixes while integrating into webapp 2021-04-01 15:04:36 -07:00
Ben Kraft fbf00f3bef add option to export all operations 2021-04-01 13:11:23 -07:00
Ben Kraft eae68e43c9 allow globbing, and extract queries from .go files 2021-04-01 12:34:51 -07:00
Ben Kraft ec6681e715 allow multiple queries-files 2021-04-01 12:29:24 -07:00
Ben Kraft fed38e4f55 redo type naming 2021-03-19 18:40:58 -07:00
Ben Kraft 036d39622d move everything into snapshot-based testing; update it a bit 2020-07-16 13:42:58 -07:00