Add option to allow usage of custom parser#121
Closed
MasterOdin wants to merge 1 commit intosocketio:mainfrom
MasterOdin:feat-custom-parser
Closed
Add option to allow usage of custom parser#121MasterOdin wants to merge 1 commit intosocketio:mainfrom MasterOdin:feat-custom-parser
MasterOdin wants to merge 1 commit intosocketio:mainfrom
MasterOdin:feat-custom-parser
Conversation
Signed-off-by: Matthew Peveler <[email protected]>
Contributor
Author
|
@darrachequesne a friendly bump on this PR as I appreciate seeing socketio/socket.io-redis-adapter#471 be merged, I also need this one merged so I can use it in my own project. 😄 |
darrachequesne
pushed a commit
that referenced
this pull request
Jan 12, 2023
This PR adds a new parser option to the adapter constructor to allow setting a custom parser to use, defaulting to using notepack.io. This would allow someone to use a different msgpack library if they wanted, or even an entirely different protocol altogether (e.g. protobuf). Related: socketio/socket.io-redis-adapter@73f6320 Signed-off-by: Matthew Peveler <[email protected]>
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sibling PR to socketio/socket.io-redis-adapter#471. This adds the same
parseroption here so that can plug in an alternative parser to encode messages to redis. See the other PR and its associated issue for more context.One thing of note is that this is technically not BC due to modifying the
broadcastOptionsto requireparserand thatBroadcastOperatoris exported, so if someone was importing it themselves for some reason, then they'd need to update their usage. However, I'm not sure how common a use case that is or how many people would be affected for this to warrant much concern. An alternative BC approach would be to makeparseroptional tobroadcastOptionsand then set it to the default within the constructor, but I rejected that approach initially just to keep the code as similar as it currently is.