-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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::Schema::Interface#resolve_type
appears to resolve lazy results too early
#4835
Comments
Hey, thanks for the detailed write-up. This is the right place for the issue -- GraphQL-Batch has done its job, now it's up to GraphQL-Ruby to resolve those promises as best it can! I bet this can be improved in GraphQL-Ruby's runtime somehow. I'll take a closer look soon and work up a small replication based on your script above, then see about improving the promise resolving code to make it work better, and follow up here 👍 |
Thanks again for the detailed report. I copied your example into the GraphQL-Ruby tests and spent some time trying to understand why it's not working "properly." I couldn't quite grok what needs to be better ... but I'll keep trying! |
Describe the bug
When lazily-loading values within
resolve_type
, the promise appears to be immediately synced. This causes N+1 queries when usinggraphql-batch
, where the result of aresolve_type
depends on another record.Versions
graphql
version: 2.2.8rails
(or other framework): ActiveRecord 7.1.3other applicable versions (
graphql-batch
, etc):graphql_batch
0.5.4Example test cases
resolve_type
Standard field-based lazy loading, for comparison
Expected behavior
The
resolve_type
example above should only issue 3 queries, same as the "standard" test.Log output from "standard" test
Actual behavior
The
resolve_type
test issues one query perresolve_type
call, and output implies the promise is being immediately synced upon return fromresolve_type
.Log output from "resolve_type" example
Additional context
I'm not sure if
resolve_type
is intended to be used in this way, but as far as I can tell from reading over previous issues and PRs, lazy resolution is an intended feature. If this is expected behavior, or if it would be better reported tographql-batch
, feel free to let me know and close this issue.Also, I recognize the schema presented is rather cursed. However, it mirrors a similar example to something we're encountering in our application; we have a versioning system which can have one of two STI types as its parent, and we want to transparently return this versioned data to our clients. In order to know which type to return however, we must know the type of the parent, which requires loading the associated parent within
resolve_type
.The text was updated successfully, but these errors were encountered: