Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for node-redis @4 (next to @3) as well #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WietseWind
Copy link

@WietseWind WietseWind commented Jul 3, 2023

This package supported node-redis (redis) version 3. Version 4 has some breaking changes.

This PR adds support for redis@4 while staying backwards compatible with the node redis client v3 & ioredis.

The redis@4 package is aliassed as redis4 and a separate test case is added.

@@ -33,11 +33,25 @@ class RollingLimit {
if(!/:$/.test(this.prefix)) this.prefix += ':';
this.force = options.force ? 'true' : 'false';
if (!this.redis.evalshaAsync) {
if (this.redis.Promise) {
if (typeof this.redis.executeScript !== 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a different way we can check this? Seems brittle

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't found something very straight forward to check, but I did find the methods to interact with LUA scripts to be changed, with the change you commented on as the difference between the two versions. I'm sure there will be other minor API changes, but I didn't find something as explicit as you'd wish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants