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

Allow __all to request all fields #27

Open
benjaminjkraft opened this issue Apr 22, 2021 · 6 comments
Open

Allow __all to request all fields #27

benjaminjkraft opened this issue Apr 22, 2021 · 6 comments
Labels
enhancement New feature or request needs use cases Feature we can add if people find use cases that need it (comment if that's you)

Comments

@benjaminjkraft
Copy link
Collaborator

benjaminjkraft commented Apr 22, 2021

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
Copy link
Collaborator Author

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?

@droslean
Copy link

@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
Copy link

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.

@benjaminjkraft
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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).

@droslean
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs use cases Feature we can add if people find use cases that need it (comment if that's you)
Projects
None yet
Development

No branches or pull requests

2 participants