-
-
Notifications
You must be signed in to change notification settings - Fork 116
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
responseCache plugin additional extension metadata when disabled #2224
Comments
@n1ru4l thoughts on this? I have a local patch that adds the |
Hi @mmadson ! Sorry for the response delay here. If you're not sure how to do it, don't hesitate to create a draft PR and ask any questions :-) We are here to help! |
Thanks @EmrysMyrddin, no worries about the delay. I'll take a pass at an implementation and get back to ya asap. |
Okay so here is the issue I'm having @EmrysMyrddin, hopefully you have a suggestion. I'd like For the
The Instead of simply returning on this line I need to add the enabled: false to the result metadata: so I was thinking something like:
where disabledResult returns something like:
This gets us most of the way there, but there is still one bit of metadata related to the response cache plugin that for some reason gets set from the graphql yoga package. I believe this is the case when there is a cache hit. So we'd need to modify this line to include The fact that this metadata is spread across multiple projects is a bit surprising so let me know if I've missed something. If you agree with all of this, I can go ahead and get the 2 PRs ready -- one for this project and one for graphql yoga. Not really sure how to coordinate releases. |
I am not sure if we really need an extension metadata when it is disabled. |
Unless there is another reason on why we dould need this new field, I agree with @ardatan. Afaik dows not provide any additional value over checking whether the extendion field exists. |
@ardatan honestly, I didn't even think about using the presence of the root metadata node to detect if caching was or wasn't enabled -- so now that you mention it, I suppose you are right, that would be sufficient. Thanks for taking the time to consider this issue, feel free to consider it resolved. |
Is your feature request related to a problem? Please describe.
When enabled expression evaluates to false and includeExtensionMetadata is set to true, there is no indication in the response that caching did or did not occur.
Describe the solution you'd like
Given
When
Then
Include extension metadata indicating that
This new enabled field in the extension metadata should also be present for other responses when enabled evaluates to true and should indicate true in these cases.
Describe alternatives you've considered
Patching the lib via package-patch
Additional context
N/A
The text was updated successfully, but these errors were encountered: