-
Notifications
You must be signed in to change notification settings - Fork 18
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
Conversation
If we already making in breaking maybe it is good time to rewrite it to typescript and use normal classes? |
@dawidreedsy feels way out of scope of this change |
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... |
Okay. Coverage fixed. Got caught out by the fact that |
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
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 ofsharedb-redis-pubsub
to v4.This change adapts to the breakages:
Promise
s in callbacksclient
, which no longer auto-connectsmessage
event handler into the subscription callbackclient.eval()