-
Notifications
You must be signed in to change notification settings - Fork 485
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
Allow setting custom msgpack library to use #469
Comments
That sounds reasonable 👍 Minor nitpick: I don't think the interface should include "msgpack" in its name, as any format would work (protobuf for example). |
Does |
Yes that sounds great 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I'd like to add in the ability to swap in a custom
msgpack
library to use. I'm making a proposal first before opening a PR just to see if there's support. This would somewhat mirror the existing capacity to bring your own packet parser to socket.io.My proposal for the change would be to add a new key/value to
RedisAdapterOptions
of something like:and then in the
RedisAdapter
constructor, have something likethis.msgpack = opts.msgpack || msgpack
, and then any future call tomsgpack
in the class would be replaced withthis.msgpack
. I've loosely tested this within my own application and haven't seen anything break, but I may be missing something on how this is used.I'd make a similar change for the
@socket.io/redis-emitter
library as well.For the motivation for this change, this library currently uses the
notepack.io
library to handle encoding/decoding messages into/out of redis. In benchmarking our application, we are finding thatmsgpackr
library would give us a decent speed-up for the sorts of payloads we have (We also already use it in a number of other places in our stack):As such, I'd like to have the ability to provide my own msgpack implementation to use for the adapter.
The text was updated successfully, but these errors were encountered: