got icon indicating copy to clipboard operation
got copied to clipboard

Enhance the `cache` option

Open szmarczak opened this issue 4 years ago • 28 comments

Issuehunt badges

The cache mechanism:

  • [x] should be a promise
  • [x] should be replaceable with any other module*
  • [ ] should have something hook-like to allow storing different properties e.g. ip addresses
  • [ ] should accept different caching logic
  • [ ] should make a new request when the cached response cannot be updated (#963)
  • [ ] beforeCache hook

* Through the request option. Accepting ClientRequest and IncomingMessage as the return type. Return undefined to fallback. You can override the request option also by returning a ClientRequest or an IncomingMessage instance in a beforeRequest hook.

szmarczak avatar Sep 16 '19 21:09 szmarczak

About #746: What about the approach of adding an hook that can override the headers (including the caching ones), before they are evaluated by cacheable-request ? This way you can set different caching headers, tweaking the cache key (etag or even the url) etc.

sithmel avatar Feb 15 '20 23:02 sithmel

@sithmel beforeCache hook? Sounds good to me.

szmarczak avatar Feb 16 '20 09:02 szmarczak

Happy to implement it as soon as this is done and merged

sithmel avatar Feb 16 '20 11:02 sithmel

@szmarczak I guess I should wait for https://github.com/sindresorhus/got/pull/1051 to be merged ... do you agree ?

sithmel avatar Feb 18 '20 08:02 sithmel

@sithmel You don't have to. Feel free to send a PR anytime, I'll merge with master later. No worries! :)

szmarczak avatar Feb 18 '20 08:02 szmarczak

Can i add another suggest feature which is kind of related to being a promise #1078

MrSwitch avatar Feb 19 '20 12:02 MrSwitch

i've just read ky docs https://www.npmjs.com/package/ky#hooksbeforerequest

The hook can return a Request to replace the outgoing request, or return a Response to completely avoid making an HTTP request. This can be used to mock a request, check an internal cache, etc

sounds quite similar for https://github.com/sindresorhus/got/issues/746 needs

rifler avatar Jun 18 '20 19:06 rifler

@rifler It's already done. It hasn't been documented yet.

szmarczak avatar Jul 04 '20 14:07 szmarczak

@rifler 779062a1b99a9a1c13737b9fd613e185d97b158b

szmarczak avatar Jul 04 '20 14:07 szmarczak

It would be great if you can also write how can we give custom request. It's still not clear from Readme.

adityapatadia avatar Jul 04 '20 15:07 adityapatadia

@adityapatadia http.request returns a ClientRequest instance for example. I will write a more detailed tutorial soon.

szmarczak avatar Jul 04 '20 16:07 szmarczak

@adityapatadia http.request returns a ClientRequest instance for example. I will write a more detailed tutorial soon.

I understand the return type but I am not sure how can I return custom request function.

Do you mean something like this?


beforeRequest: [
			async options => {
				return new CacheableRequest(http.request);
			}
		]

adityapatadia avatar Jul 04 '20 16:07 adityapatadia

No. I mean ClientRequest, not CacheableRequest:

const got = require('got');
const https = require('https');

got('https://example.com', {
    hooks: {
        beforeRequest: [
            () => {
				// `https.request` returns a ClientRequest instance
                return https.request('https://httpbin.org/anything');
            }
        ]
    }
}).json();

szmarczak avatar Jul 04 '20 18:07 szmarczak

You can pass an IncomingMessage instance as well. To generate one you can use for example https://github.com/lukechilds/responselike

szmarczak avatar Jul 04 '20 18:07 szmarczak

Understood. Thanks for help.

adityapatadia avatar Jul 04 '20 18:07 adityapatadia

No problem, if you got any other questions, feel free to ask :)

szmarczak avatar Jul 04 '20 19:07 szmarczak

May I know if #746 is possible yet? When remote host returns headers with {"cache-control": "no-cache"}, is there a way to override it without adding beforeRequest and afterResponse hooks to implement custom caching? Thank you very much!

hk7math avatar Oct 30 '20 08:10 hk7math

No problem, if you got any other questions, feel free to ask :)

Sorry to get back after long time but I tried this and it throws many errors related to agent and timeout.


beforeRequest: [async options => {
        return new Promise((resolve, reject) => {
          let cr;
          if(options.url.protocol == "https:"){
            cr = new cacheableRequest(https.request);
          } else {
            cr = new cacheableRequest(http.request);
          }
          cr(options).on("request", resolve);
        });
      }],

Basically there is no easy way to just edit some part of cacheableRequest module and then plug it in this library.

adityapatadia avatar Jan 17 '21 18:01 adityapatadia

@adityapatadia You're creating a new CacheableRequest instance on every got call. You can do this instead:

const runCachedHttps = new CacheableRequest(https.request);
const runCachedHttp = new CacheableRequest(http.request);

...

beforeRequest: [async options => {
        return new Promise((resolve, reject) => {
          const cr = options.url.protocol == "https:" ? runCachedHttps : runCachedHttp;
          const instance = cr(options);
		  instance.on("request", resolve);
		  instance.on("error", reject);
        });
      }],

...

szmarczak avatar Jan 17 '21 19:01 szmarczak

Anything we can do to push this feature further?

StarpTech avatar Jun 15 '21 07:06 StarpTech

I had an issue where I couldn't trust the cache-control headers so edited them before saving the cached response https://github.com/gatsbyjs/gatsby/pull/32012

KyleAMathews avatar Jun 21 '21 23:06 KyleAMathews

I have started working on a new cache implementation. I will let you know when I have a working example.

szmarczak avatar Aug 01 '21 18:08 szmarczak

Update: more than 50% RFC-compliant things finished.

szmarczak avatar Aug 02 '21 21:08 szmarczak

Update: I just need to finish revalidation, conditional requests, tests and we're all set ✨

szmarczak avatar Aug 05 '21 22:08 szmarczak

any news on this? :)

chris-schra avatar Dec 02 '21 08:12 chris-schra

Sorry, been busy w/ work and uni. Will work on this asap.

szmarczak avatar Dec 10 '21 19:12 szmarczak

Any progress on this?

I just want to be able to always cache a response...is there any hook I can use to do this?

vjpr avatar Apr 26 '22 17:04 vjpr

Any progress on this?

I just want to be able to always cache a response...is there any hook I can use to do this?

The same requirement.

wilsonssss avatar Oct 27 '23 05:10 wilsonssss