genqlient icon indicating copy to clipboard operation
genqlient copied to clipboard

Allow __all to request all fields

Open benjaminjkraft opened this issue 5 years ago • 6 comments

A fun thing we can do, because we generate code, is to allow you to put in a pseudo-field like __all, which we expand to all the fields. That's probably a terrible idea, both in that it diverges from the spec, and you shouldn't want to do that, but we could do it anyway! And it would be useful especially for tests.

benjaminjkraft avatar Apr 22 '21 01:04 benjaminjkraft

Another issue to think about here is it's not obvious what __all would expand to for non-leaf fields. For example suppose you have

type Query { a: A }
type A { b: B }
type B { a: A }

query { a { __all } }

We can't expand all fields recursively, because it would go on forever! Do we just never expand non-leaf fields? Do we expand everything we can do without circular references?

benjaminjkraft avatar Nov 11 '22 07:11 benjaminjkraft

@benjaminjkraft That will be a really nice feature. There is a case in generating all the fields with __all that makes absolute sense. I can give you an example. I already have a graphql server generated with the 99designs generator. The filtering and the fields are already being handled by this server based on what the user requests. From the server side, we need to reach out to the database using graphql as well, but instead, we need to take the whole object in order to control the response to the user.

droslean avatar Nov 11 '22 09:11 droslean

Also, you will need to implement a way to control the depth of the fields. Otherwise, this can end up in an endless loop for the reversed fields.

droslean avatar Nov 11 '22 09:11 droslean

Ah, querying a database where you want to SELECT * does make sense. That plus the test use case seems like enough reason to add this feature, albeit still probably with some warnings. Limiting depth, maybe as __all(depth: Int) seems like a great mechanism, not sure why I didn't think of that!

benjaminjkraft avatar Nov 11 '22 20:11 benjaminjkraft

I wrote a quick POC on benkraft.__all, implementing this for depth: 1 only, without real error handling or documentation, and with some other limitations. (You can see a sample of the syntax and what it expands to in the tests.) It looks like it should be plenty doable, maybe later this weekend I'll have some time to finish it off. The only significant question I ran into so far is what to do about fields with required arguments: maybe just skip them just as we skip non-leaf fields (with depth 1 that is).

One question about depth is: is it going to be enough? What if you want to fetch, say, all leaf fields plus field { id } for non-leaf fields? I guess you can manually enumerate the non-leaf fields, or eventually we could support something like __all { id } that expands to include all fields with a child field id (although that would be a bit more work).

benjaminjkraft avatar Nov 11 '22 21:11 benjaminjkraft

I think it would be better to start by supporting only _all with depth and then if it is needed, non-leaf fields can be supported as well. But the POC looks super OK.

droslean avatar Nov 11 '22 21:11 droslean