talksik/genqlient
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

clean up various TODOs and comments

Browse Source
This commit is contained in:
Ben Kraft
2021-03-22 18:11:51 -07:00
parent 463e3ed319
commit a42c9b8166
12 changed files with 63 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/go.yml
+1 -1
View File
@@ -12,7 +12,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
go: [ '1.13', '1.14' ]
go: [ '1.13', '1.14', '1.15', '1.16' ]
steps:
- name: Set up Go
DESIGN.md
+2
View File
@@ -97,6 +97,8 @@ In other tools:
- gqlgen doesn't have this problem, because on the server each GraphQL type maps to a unique Go type; and to the extent that different queries need different fields this is handled at the level of which fields are serialized or which resolvers are called.
- shurcooL/graphql allows either way.
A side advantage of an approach that prefixes the query name to all the type names, for us, is that it means that the caller can decide in a very simple way whether to make the name exported, and avoids conflicts between queries.
**Decision:** In general, it seems like even in languages with better ergonomics for unnamed types, Apollo's approach is somewhat reasonable. And unnamed types will get really hairy for large queries in Go. On some level, even if the naming scheme is bad, it won't be as bad as unnamed types -- if you don't need to refer to the intermediate type, you don't care, and if you do, it's better than a giant inline struct. (But it's hard to change later without a flag as existing code may depend on those types.)
We'll do something similar to Apollo's naming scheme. Specifically:
Makefile
+3
View File
@@ -2,4 +2,7 @@ example:
go generate ./...
go run ./example/cmd/example/main.go csilvers
check:
go test ./...
.PHONY: example
README.md
+4 -2
View File
@@ -42,7 +42,7 @@ func getViewer(ctx context.Context, client *graphql.Client) (*getViewerResponse,
// your code (error handling omitted for brevity)
graphqlClient := graphql.NewClient("https://example.com/graphql", http.DefaultClient)
viewerResp, _ := getViewer(context.Background(), graphqlClient)
fmt.Println("you are", *viewerResp.Viewer.MyName)
fmt.Println("you are", viewerResp.Viewer.MyName)
//go:generate go run github.com/Khan/genql
```
@@ -80,11 +80,13 @@ Config options:
- send hash rather than full query
- whether names should be exported
- default handling for optional fields (pointers, HasFoo, etc.)
- response/function-name format (e.g. force exported/unexported, change "Response" suffix, etc.)
- generate mocks?
Other:
- (*) error-checking/validation/etc. everywhere
- (*) a name that's more clearly distinct from other libraries out there and conveys what this does
- (+) more tests
- (+) documentation
- custom scalar types
- custom scalar types (or custom mappings for standard scalars, if you want a special ID type say)
- allow mapping a custom type to a particular val (if you want to use a named type for some string, say)
example/generated.go
+10 -10
View File
@@ -8,24 +8,24 @@ import (
"github.com/Khan/genql/graphql"
)
type GetUserResponse struct {
User GetUserUser `json:"user"`
type getUserResponse struct {
User getUserUser `json:"user"`
}
type GetUserUser struct {
type getUserUser struct {
TheirName string `json:"theirName"`
}
type GetViewerResponse struct {
Viewer GetViewerViewerUser `json:"viewer"`
type getViewerResponse struct {
Viewer getViewerViewerUser `json:"viewer"`
}
type GetViewerViewerUser struct {
type getViewerViewerUser struct {
MyName string
}
func getViewer(ctx context.Context, client *graphql.Client) (*GetViewerResponse, error) {
var retval GetViewerResponse
func getViewer(ctx context.Context, client *graphql.Client) (*getViewerResponse, error) {
var retval getViewerResponse
err := client.MakeRequest(ctx, `
query getViewer {
viewer {
@@ -37,12 +37,12 @@ query getViewer {
}
// getUser gets the given user's name from their username.
func getUser(ctx context.Context, client *graphql.Client, login string) (*GetUserResponse, error) {
func getUser(ctx context.Context, client *graphql.Client, login string) (*getUserResponse, error) {
variables := map[string]interface{}{
"Login": login,
}
var retval GetUserResponse
var retval getUserResponse
err := client.MakeRequest(ctx, `
query getUser ($Login: String!) {
user(login: $Login) {
generate/config.go
+2 -2
View File
@@ -21,8 +21,8 @@ type Config struct {
Package string `yaml:"package"`
// The filename with the GraphQL schema (in SDL format); defaults to
// schema.graphql
// TODO: allow other formats
// TODO: allow URLs
// TODO: Allow fetching a schema via introspection (will need to figure out
// how to convert that to SDL).
Schema string `yaml:"schema"`
// The filename with the queries; defaults to queries.graphql
Queries string `yaml:"queries"`
generate/generate.go
+14 -8
View File
@@ -56,14 +56,20 @@ func newGenerator(config *Config, schema *ast.Schema) *generator {
}
func (g *generator) Types() string {
defs := make([]string, 0, len(g.typeMap))
for _, def := range g.typeMap {
defs = append(defs, def)
names := make([]string, 0, len(g.typeMap))
for name := range g.typeMap {
names = append(names, name)
}
// Sort alphabetically by type-name. Sorting somehow deterministically is
// important to ensure generated code is deterministic. Alphabetical is
// nice because it's easy, and in the current naming scheme, it's even
// vaguely aligned to the structure of the queries.
sort.Strings(names)
defs := make([]string, 0, len(g.typeMap))
for _, name := range names {
defs = append(defs, g.typeMap[name])
}
// Make sure we have a stable order. (It's somewhat
// arbitrary but in practice mostly alphabetical.)
// TODO: ideally we'd do a nice semantic ordering.
sort.Strings(defs)
return strings.Join(defs, "\n\n")
}
@@ -84,7 +90,7 @@ func (g *generator) getArgument(arg *ast.VariableDefinition) (argument, error) {
func (g *generator) getDocComment(op *ast.OperationDefinition) string {
var commentLines []string
var sourceLines = strings.Split(op.Position.Src.Input, "\n")
sourceLines := strings.Split(op.Position.Src.Input, "\n")
for i := op.Position.Line - 1; i > 0; i-- {
line := sourceLines[i-1]
if strings.HasPrefix(line, "#") {
generate/parse.go
-3
View File
@@ -11,9 +11,6 @@ import (
)
func getSchema(filename string) (*ast.Schema, error) {
// TODO: IRL we have to get the schema from GraphQL (maybe we can generate
// that once we can bootstrap) where it comes as JSON, not SDL, so we have
// to convert (or add gqlparser support to convert)
text, err := ioutil.ReadFile(filename)
if err != nil {
return nil, fmt.Errorf("unreadable schema file %v: %v", filename, err)
generate/template.go
+5 -2
View File
@@ -7,8 +7,11 @@ import (
)
// TODO: package templates into the binary using one of those asset thingies
var _, thisFilename, _, _ = runtime.Caller(0)
var thisDir = filepath.Dir(thisFilename)
// (e.g. embed, if we wait until 1.16 to do this)
var (
_, thisFilename, _, _ = runtime.Caller(0)
thisDir = filepath.Dir(thisFilename)
)
func mustTemplate(relFilename string) *template.Template {
return template.Must(template.ParseFiles(filepath.Join(thisDir, relFilename)))
generate/types.go
+14 -18
View File
@@ -29,12 +29,11 @@ func (g *generator) baseTypeForOperation(operation ast.Operation) *ast.Definitio
func (g *generator) getTypeForOperation(operation *ast.OperationDefinition) (name string, err error) {
// TODO: configure ResponseName format
namePrefix := upperFirst(operation.Name)
name = namePrefix + "Response"
name = operation.Name + "Response"
if def, ok := g.typeMap[name]; ok {
// TODO: check for and handle conflicts a better way
return "", fmt.Errorf("%s already defined:\n%s", name, def)
return "", fmt.Errorf("%s defined twice:\n%s", name, def)
}
fields, err := selections(operation.SelectionSet)
@@ -43,15 +42,16 @@ func (g *generator) getTypeForOperation(operation *ast.OperationDefinition) (nam
}
return g.addTypeForDefinition(
namePrefix, name, g.baseTypeForOperation(operation.Operation), fields)
operation.Name, name, g.baseTypeForOperation(operation.Operation), fields)
}
var builtinTypes = map[string]string{
"Int": "int", // TODO: technically int32 is always enough, use that?
// GraphQL guarantees int32 is enough, but using int seems more idiomatic
"Int": "int",
"Float": "float64",
"String": "string",
"Boolean": "bool",
"ID": "string", // TODO: named type for IDs?
"ID": "string",
}
func (g *generator) addTypeForDefinition(namePrefix, nameOverride string, typ *ast.Definition, fields []field) (name string, err error) {
@@ -117,11 +117,9 @@ type field interface {
type outputField struct{ field *ast.Field }
func (s outputField) Alias() string {
if s.field.Alias != "" {
return s.field.Alias
}
// TODO: is this case needed? tests don't seem to get here.
return s.field.Name
// gqlparser sets Alias even if the field is not aliased, see e.g.
// https://github.com/vektah/gqlparser/blob/c06d8e0d135f285e37e7f1ff397f10e049733eb3/parser/query.go#L150
return s.field.Alias
}
func (s outputField) Type() *ast.Type {
@@ -192,11 +190,11 @@ func (builder *typeBuilder) writeField(field field) error {
}
err = builder.writeType(
// Note we don't deduplicate here -- if our prefix is GetUser and the
// field name is User, we do GetUserUser. This is important because if
// you have a field called user on a type called User we need
// `query q { user { user { id } } }` to generate two types, QUser and
// QUserUser.
// Note we don't deduplicate suffixes here -- if our prefix is GetUser
// and the field name is User, we do GetUserUser. This is important
// because if you have a field called user on a type called User we
// need `query q { user { user { id } } }` to generate two types, QUser
// and QUserUser.
// Note also this is the alias, not the field-name, because if we have
// `query q { a: f { b }, c: f { d } }` we need separate types for a
// and c, even though they are the same type in GraphQL, because they
@@ -273,7 +271,6 @@ func (builder *typeBuilder) writeTypedef(typedef *ast.Definition, fields []field
return err
}
// HACK HACK HACK
builder.typeMap[name] += fmt.Sprintf(
"\nfunc (v %v) %v() {}", name, implementsMethodName)
}
@@ -285,7 +282,6 @@ func (builder *typeBuilder) writeTypedef(typedef *ast.Definition, fields []field
builder.WriteString("string\n")
builder.WriteString("const (\n")
for _, val := range typedef.EnumValues {
// TODO: casing should be configurable
fmt.Fprintf(builder, "%s %s = \"%s\"\n",
builder.typeNamePrefix+goConstName(val.Name),
builder.typeName, val.Name)
generate/unmarshal.go
+1 -1
View File
@@ -33,7 +33,7 @@ func (builder *typeBuilder) maybeWriteUnmarshal(fields []field) error {
for _, typedef := range builder.schema.GetPossibleTypes(typedef) {
fieldInfo.ConcreteTypes = append(fieldInfo.ConcreteTypes,
concreteType{
// TODO: this is quite fragile (and maybe wrong if the
// TODO: this is quite fragile (and wrong if the
// field name + type name are the same)
GoName: builder.typeNamePrefix + fieldInfo.GoName + upperFirst(typedef.Name),
GraphQLName: typedef.Name,
graphql/client.go
+7 -13
View File
@@ -5,7 +5,6 @@ import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"github.com/vektah/gqlparser/gqlerror"
@@ -29,8 +28,12 @@ type payload struct {
Variables map[string]interface{} `json:"variables"`
}
type response struct {
Data json.RawMessage `json:"data"`
Errors gqlerror.List `json:"errors"`
}
func (client *Client) MakeRequest(ctx context.Context, query string, retval interface{}, variables map[string]interface{}) error {
// TODO: streaming reads and writes
body, err := json.Marshal(payload{
Query: query,
Variables: variables,
@@ -54,21 +57,12 @@ func (client *Client) MakeRequest(ctx context.Context, query string, retval inte
}
defer resp.Body.Close()
body, err = ioutil.ReadAll(resp.Body)
if err != nil {
return err
}
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("returned error %v: %v", resp.Status, string(body))
}
var dataAndErrors struct {
Data json.RawMessage `json:"data"`
Errors gqlerror.List `json:"errors"`
}
err = json.Unmarshal(body, &dataAndErrors)
var dataAndErrors response
err = json.NewDecoder(resp.Body).Decode(&dataAndErrors)
if err != nil {
return err
}