Caching strategies

[bpolaszek]

Hey there,

Thanks for the good work! I needed cache management with Axios and this library does a good job.

I'm not a Javascript expert but if I may suggest some enhancements:

• Caching strategies:
  • Native (read Cache-Control and Vary headers) - would be the default behavior (major version bump) - because that's what we expect at first when using a cache adapter
  • Fixed (a fixed TTL like the current behavior)
  • Custom (By request/response matcher: native or fixed)
• Include query params by default - see #31
• Canonicalize URL (with a default canonicalizer)
• (Completely) Ignore non-GET requests by default *
• Optionally allow other methods
• TTL unit should be seconds, not milliseconds

* Currently a POST request matching the same URL than a GET request results in a cache item removal. IMHO non-allowed methods (default to non-GET) should be completely discarded from both read and writes for 3 reasons:

• The developer may enable cache on POST for some reasons (sometimes you didn't design the API you're calling and you're likely to kill its author)
• Currently GET /foos, POST /foos and GET /foos results in 3 cache MISS, even if POST /foos failed.
• Even if POST /foos succeeds, it is the developer's responsibility to decide whether or not POST /foo should invalidate GET /foos (then they could also decide that PUT /foo both invalidates GET /foo and GET /foos, for instance). The library should not enforce that and be completely blind from non-allowed-methods calls.

What do you think?

[ghost]

Hi @bpolaszek

Thanks a lot for your interest and feedback!

TL;DR I completely agree with all your suggestions 😄

• Caching strategies

Very interesting to use the cache headers by default and then let the developer override it with a fixed TTL.

• Include query params by default

Yes I guess it would make more sense but only if the cache invalidation is not automatic like it is now. The current default behavior was what was needed in the first app I developed that used this library, not relevant anymore.

• Canonicalize URL

Absolutely needed yes 😄

• (Completely) Ignore non-GET requests by default *
• Optionally allow other methods

Makes sense as well if we remove the automatic cache invalidation on POST requests

• TTL unit should be seconds, not milliseconds

Why is this so? Is it the way servers express it? My knowledge on cache falls short here.

All these are breaking changes to the current behavior so should be released in a major version.

Just being curious, how do you use axios-cache-adapter? on the client, server or even both?

If you'd like to contribute to making this shift I'd be delighted to help.

Cheers

[bpolaszek]

Hi @RasCarlito,

Thank you for your feedback!

I use it on the client side (my stack is PHP for the back-end, VueJS for the front).

Commonly cache expirations are expressed in seconds (see here) - and I honestly don't see a use case where the cache should invalidate at a specific millisecond.

All these are big breaking changes indeed and should be heavily documented 🙂

Unfortunately I'm not a very good JS coder, but I contributed on a similar package for a PHP library, so don't hesitate to ping me if I can help in architecting this.

[bpolaszek]

Just looked at how the Cache API works, it can be a good start for abstracting a Native strategy + a Browser storage (because it won't work in Node):

• It already parses the Cache-Control header
• It parses the Vary header
• It has an option to ignore the query string
• It has an option to ignore the request method

Related: #53

[ghost]

Should be nice indeed! Where did you find documentation about Cache API parsing Cache-Control and Vary headers? I can't seem to find it 😞

[bpolaszek]

TBH I didn't find the explicit information, and this deserves to be tested. I guessed this because the cache API has a ignoreVary option, which means it reads the Vary header by default, which only makes sense if it reads Cache-Control first, IMO. And it has no TTL option so I really doubt it doesn't rely on response headers, otherwise there's no way to invalidate the cache.

[jahead]

@bpolaszek

Currently a POST request matching the same URL than a GET request results in a cache item removal. IMHO non-allowed methods (default to non-GET) should be completely discarded from both read and writes for 3 reasons: ...

• Even if POST /foos succeeds, it is the developer's responsibility to decide whether or not POST /foo should invalidate GET /foos (then they could also decide that PUT /foo both invalidates GET /foo and GET /foos, for instance). The library should not enforce that and be completely blind from non-allowed-methods calls

Has been fixed in the latest build. Allowing you to override the invalidate function.