arthurfiorette / axios-cache-interceptor

📬 Small and efficient cache interceptor for axios. Etag, Cache-Control, TTL, HTTP headers and more!
https://axios-cache-interceptor.js.org
MIT License
703 stars 58 forks source link

Custom storage > Must save all states #846

Closed mihai-stancu closed 5 months ago

mihai-stancu commented 5 months ago

Hi,

I wanted to suggest a warning in the documentation for custom storage examples. I would stress the point that the backend storage is expected to store all cache requests -- the full object, regardless of the object state -- and that the examples are not simplified for clarity.


My sad tale of acting smart:

set: async (key, {state, data}, cfg) => !!data ? await redis.set(key, JSON.stringify(data), {EX: cfg.cache.ttl}) : null

For some reason I "intuited" that the value of the state property is something I have to deal with.

I also bravely assumed that the example in the documentation was simplified for convenience.


My erroneous conclusion was that I need to skip saving the data in some combination of values of state and/or absence of data.

I assumed that the state value is very relevant when more complex usecases of caching are needed (cached/stale/expired states; expiry handled in code; etc.).

Then I spent (more) time (than I am willing to admit) debugging why this is happening.

Even after I caught on to the problem I still assumed I have to save the cacheRequest.data not the full cacheRequest -- at least this one was easy to fix.

arthurfiorette commented 5 months ago

If it were for you to code the caching logic, why would I have created this library? You over complicated things, by a LOT.


After just a bit of scrolling in the storage doc page, you would find this example on how to implement your own custom storage (using a simply key=value library that doesn't matter for our usecase):

https://axios-cache-interceptor.js.org/guide/storages#indexeddb

import axios from 'axios';
import { buildStorage } from 'axios-cache-interceptor';
import { clear, del, get, set } from 'idb-keyval';

const indexedDbStorage = buildStorage({
  async find(key) {
    const value = await get(key);

    if (!value) {
      return;
    }

    return JSON.parse(value);
  },

  async set(key, value) {
    await set(key, JSON.stringify(value));
  },

  async remove(key) {
    await del(key);
  }
});

As you can see, nowhere it says nor asks you to code anything, you just have to pass it to your storage system.

There also a lot of other examples and none of them asks you to do what you've done.

mihai-stancu commented 5 months ago

Hi,

The library is awesome and the documentation is 100% correct. The issue was only referencing my bad assumptions.

If those assumptions don't ring a bell then of course let's not waste energy.


You over complicated things, by a LOT.

Indeed, most of the cache driver implementations I've seen lean on driver to smooth over feature parity.

That allows the abstract cache to rely on all drivers exhibiting the same features and leans on the drivers to manage missing features.

Ex.: I've seen many in memory drivers that take the responsibility of expiry because they're the "odd one out" that doesn't support it while Memcached & Redis do.