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-yoga/plugin-response-cache: Support more advanced use cases #3018

Closed
Tracked by #2700 ...
klippx opened this issue Sep 27, 2023 · 2 comments
Closed
Tracked by #2700 ...

@graphql-yoga/plugin-response-cache: Support more advanced use cases #3018

klippx opened this issue Sep 27, 2023 · 2 comments

Comments

@klippx
Copy link
Contributor

klippx commented Sep 27, 2023

Is your feature request related to a problem? Please describe.

I am trying out Yoga as an alternative to Apollo Server 4, and when it comes to response cache plugin the one in Yoga is a more crude version that doesn't have any means to support the following API:

  • extraCacheKeyData
  • calculateCacheKey
  • shouldReadFromCache and shouldWriteToCache

Describe the solution you'd like

  • extraCacheKeyData(requestContext): any - Ability to give a hook for adding an extra key with value being result of callback, which internally is passed to buildResponseCacheKey({ ...original, extra })
  • calculateCacheKey(requestContext): string - Ability to alter cache key used for reading and writing to cache, if this is passed use const key = calculateCacheKey ? ${originalKey}:${calculateCacheKey(requestContext)} : originalKey.
  • shouldReadFromCache abd shouldWriteToCache - Ability to skip response cache altogether, such as based on header (might not need a code change)*

Describe alternatives you've considered

  • calculateCacheKey might be might be more of a @envelop/response-cache-redis concern, and as such I might have been able to use buildRedisOperationResultCacheKey, but it is not yielding requestContext. Still we might want to split this feature request to that library instead.
    *) I think shouldReadFromCache abd shouldWriteToCache can be combined into simply "skip cache", since we are always using them together and set to the same value. But we might not even need support for these in the plugin itself, as we can just skip adding this plugin altogether instead.

Additional context

N/A

@klippx klippx changed the title @graphql-yoga/plugin-response-cache: Support more advanced use cases @graphql-yoga/plugin-response-cache: Support more advanced use cases Sep 27, 2023
@EmrysMyrddin
Copy link
Collaborator

EmrysMyrddin commented Sep 27, 2023

Hi,

I'm sorry we are lacking some documentation on this plugin (it evolved quickly during the last months).

Some of the point's are already possible, or partially possible at least.

  • calculateCacheKey(requestContext): string already exists in the Envelop plugin and is named buildResponseCacheKey. But it is a bit limited in the Yoga version where it doesn't give you access to the context or the request. It should be easy to add it :-)
  • extraCacheKeyData(requestContext): any I'm not sure to see the uses cases of this. How is it different than using calculateCacheKey ?
  • shouldReadFromCache and shouldWriteToCache are replaced by 2 options :
    • enabled(request: Request): boolean which allow to entirely skip the cache (reading and righting) based on request
    • shouldCacheResult(cacheKey: string, result: ExecutionResult): boolean which allow to skip cache (reading and writing) based on the query's result. The default implementation for example skips results if it contains any error.

So to sum things up, i think we have 2 actions that are needed from our parts here:

@klippx
Copy link
Contributor Author

klippx commented Sep 28, 2023

Agreed, extraCacheKeyData and calculateCacheKey can both be implemented using buildResponseCacheKey.

And using the 2.2.0-rc I can see that having access to request is good enough!

Do you know if there is a util method to extract operationName from a request: Request? We usually filter out __ApolloGetServiceDefinition__ and IntrospectionQuery by default.

@klippx klippx closed this as completed Sep 29, 2023
This was referenced May 7, 2024
This was referenced May 23, 2024
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

No branches or pull requests

2 participants