search

StarpTech / apollo-datasource-http

Optimized JSON HTTP Data Source for Apollo Server
MIT License
73 stars 32 forks source link

If Response is cacheable but method is not GET then it will never be read #19

Closed paolommj closed 2 years ago

paolommj commented 3 years ago

Nice job on the datasource, it is a joy to use.

there may be some use cases in with a cacheable response doesn't have a GET method, ex: when querying another graphql or an elasticsearch, but in https://github.com/StarpTech/apollo-datasource-http/blob/d234a105b3166d2ec0fd4b12ebb6764145005fb2/src/http-data-source.ts#L378 only GET requests are retrieved from cache

StarpTech commented 3 years ago

Do you want to cache POST requests? We can introduce a request option enforceCache? Would you like to create a PR? Don't forget to add tests.

paolommj commented 3 years ago

you already have this overridable method https://github.com/StarpTech/apollo-datasource-http/blob/d234a105b3166d2ec0fd4b12ebb6764145005fb2/src/http-data-source.ts#L133-L138

that may be used in some way? what do you think?

paolommj commented 3 years ago

I'm not sure if I have the time for a PR soon, but I might be

StarpTech commented 3 years ago

you already have this overridable method

This is about the response cache not if the request should be memorized. At this point, we have no response.

paolommj commented 3 years ago

yes, i was implying something with the same approach

protected isRequestCacheable(request: Request): boolean

StarpTech commented 2 years ago

Closing due to inactivity. Feel free to reopen.

dwrBad commented 2 years ago

Hi there, I would like to reopen this one, appending a PR https://github.com/StarpTech/apollo-datasource-http/pull/24 Feel free to change or comment on that.

Thank you