epicweb-dev / cachified

🤑 wrap virtually everything that can store by key to act as cache with ttl/max-age, stale-while-validate, parallel fetch protection and type-safety support
MIT License
916 stars 26 forks source link

Add support for redis-json #45

Closed tearingItUp786 closed 10 months ago

tearingItUp786 commented 1 year ago

Hey there friend,

Would you be interested in adding support for RedisJson in cachified? Regarding implementations, I think we can pass in an additional argument to the createRedisAdapter (an options object) or create another adapter! Let me know your thoughts 😊.

Example code:

// default to false for enableRedisJson
export declare function redisCacheAdapter(redisCache: RedisLikeCache, options?: { enableRedisJson?: boolean }): Cache;
Xiphe commented 1 year ago

Hi @tearingItUp786 thanks for reaching out!

I have not worked with redis-json, I assume you mean the redis-json package right?

PRs for this are very welcome!

I'm not entirely sure if it fits into the existing redisCacheAdapter and if the options are a good Idea. What would happen when I pass the default redisClient with enableRedisJson: true? or the other way round?

If this turns out not to be an issue, go ahead and add this to the adapter. Otherwise there's nothing wrong with providing a dedicated redisJsonCacheAdapter adapter.

Before a PR can be merged we need to ensure that

Let me know when I can help with anything

tearingItUp786 commented 1 year ago

Hey there @Xiphe !

I was thinking of the @redis/json npm package!

I see your point about extending the old redisCacheAdapter; I'm game to create a new adapter! I'll make a PR sometime this week 😄.

tearingItUp786 commented 1 year ago

So it seems that redis-mock doesn't have support for the json module that ships with@redis/json. Do you see any issues with me doing this in my tests @Xiphe :

  const redis: RedisJsonCache = {
    json: {
      set: jest.fn(),
      get: jest.fn(),
      del: jest.fn(),
    },
    expireAt: jest.fn(),
  };
  const cache = redisJsonCacheAdapter(redis);
  // some tests
Xiphe commented 1 year ago

No, this is totally fine with me. It's not chachifieds responsibility to test the integration of the caches. The goal is to use what's the easiest for the tests.

That said. Would be great if the RedisJsonCache interface comes from redis-json so we ensure type parity of this mock.