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

Support compression #1958

Merged
merged 2 commits into from
Jan 23, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
support compression
  • Loading branch information
nkzawa committed Jan 21, 2015
commit ac8e8598d7dea67fbe5347a2b45e6f92d68df025
12 changes: 12 additions & 0 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,18 @@ server.listen(3000);
});
```

### Socket#compress(v:Boolean):Socket

Sets a modifier for a subsequent event emission that the event data will
only be _compressed_ if the value is `true`. Defaults to `true` when you don't call the method.

```js
var io = require('socket.io')();
io.on('connection', function(socket){
socket.compress(false).emit('an event', { some: 'data' });
});
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beautiful


### Client

The `Client` class represents an incoming transport (engine.io)
Expand Down
7 changes: 4 additions & 3 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ Client.prototype.setup = function(){
Client.prototype.connect = function(name){
debug('connecting to namespace %s', name);
if (!this.server.nsps[name]) {
this.packet({ type: parser.ERROR, nsp: name, data : 'Invalid namespace'});
this.packet({ type: parser.ERROR, nsp: name, data : 'Invalid namespace'}, false, false, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to reconsider this signature later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I also don't like this. options object would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a packet should have these options, so that we can make it isomorphic with the client.

https://github.com/Automattic/socket.io-client/pull/805/files#diff-9b2f90c89d460ee80ced18e01748824eR139

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave this for a different PR @nkzawa

return;
}
var nsp = this.server.of(name);
Expand Down Expand Up @@ -135,17 +135,18 @@ Client.prototype.close = function(){
* @param {Object} packet object
* @param {Boolean} whether packet is already encoded
* @param {Boolean} whether packet is volatile
* @param {Boolean} whether packet should be compressed
* @api private
*/

Client.prototype.packet = function(packet, preEncoded, volatile){
Client.prototype.packet = function(packet, preEncoded, volatile, compress){
var self = this;

// this writes to the actual connection
function writeToEngine(encodedPackets) {
if (volatile && !self.conn.transport.writable) return;
for (var i = 0; i < encodedPackets.length; i++) {
self.conn.write(encodedPackets[i]);
self.conn.write(encodedPackets[i], { compress: compress });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, is this the same signature the WS client would use? I mention this since it'd be nice to maintain parity so that you could technically replace engine.io-client with a WebSocket client altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, ws module has the same signature, but it's not something formal since WS API for browser has no spec for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but that's good enough then…

}
}

Expand Down
17 changes: 16 additions & 1 deletion lib/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ Socket.prototype.write = function(){
Socket.prototype.packet = function(packet, preEncoded){
packet.nsp = this.nsp.name;
var volatile = this.flags && this.flags.volatile;
this.client.packet(packet, preEncoded, volatile);
var compress = !this.flags || false !== this.flags.compress;
this.client.packet(packet, preEncoded, volatile, compress);
};

/**
Expand Down Expand Up @@ -442,3 +443,17 @@ Socket.prototype.disconnect = function(close){
}
return this;
};

/**
* Sets the compress flag.
*
* @param {Boolean} if `true`, compresses the sending data
* @return {Socket} self
* @api public
*/

Socket.prototype.compress = function(compress){
this.flags = this.flags || {};
this.flags.compress = compress;
return this;
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"test": "mocha --reporter dot --slow 200ms --bail"
},
"dependencies": {
"engine.io": "1.5.1",
"engine.io": "Automattic/engine.io",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's point to a specific #commit

"socket.io-parser": "2.2.2",
"socket.io-client": "1.3.2",
"socket.io-adapter": "0.3.1",
Expand Down
30 changes: 30 additions & 0 deletions test/socket.io.js
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,36 @@ describe('socket.io', function(){
});
});
});

it('should enable compression by default', function(done){
var srv = http();
var sio = io(srv);
srv.listen(function(){
var socket = client(srv);
sio.on('connection', function(s){
s.conn.once('packetCreate', function(packet) {
expect(packet.options.compress).to.be(true);
done();
});
s.emit('woot', 'hi');
});
});
});

it('should disable compression', function(done){
var srv = http();
var sio = io(srv);
srv.listen(function(){
var socket = client(srv);
sio.on('connection', function(s){
s.conn.once('packetCreate', function(packet) {
expect(packet.options.compress).to.be(false);
done();
});
s.compress(false).emit('woot', 'hi');
});
});
});
});

describe('messaging many', function(){
Expand Down