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

graphql: add FieldContext.Child field function and enable it in codegen #2062

Merged
merged 1 commit into from
Mar 29, 2022

Conversation

a8m
Copy link
Collaborator

@a8m a8m commented Mar 22, 2022

Pasting here the discussion from gqlgen Discord:

In Ent, we have a nice workaround for the N+1 problem when working with gqlgen (see https://entgo.io/docs/tutorial-todo-gql-field-collection#ent-solution), and we do it by reading the query fields recursively and constructing one Ent query for the GraphQL query.

However, we have a problem when a field accepts arguments as parameters - we can't get them today with the current API. We use "fields collection" to get the "raw arguments", but that's not enough as we need the "processed version" that exists in the FieldContext in order to evaluate the Ent query.

This PR adds a new field function to the graphql.FieldContext named Child that gets a graphql.CollectedField object and returns the processed graphql.FieldContext for the collected field. This can also solve #1144.

@a8m a8m changed the title graphql: add FieldContext.ChildArgs field and enable it in codegen graphql: add FieldContext.Child field function and enable it in codegen Mar 22, 2022
@coveralls
Copy link

coveralls commented Mar 22, 2022

Coverage Status

Coverage increased (+0.08%) to 74.501% when pulling bf9caea on a8m:childfield into 36fb3dc on 99designs:master.

@a8m a8m force-pushed the childfield branch 4 times, most recently from d6b2af0 to 0b60785 Compare March 22, 2022 20:08
@StevenACoffman
Copy link
Collaborator

This is great! Thanks so much!

@frederikhors
Copy link
Collaborator

Do we need to update docs for these changes?

@StevenACoffman
Copy link
Collaborator

I rebased it for you to fix the conflicts

@a8m
Copy link
Collaborator Author

a8m commented Mar 25, 2022

Thanks, @StevenACoffman.

@frederikhors, I'll extend the godoc for it before landing it.

@frederikhors
Copy link
Collaborator

I'll extend the godoc for it before landing it

Yes, please. Features without docs == no features at all.

Copy link
Collaborator

@StevenACoffman StevenACoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let you land it yourself.

@a8m a8m merged commit 1324c3f into 99designs:master Mar 29, 2022
@a8m a8m deleted the childfield branch March 29, 2022 11:37
@a8m
Copy link
Collaborator Author

a8m commented Mar 29, 2022

Added godoc and merged. Thanks for the review @StevenACoffman and @frederikhors.

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

Successfully merging this pull request may close these issues.

4 participants