search

mistval / node-fetch-cache

Node-fetch with built-in response caching.
MIT License
51 stars 8 forks source link

Support redis? #43

Closed RichardWright closed 2 months ago

RichardWright commented 7 months ago

Hi

Are there any plans to extend this support a redis plugin?

Thanks

mistval commented 7 months ago

Hi,

No specific plans but I do indeed want to create a Redis plugin! It will happen but I can't say when.

campbellmc commented 2 months ago

Hi. We created a Redis custom cache. Works fine and tests all pass. ioredis is an optional dependency - required to use redis as a cache or perform the redis tests. You can see it in this fork in the redis-cache branch: https://github.com/dxclabs/node-fetch-cache/tree/redis-cache Needs some fixes, but if we can get through them, happy to submit as a PR.

mistval commented 2 months ago

Hi @campbellmc , that looks great 🙌 A PR would be very much welcome!

campbellmc commented 2 months ago

Yeah ok.

We have issues which are blocks at the moment.

These are all around having ioredis as an optional dependency:

(1) We detect ioredis being present with await import ... Problem is that the await at top level prevents cjs being built.

(2) we still need ioredis types which are in the ioredis package - doesn't appear possible to install independently

(3) We need to be able to return undefined from the set method in the case that ioredis is not installed. Although README.md says that a return value of undefined is ok, it's not actually in src/types.ts.

So if you ware ok with it, we will make ioredis a dependency (rather than an optional dependency).

It will have an impact on package size, but not a great one:

https://bundlephobia.com/package/ioredis@5.4.1

mistval commented 2 months ago

I think it may be best then to publish it as a separate package, with ioredis as a non-optional dependency. We could either add a plugins folder in this repo and add it there, or it could be an entirely separate repo.

If you want, you could just MR what you have, and then I'd be glad to do the plumbing and move it into a plugins folder and get it published. Or if you would like to publish it yourself in a separate repo, then of course feel free!

campbellmc commented 2 months ago

All yours. 😎

mistval commented 2 months ago

Redis is now supported via the @node-fetch-cache/redis package thanks to @campbellmc and associates.