-
Notifications
You must be signed in to change notification settings - Fork 491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use field as resolver instead of method. #28
Comments
That would be really useful, maybe it could use introspection and struct tags like |
not sure how much performance penalty we pay for using reflection on this. I do codegen to write these simple resolvers. |
We should test and get some data, that would get us some information. I will do my best to try to get some benchmarks. |
The amount of reflection that this implementation uses is probably my biggest concern. I do love how minimal the API is though. @bsr203 I'd love to see your code gen tools if you'd be willing to share. |
I have two concerns about this:
|
@nicksrandall I agree with neelance fully, and used codegen during the initial migration. The code is tied with our internal stuff, and not intend to use for general purpose. I could highlight how we do it. for a type
|
@neelance I agree with your point of view in golang simplicity. Simplicity is really complicated. And i was thinking how this problem would be solved by the go team and this are my thoughts. What about adding tags to structs? As i'm sure you may know, the json Decoder/Encoder transforms structs into json by looking it's field name, and if tags are present it chooses them over the field name. We could have a similar behavior as the decoder, and i think we could stay well into the go philosophy. This would allow us to have similar behavior to graphql-js, that resolves fields using the same name given in the json object. I was thinking something like this
As you can see here, i could have a Vendor struct that only works with the fields that conform to the schema by having a tag, or if i need i could have a resolver func to conform the schema. Let me know what you think. |
@neelance your points make sense to me. Thanks for explaining. |
Another design point might be the API we are currently using in Thunder's GraphQL implementation (see https://github.com/samsarahq/thunder/blob/master/example/main.go). By default, all fields on the Go struct get exposed as GraphQL fields. For example, the fragment type Address struct {
City, State string
}
type Friend struct {
Name string
Addresses []*Address
}
func (s *Schema) Friend() schemabuilder.Object {
object := schemabuilder.Object{
Type: Friend{},
}
object.FieldFunc("numAddresses", func(f *Friend) int {
return len(f.Addresses)
}
return object
} would result in a GraphQL schema type Address {
city string
state string
}
type Friend {
name string
addresses [Address]
numAddresses int
} Automatically exposing fields feels similar to |
For Same for graphql-go: The only standard way to read values are methods, because that fits better to the GraphQL model. Also adding the option to read struct field values would only make the API more complicated. However, we may want to add an entry point for custom behavior, e.g. |
I wrote some code generation to do this. Theoretically using codegen to do this has some slowdowns, namely because you have to return a slice of the resolver type rather than just returning pointers to structs themselves. Therefore you require an extra O(n) operation with memory allocation (albeit just ~12 bytes or so) on every slice you return. It's fast, but for large datasets maybe it might incur a performance penalty. |
Hey all I though I would just share the quick hacked together generator we made to generate most of the boiler-plate code required. It's by no means a full solution, but we plan to continue working on it to cover all scenarios. PRs are always welcome too ;-) https://github.com/bLevein/graphql-gen-go |
Just a heads up I updated https://github.com/bLevein/graphql-gen-go and it should now generate all the correct type resolver funcs for fields and the correlating go types. Still need to add unions, interfaces, and a few other things. Any input is appreciated, so feel free to file bug and/or feature requests. (p.s. I know the code is still kind of ugly, but it works for now ;-) ) |
@euforic is there a reason why you didn't choose to use golang's text/template? |
@tonyghita I like the approach grpc takes with filling a buffer, so that is the direction I wanted to go with it. Makes things a bit more flexible. |
This may be addressed by #88. |
Would it be possible to use a struct field as a resolver instead of always using methods? Most of my methods are just simple getters so it would be convenient to allow resolver to default to field. Then I could implement method only when I need special logic.
The text was updated successfully, but these errors were encountered: