Added binary support to broadcasting of events#1440
Merged
rauchg merged 1 commit intosocketio:masterfrom Mar 3, 2014
Merged
Conversation
lib/namespace.js previously was not considering whether an event had binary data and was giving all events parser.EVENT type -- now it uses the has-binary-data module to set the event type appropriately. Then, after this fix there was a problem with lib/adapter.js -- because socket.io-parser modifies the packet object in its encoding, the sockets after the 1st in a broadcast were not getting the correct data. To fix this, the data is encoded once in adapter, and then the encoded data is passed to each of the sockets. lib/socket.js and lib/client.js were updated to allow for the above. The .packet method of each now takes an optional second "preEncoded" parameter -- if this is true, then client skips the encoding and just writes the packet argument directly to engine. test/socket.io.js was updated to add two new tests that test multi-messaging of events with binary data.
rauchg
added a commit
that referenced
this pull request
Mar 3, 2014
Added binary support to broadcasting of events
This was referenced Apr 26, 2021
This was referenced May 4, 2021
darrachequesne
pushed a commit
that referenced
this pull request
Jul 4, 2024
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.
Summary:
Added binary support to broadcasting by adding a binary check to namespace.js and pre-encoding events in adapter.js.
Question:
I think the pre-encoding stuff in adapter (and the new parameter in client.js) is relatively clean, but it could possibly be better. I was thinking of checking if packet is an array (it is post-encoding) or an object (pre-encoding) rather than passing the "preEncoded" flag, but let me know what you think.
The commit message:
lib/namespace.js previously was not considering whether an event had
binary data and was giving all events parser.EVENT type -- now it uses
the has-binary-data module to set the event type appropriately.
Then, after this fix there was a problem with lib/adapter.js -- because
socket.io-parser modifies the packet object in its encoding, the sockets
after the 1st in a broadcast were not getting the correct data. To fix
this, the data is encoded once in adapter, and then the encoded data
is passed to each of the sockets.
lib/socket.js and lib/client.js were updated to allow for the above. The
.packet method of each now takes an optional second "preEncoded" parameter
-- if this is true, then client skips the encoding and just writes the
packet argument directly to engine.
test/socket.io.js was updated to add two new tests that test
multi-messaging of events with binary data.