-
-
Notifications
You must be signed in to change notification settings - Fork 251
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
Optimize execution of pure and single-field object queries #2338
Conversation
stepReducer.reduceStep(plan, request.field, Map.empty, Nil) match { | ||
case PureStep(resp) => Exit.succeed(GraphQLResponse(resp, Nil)) | ||
case reducedStep => makeCache.flatMap(runQuery(reducedStep, _)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we still need to suspend this? Returning an Exit means we lose referential transparency if it's now getting strictly evaluated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technicaly, you're right, this is a bit of an interesting because we execute this method on each request, and it's not really a user-facing method (even if it's public). So it doesn't need to be referentially transparent. But since it does return a ZIO
, I think we should make it referentially transparent to avoid any potential issues in the future
case None => Exit.unit | ||
case Some(w) => w | ||
else | ||
ZIO.suspendSucceed { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this here in order to align with https://github.com/ghostdogpr/caliban/pull/2338/files#r1673288713. We don't need this method to be referentially transparent because it's executed on each request, but better to have it referentially transparent for consistency
This PR optimizes the execution of the following: