move TODOs into issues

This commit is contained in:
Ben Kraft
2021-04-21 19:05:38 -07:00
parent 748f2cf072
commit b399e0d740
8 changed files with 5 additions and 78 deletions
-42
View File
@@ -87,45 +87,3 @@ Khan Academy is a non-profit organization with a mission to provide a free, worl
### Design ### Design
See [DESIGN.md](DESIGN.md) for documentation of major design decisions in this library. See [DESIGN.md](DESIGN.md) for documentation of major design decisions in this library.
### Major TODOs
(+) denotes things we further need before recommending anyone else use this in prod
Basic features:
- redo support for interfaces, unions, fragments (see DESIGN)
Fancy features:
- allow `__all` to request all fields (probably a terrible idea though)
- plugin to make `_entities` queries for apollo federation easy (basically you would just write the fragment)
- auto-caching -- if you `@genqlient(cache: true)` we cache (you can pass a cache into the client or something)
Generated code customization:
- add flag(s) to make a field use a pointer (for optionality or perf; see DESIGN)
- collapsing -- should be able to have `mutation { myMutation { error { code } } }` just return `(code string, err error)`
- map a field to a particular Go type (if you want to use a named type for some string, say)
- specify a particular name for a Go type, or for the helper function, or whatever else
- include full query in generated godoc
Config options:
- proper config/arguments setup (e.g. with [viper](https://github.com/spf13/viper))
- (+) improve client_getter to be more usable (and document it), maybe by just saying it has to be a function that takes context, or flag it out for now.
- get schema via GraphQL introspection (although honestly, it seems like SDL is becoming popular enough there may be no need)
- whether names should be exported
- default handling for optional fields? (maybe generate a HasFoo, you can always ignore if you don't care)
- generate mocks?
Runtime:
- (+) basic tests for graphql package
- integration tests against common servers for graphql package
- send hash rather than full query
Internal:
- set up linting beyond `go vet`
- switch to a snapshot testing lib (cupaloy looks good) for tests
Other:
- (+) API cleanup (check godoc)
- (+) improved validation and error checking
- (+) documentation
- get a designer to fix my bad logo-thing
+1 -8
View File
@@ -18,8 +18,6 @@ var defaultConfig = &Config{
type Config struct { type Config struct {
// The filename with the GraphQL schema (in SDL format); defaults to // The filename with the GraphQL schema (in SDL format); defaults to
// schema.graphql // schema.graphql
// TODO: Allow fetching a schema via introspection (will need to figure out
// how to convert that to SDL).
Schema string `yaml:"schema"` Schema string `yaml:"schema"`
// Filenames or globs with the operations for which to generate code; // Filenames or globs with the operations for which to generate code;
@@ -64,9 +62,7 @@ type Config struct {
// (which will be named ctx). For example, this might do // (which will be named ctx). For example, this might do
// ctx.Value(myKey).(*graphql.Client). If omitted, client must be // ctx.Value(myKey).(*graphql.Client). If omitted, client must be
// passed to each method explicitly. // passed to each method explicitly.
// TODO: what if you want to do an import in this snippet, e.g. for a // TODO(#5): This is a bit broken, fix it.
// getter function, global var, or a context-key-type?
// TODO: what if you want to return err?
ClientGetter string `yaml:"client_getter"` ClientGetter string `yaml:"client_getter"`
// A map from GraphQL scalar type name to Go fully-qualified type name for // A map from GraphQL scalar type name to Go fully-qualified type name for
@@ -75,9 +71,6 @@ type Config struct {
// to int, Float to float64, and Boolean to bool), but this setting will // to int, Float to float64, and Boolean to bool), but this setting will
// extend or override those mappings. These types must define MarshalJSON // extend or override those mappings. These types must define MarshalJSON
// and UnmarshalJSON methods, or otherwise be convertible to JSON. // and UnmarshalJSON methods, or otherwise be convertible to JSON.
// TODO: figure out if it makes sense to say you can use these for
// non-scalar types; technically it should just work, but what if you
// didn't request the right fields?
Scalars map[string]string `yaml:"scalars"` Scalars map[string]string `yaml:"scalars"`
// Set automatically to the filename of the config file itself. // Set automatically to the filename of the config file itself.
-1
View File
@@ -138,7 +138,6 @@ func (g *generator) addOperation(op *ast.OperationDefinition) error {
var builder strings.Builder var builder strings.Builder
f := formatter.NewFormatter(&builder) f := formatter.NewFormatter(&builder)
// TODO: this could even get minifed.
f.FormatQueryDocument(&ast.QueryDocument{ f.FormatQueryDocument(&ast.QueryDocument{
Operations: ast.OperationList{op}, Operations: ast.OperationList{op},
// TODO: handle fragments // TODO: handle fragments
+2 -1
View File
@@ -53,7 +53,8 @@ func getQueries(basedir string, filenames []string) (*ast.QueryDocument, error)
// in one might reference fragments in another. // in one might reference fragments in another.
// //
// TODO(benkraft): It might be better to merge just within a filename, so // TODO(benkraft): It might be better to merge just within a filename, so
// that fragment-names don't need to be unique across files. // that fragment-names don't need to be unique across files. (Although
// then we may have other problems; and query-names still need to be.)
mergedQueryDoc := new(ast.QueryDocument) mergedQueryDoc := new(ast.QueryDocument)
addQueryDoc := func(queryDoc *ast.QueryDocument) { addQueryDoc := func(queryDoc *ast.QueryDocument) {
mergedQueryDoc.Operations = append(mergedQueryDoc.Operations, queryDoc.Operations...) mergedQueryDoc.Operations = append(mergedQueryDoc.Operations, queryDoc.Operations...)
-2
View File
@@ -7,8 +7,6 @@ import (
"text/template" "text/template"
) )
// TODO: package templates into the binary using one of those asset thingies
// (e.g. embed, if we wait until 1.16 to do this)
var ( var (
_, thisFilename, _, _ = runtime.Caller(0) _, thisFilename, _, _ = runtime.Caller(0)
thisDir = filepath.Dir(thisFilename) thisDir = filepath.Dir(thisFilename)
+1 -19
View File
@@ -36,7 +36,6 @@ func (g *generator) baseTypeForOperation(operation ast.Operation) (*ast.Definiti
} }
func (g *generator) getTypeForOperation(operation *ast.OperationDefinition, queryOptions *GenqlientDirective) (name string, err error) { func (g *generator) getTypeForOperation(operation *ast.OperationDefinition, queryOptions *GenqlientDirective) (name string, err error) {
// TODO: configure ResponseName format
name = operation.Name + "Response" name = operation.Name + "Response"
if def, ok := g.typeMap[name]; ok { if def, ok := g.typeMap[name]; ok {
@@ -90,12 +89,6 @@ func (g *generator) typeName(prefix string, typ *ast.Definition) (name, nextPref
// that's the actual name (the rest are really qualifiers); but if // that's the actual name (the rest are really qualifiers); but if
// they are the same then including it once suffices for both // they are the same then including it once suffices for both
// purposes.) // purposes.)
// TODO: do this a bit more fuzzily; for example if you have a field
// doThing: DoThingMutation
// or
// error: MyError
// we should be able to be a bit smarter than
// DoThingDoThingMutation/ErrorMyError.
name += typeGoName name += typeGoName
} }
@@ -115,15 +108,6 @@ func (g *generator) getTypeForInputType(opName string, typ *ast.Type, options, q
name := matchFirst(typ.Name(), opName) name := matchFirst(typ.Name(), opName)
builder := &typeBuilder{generator: g} builder := &typeBuilder{generator: g}
// note prefix is ignored here (see generator.typeName) // note prefix is ignored here (see generator.typeName)
// TODO: passing options is actually kinda wrong, because it means we could
// break the "there is only Go type for each input type" rule. In practice
// it's probably rare that you use the same input type twice in a query and
// want different settings, though, and it just means we choose one or the
// other set of options.
// TODO: it's also awkward because you have no way to pass an option for an
// individual input-type field.
// TODO: should we use pointers by default for input-types if they're
// structs?
err := builder.writeType(name, "", typ, selectionsForInputType(g, typ, queryOptions), options) err := builder.writeType(name, "", typ, selectionsForInputType(g, typ, queryOptions), options)
return builder.String(), err return builder.String(), err
} }
@@ -289,9 +273,7 @@ func (builder *typeBuilder) writeType(name, namePrefix string, typ *ast.Type, fi
typ = typ.Elem typ = typ.Elem
} }
if options.GetPointer() { if options.GetPointer() {
// TODO: this does []*T, you might in principle want *[]T or // Note this does []*T or [][]*T, not e.g. *[][]T. See #16.
// *[]*T. We could add a "sliceptr" option if it comes up (that's
// still not correct if you wanted *[][]*[]T, but, like, tough luck).
builder.WriteString("*") builder.WriteString("*")
} }
-1
View File
@@ -25,7 +25,6 @@ func lowerFirst(s string) string {
} }
func upperFirst(s string) string { func upperFirst(s string) string {
// TODO: initialisms
return changeFirst(strings.TrimLeft(s, "_"), unicode.ToUpper) return changeFirst(strings.TrimLeft(s, "_"), unicode.ToUpper)
} }
+1 -4
View File
@@ -13,10 +13,7 @@ import (
// Client is the interface that the generate code calls into to actually make // Client is the interface that the generate code calls into to actually make
// requests. // requests.
// //
// Unstable: This interface is likely to change before v1.0. // Unstable: This interface is likely to change before v1.0, see #19.
//
// TODO: figure out what we, and others, actually want to use this for, and
// consider supporting explicit hooks for that instead.
type Client interface { type Client interface {
// MakeRequest must make a request to the client's GraphQL API. // MakeRequest must make a request to the client's GraphQL API.
// //