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

Resolverless objects #80

Closed
eric-am opened this issue Apr 5, 2018 · 11 comments
Closed

Resolverless objects #80

eric-am opened this issue Apr 5, 2018 · 11 comments

Comments

@eric-am
Copy link

eric-am commented Apr 5, 2018

Is there a good way to handle objects which are plain old Go objects that do not require a Resolver func?

For example, my schema has some objects like this:

type Widget {
  foo: String!
  bar: String!
  children: [Tuple!]!
}

type Tuple {
  frob: String!
  noz: Int
}

On my backend, the real computation work is for producing a Widget... and it comes complete with all the Tuples. I want the Tuple fields to be all listed out in my graphql schema and queries so that I'm taking advantage of all the type checking... but there's nothing interesting to put in for "resolver" logic -- the Widget resolver already did all the work.

So, ideally, I'd like gqlgen's model generation to make a Widget type in Go that contains the whole thing at once:

type Widget struct {
  Foo string
  Bar string
  Children []Tuple
}

Correspondingly, my Resolver interface need not have a Widget_children() method; gqlgen should be able to generate an executionContext._Widget_children method that does all the remaining recursion itself.

I'm not sure if this relates to custom scalars. My intuition is "no" because I don't really consider these things scalars -- it is still desirable for the GQL query to specify fields inside these objects. This specification of types for trees of fields is most of the reason we're using graphQL schemas, after all! The example I gave here with just one nested struct and a few primitives is a simple one; I have this effect for objects around 4~5 layers deep.

Supporting this kind of superscalar/resolverless object would cover... honestly, a lot of the objects in the graphql schemas I'm working with. It would drastically reduce the number of methods on my Resolver interface (and thus drastically reduce desires for resolver generators like described in #9 , for example).

Is there any feature like this in gqlgen I haven't seen yet? Do you think it sounds possible to add codegen that has this kind of behavior? (Or am I just using graphql "wrong"?)

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

Have you tried it, It should just work?

type Widget struct {
  Foo string
  Bar string
  Children []Tuple
}

if you create a types.json with

{
   "Widget": "package/for/widget.Widget"
}

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

The introspection objects are all defined this way so only the root query resolvers need to be added to the graph manually.

https://github.com/vektah/gqlgen/blob/master/neelance/introspection/introspection.go

@eric-am
Copy link
Author

eric-am commented Apr 5, 2018

Ohhhhh. I didn't realize that's how to hit that codegen path. Trying it now.

So can I flag the model codegen to make the full objects somehow? 🤔 I could do all the types manually like this, but the model codegen is actually already super close to what I want anyway...

@eric-am
Copy link
Author

eric-am commented Apr 5, 2018

Holy smokes, I made some other errors declaring my type manually and your error messages were excellent and now it works.

This library is kind of amazing 🎉

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

Perhaps replacing non scalars with an ID isn't as useful as I first thought. Chances are if that ID is useful you would have already created an external database model for it.

I don't really want to break BC, but perhaps a cli switch for inlining nested models and a deprecation warning...

edit:
I guess the problem is if you generate every non scalar field inline you can never go back to a resolver. This would make all generated models terminal leaves in the graph.

@eric-am
Copy link
Author

eric-am commented Apr 5, 2018

I think the types.json allowing the user to declare their own types and ask to use them is pretty great. Sometimes I want to use that for some very custom type. Sometimes the generated types (plus children) will fit perfectly though. Why not both? 😄

Perhaps if types.json was a union of options for each type? e.g.

{
  "Widget": {"leaf": true},
  "OtherThing: {"userType": "mycorp.org/models.OtherThing"}
}

or some other way of configuring like this.

This would make all generated models terminal leaves in the graph.

My immediate gut reaction was thinking that should be fine in practice, but maybe I just haven't thought of enough use cases yet. Would config options like the above give us a way to avoid this limit anyway though?

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

I'm hoping to get rid of types.json in favour of schema directives, eg:

type Widget @generate(leaf: true) {
 ...
}

type OtherThing @bind(type: "mycorp.org/models.OtherThing") {
 ...
}

Unfortunately the parser I'm using doesn't currently support them, but thats the end goal. Putting effort into doing more with types.json feels like wasted effort. If an all or nothing cli flag is enough it might be a decent tradeoff until the directives land.

Lots of good reading in the apollo docs, as usual https://www.apollographql.com/docs/graphql-tools/schema-directives.html

@eric-am
Copy link
Author

eric-am commented Apr 5, 2018

Are directives really appropriate? Whether things are e.g. a leaf type like this seems like an implementation detail... and of my server side specifically, not of anything any clients should know about.

I guess if it works, it works, but... I figure I should be able to use the same schema to generate basic compatibility checks and test mocks and such in the client (even in other languages, etc...), so introducing more directives that other tools need to most certainly ignore would seem... odd. My two cents would be that keeping a separate configuration file (like types.json) with these details apart from the schema seems much cleaner.

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

not of anything any clients should know about.

Anything used by the server to generate would get stripped from the introspection api, clients shouldn't see it. Most of the client tools should be generating the schema using reflection, not poking at the file directly. just my $0.02

@eric-am
Copy link
Author

eric-am commented Apr 5, 2018

Well, this is not a hill I'm going to die on or anything 😉 but for what it's worth: I replicate the schema file between projects and generate tests in the other repos for CI, offline -- not using the introspection API. I like the offline nature of this. Doing it through introspection would "work" but seems like 10x more moving parts -- I'd have to launch a whole server, configure my other project's tools to aim at that server just to get what should be an almost-identical schema file back out, and automating this would be... cough. Meanwhile, just copying a file between repos, a trivial shell script can check if they're in sync or not, and even do a git bisect in seconds; simple is good.

From the other way... I can't really think of anything that gets better by putting Golang type mapping info -- which is almost definitionally orthagonal to the graphql schema, since language agnosticism was one of the original selling points of GQL -- into the schema file. It just avoids adding another file...? Inodes are cheap :)

@vektah
Copy link
Collaborator

vektah commented Apr 5, 2018

Yeah, fair points. I'll make sure the current system keeps working.

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

2 participants