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

💥 Bump redis to v4 #23

Merged
merged 1 commit into from
Feb 13, 2024
Merged

💥 Bump redis to v4 #23

merged 1 commit into from
Feb 13, 2024

Conversation

alecgibson
Copy link
Contributor

Fixes #19

This is a BREAKING change that upgrades the underlying redis client to v4, which had so many breaking changes, that it's infeasible to maintain backwards-compatibility with v3. If consumers need to use v3, they are advised to pin their version of sharedb-redis-pubsub to v4.

This change adapts to the breakages:

  • wrap the returned Promises in callbacks
  • actively connect the client, which no longer auto-connects
  • move the message event handler into the subscription callback
  • adapt to the new call signature of client.eval()

@dawidreedsy
Copy link

If we already making in breaking maybe it is good time to rewrite it to typescript and use normal classes?

@alecgibson
Copy link
Contributor Author

@dawidreedsy feels way out of scope of this change

@alecgibson
Copy link
Contributor Author

Cannot work out why the coverage is failing here :/

Can repro locally, too, but I've not touched anything to do with the coverage machinery...

@coveralls
Copy link

coveralls commented Feb 8, 2024

Coverage Status

coverage: 100.0%. remained the same
when pulling 6708c9f on redis-v4
into fa5921e on master.

@alecgibson
Copy link
Contributor Author

Okay. Coverage fixed. Got caught out by the fact that arguments is a sneaky reserved-but-not-actually-reserved word 🤦🏼‍♂️

@curran
Copy link
Contributor

curran commented Feb 9, 2024

Excellent! Thanks for doing this.

Fixes #19

This is a **BREAKING** change that upgrades the underlying `redis`
client to v4, which had so many [breaking changes][1], that it's
infeasible to maintain backwards-compatibility with v3. If consumers
need to use v3, they are advised to pin their version of
`sharedb-redis-pubsub` to v4.

This change adapts to the breakages:

 - wrap the returned `Promise`s in callbacks
 - actively connect the `client`, which no longer auto-connects
 - move the `message` event handler into the subscription callback
 - adapt to the new call signature of `client.eval()`

[1]: https://github.com/redis/node-redis/blob/HEAD/docs/v3-to-v4.md
@alecgibson alecgibson merged commit baae5e5 into master Feb 13, 2024
8 checks passed
@alecgibson alecgibson deleted the redis-v4 branch February 13, 2024 17:50
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.

Support redis v4
5 participants