feat: node-redis

[alesblaznik]

Hi guys 👋🏻! This is mine first contribution to this project so will need some help 🙏🏻 .

We already use this library in production and found #368 where people are interested in node-redis support. Also read the suggestions about refactoring this library to make it easier to extend to other stores, but that would be too much for now.

Understand this isn't the ideal way of adding node-redis, but if we're happy with the first iteration and at least people are able to use node-redis that would be great.

With node-redis, one will need to initialise client like this (will update the docs):

const redis = createClient({
  url: REDIS_HOST,
  scripts: {
    rateLimit: rateLimit.NodeRedisStore.rateLimitScript
  }
})

👆🏻I explored node-redis a bit if there is a way to define script on already connected client (to have the setup same for ioredis and node-redis), but didn't find any solution... will have another round...

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[gurgunday]

This already looks pretty great :)

[alesblaznik]

Need lua script for NodeRedisStore instead of copy&paste. Both of those should always be the same for all Redis clients.

Should we move lua script to other place instead?

[alesblaznik]

Copy pasted tests for ioredis... should rather just reuse tests and run them with other client instead?

[alesblaznik]

This already looks pretty great :)

Will make comments as there are many things we might want to change... just started with the initial PR and can then discuss how we want to do it.

[gurgunday]

My only gripe with this is that we now expose the name of the script while with ioredis we'd load it internally. While the idea of injecting your own script seems cool, it currently seems a little inconsistent to me

Can't we initialize or use the script after the client is created?

.eval seems like an option:

https://github.com/animir/node-rate-limiter-flexible/blob/0840cd7ebee83e9b17b8f27b53b77583dc143076/lib/RateLimiterRedis.js#L121-L125

[alesblaznik]

Agree it's not consistent. I've not seen this eval as a possible solution... will try it out if this will work out and update the PR.

Thanks for the reference 🙏🏻

[mcollina]

I would not add a depedency to @redis/client here. we should be taking an instance of that in the constructor instead.

[gurgunday]

Yeah same as #413 (comment)

The goal is to not depend on the lib

Only the client that's passed should be used