Skip to content
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

Proposal: Generate interface for schema resolver like in GRPC for static checkers #39

Closed
inotnako opened this issue Dec 8, 2016 · 10 comments

Comments

@inotnako
Copy link

inotnako commented Dec 8, 2016

Now you made check resolvers by reflections on the fly. My suggest generate some code with interface by schema file for static check resolver impl on compilation step:

$ ls -l 
schema.gql
schema.go // <- go generate graphql
schema.gql.go // generated file with interface of resolver, enum and other helpers %)))

what you think about it?

@neelance
Copy link
Collaborator

neelance commented Dec 8, 2016

Hmm, it would be possible to generate one Go interface per GraphQL type. We would lose some flexibility on how the method signatures can look like, but we may gain some ease of use because it is clear to see when a certain Go type does or does not implement an interface.

@bsr203 What do you think?

@bsr203
Copy link

bsr203 commented Dec 8, 2016

currently, it distinguish between simple resolver with no tracing info by resolver signature, and hope we can retain the same behavior. I like uniform resolvers with a standard signature, but with a way to suppress tracing on some.

Also, can it be extended to execution time performance by helping with struct packing? If not, then we have to weigh the tradeoff between improved safety and ease of validation vs extra build step and additional generated code.

I am not familiar with how it helps GRPC, but I am pro codegen if it helps to reduce magic and improve performance.

Cheers.

@inotnako
Copy link
Author

inotnako commented Dec 8, 2016

@bsr203

it helps to reduce magic and improve performance.

Yes, it is.

Also, can it be extended to execution time performance by helping with struct packing?

like in grpc, after generate you receive implementation of the interface with spec arguments your types
Example:
schema query

user(id: ID!) User
users(first: Int!,after: ID) [User]!

gen code interface

type UsersReq struct {
  First int
  After  graphql.ID
}
type UserResolver interface {
   ID() graphql.ID
   Name() string
}
type Resolver interface {
  User(id graphql.ID) UserResolver
  Users(in *UsersReq) []UserResolver
}

@bsr203
Copy link

bsr203 commented Dec 8, 2016

@antonikonovalov thanks for the details. My only concern is how it play with modularity. I currently concatenate schema fragments and pass it to the parser, and there is no single gigantic schema file. May be similar to SchemaBuilder.ToJSON, we could dump out the interface def file than using go generate. you guys would have better idea :-)

@neelance
Copy link
Collaborator

neelance commented Dec 9, 2016

Here is an example of the flexibility we would lose: Currently we can have this:

func (r *humanResolver) ID() graphql.ID {
	return r.h.ID
}

The context and error return value are optional. With generated interfaces this is not possible any more, so we would need to have this everywhere:

func (r *humanResolver) ID(ctx context.Context) (graphql.ID, error) {
	return r.h.ID, nil
}

Similar issues apply to parameter types. It would also make my proposal on #40 (comment) impossible.

@inotnako
Copy link
Author

inotnako commented Dec 9, 2016

@neelance For me one interface better, then two :) and if r.h == nil you catch panic
I am prefer interface with context and errors

@bsr203

I currently concatenate schema fragments and pass it to the parser, and there is no single gigantic schema file.

Is your gigantic schema like cross schema with one namespace ? Can you give me example?

gRPC suggest next decision:

you have 3 schemas by resources, like:

users.proto

services Users {
   GetUser(ID) returns (User)
}

message ID {
    string ID = 1;
}

message User {
    string ID = 1;
    string Name = 2;
}

groups.proto

services Groups {
   GetGroup(ID) returns (Group)
}

message ID {
    string ID = 1;
}

message Group {
    string ID = 1;
    string Name = 2;
}

and after generation you can set all to one server

        ...
        s := grpc.NewServer()

	pbu.RegisterUsersServer(s, &userResolver{})
	pbg.RegisterGroupsServer(s, &groupResolver{})

	s.Serve(lis)
        ...

We can write some implementation in here. I think need spec for namespace schemas or rules join schemas.

@bsr203
Copy link

bsr203 commented Dec 9, 2016

@antonikonovalov thanks for the detailed response. Registering schema + resolver was discussed before. see #1 .

what I mean above is, it is too hard to maintain one schema file where I like to keep it at the module along with the resolver. It is easy currently to just concatenate all the schema files and feed to the parser. so, it may better the SchemaBuilder spit out the interface definition than try to handle multiple generated files and combining them.

I don't know all the implication of above proposal, and @neelance would be the best person to weigh the tradeoffs.

@tonyghita
Copy link
Member

I'd actually like to write my own generator particular to my specific needs.

Ideally I could parse a schema file like so:

_, filename, _, _ := runtime.Caller(1)
currentDir := path.Dir(filename)

s, err := ioutil.ReadFile(path.join(currentDir, "schema.graphql"))
if err != nil {
  log.Fatalln("Oops", err)
}

schema, err := graphql.ParseSchema(string(s), &Resolver{})
if err != nil {
  log.Fatalln("Nooooo!", err)
}

And then be able to introspect the internals of the schema to generate resolvers.

@tonyghita
Copy link
Member

@neelance any thoughts on export some fields on Schema to make this easier?

@neelance
Copy link
Collaborator

@antonikonovalov The type signatures of graphql-go are too flexible (similar to encoding/json), especially after merging #3 (comment). Using code generation is not possible because of that. I'll close this issue.

@tonyghita I have plans to expose the Go resolvers for __schema. Not there yet though. Could you please open a new issue for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants