Skip to content

Commit

Permalink
feat: make Socket#join() and Socket#leave() synchronous
Browse files Browse the repository at this point in the history 
Depending on the adapter, Socket#join() may return:

- nothing (in-memory and Redis adapters)
- a promise (custom adapters)

Breaking change: Socket#join() and Socket#leave() do not accept a
callback argument anymore.

Before:

```js
socket.join("room1", () => {
 io.to("room1").emit("hello");
});
```

After:

```
socket.join("room1");
io.to("room1").emit("hello");
// or await socket.join("room1"); for custom adapters
```

Note: the need for an asynchronous method came from the Redis adapter,
which did override the Adapter#add() method in earlier versions, but
this is not the case anymore.

Reference:

- https://github.com/socketio/socket.io/blob/2.3.0/lib/socket.js#L236-L258
- https://github.com/socketio/socket.io-adapter/blob/1.1.2/index.js#L56-L65
- socketio/socket.io-redis-adapter@05f926e

Related: #3662
  • Loading branch information
@darrachequesne
darrachequesne committed Oct 21, 2020
1 parent 0d74f29 commit 129c641
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 104 deletions.
23 changes: 7 additions & 16 deletions lib/socket.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,38 +238,29 @@ export class Socket extends EventEmitter {
* Joins a room.
*
* @param {String|Array} rooms - room or array of rooms
* @param {Function} fn - optional, callback
* @return {Socket} self
* @return a Promise or nothing, depending on the adapter
* @public
*/
public join(rooms: Room | Array<Room>, fn?: (err: Error) => void): Socket {
debug("joining room %s", rooms);
public join(rooms: Room | Array<Room>): Promise<void> | void {
debug("join room %s", rooms);

this.adapter.addAll(
return this.adapter.addAll(
this.id,
new Set(Array.isArray(rooms) ? rooms : [rooms])
);
debug("joined room %s", rooms);
fn && fn(null);
return this;
}

/**
* Leaves a room.
*
* @param {String} room
* @param {Function} fn - optional, callback
* @return {Socket} self
* @return a Promise or nothing, depending on the adapter
* @public
*/
public leave(room: string, fn?: (err: Error) => void): Socket {
public leave(room: string): Promise<void> | void {
debug("leave room %s", room);
this.adapter.del(this.id, room);

debug("left room %s", room);
fn && fn(null);

return this;
return this.adapter.del(this.id, room);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"base64id": "~2.0.0",
"debug": "~4.1.0",
"engine.io": "~4.0.0",
"socket.io-adapter": "2.0.3-rc1",
"socket.io-adapter": "2.0.3-rc2",
"socket.io-parser": "4.0.1-rc2"
},
"devDependencies": {
Expand Down
146 changes: 59 additions & 87 deletions test/socket.io.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,9 @@ describe("socket.io", () => {
const socket = client(srv);

sio.on("connection", s => {
s.join("a", () => {
// FIXME not sure why process.nextTick() is needed here
process.nextTick(() => s.disconnect());
});
s.join("a");
// FIXME not sure why process.nextTick() is needed here
process.nextTick(() => s.disconnect());

let total = 2;
s.on("disconnecting", reason => {
Expand Down Expand Up @@ -578,22 +577,19 @@ describe("socket.io", () => {
let total = 3;
sio.of("/chat").on("connection", socket => {
if (chatIndex++) {
socket.join("foo", () => {
chatFooSid = socket.id;
--total || getSockets();
});
socket.join("foo");
chatFooSid = socket.id;
--total || getSockets();
} else {
socket.join("bar", () => {
chatBarSid = socket.id;
--total || getSockets();
});
socket.join("bar");
chatBarSid = socket.id;
--total || getSockets();
}
});
sio.of("/other").on("connection", socket => {
socket.join("foo", () => {
otherSid = socket.id;
--total || getSockets();
});
socket.join("foo");
otherSid = socket.id;
--total || getSockets();
});
});
async function getSockets() {
Expand Down Expand Up @@ -623,22 +619,19 @@ describe("socket.io", () => {
let total = 3;
sio.of("/chat").on("connection", socket => {
if (chatIndex++) {
socket.join("foo", () => {
chatFooSid = socket.id;
--total || getSockets();
});
socket.join("foo");
chatFooSid = socket.id;
--total || getSockets();
} else {
socket.join("bar", () => {
chatBarSid = socket.id;
--total || getSockets();
});
socket.join("bar");
chatBarSid = socket.id;
--total || getSockets();
}
});
sio.of("/other").on("connection", socket => {
socket.join("foo", () => {
otherSid = socket.id;
--total || getSockets();
});
socket.join("foo");
otherSid = socket.id;
--total || getSockets();
});
});
async function getSockets() {
Expand Down Expand Up @@ -1711,20 +1704,6 @@ describe("socket.io", () => {
});
});

it("should always trigger the callback (if provided) when joining a room", done => {
const srv = createServer();
const sio = new Server(srv);

srv.listen(() => {
const socket = client(srv);
sio.on("connection", s => {
s.join("a", () => {
s.join("a", done);
});
});
});
});

it("should throw on reserved event", done => {
const srv = createServer();
const sio = new Server(srv);
Expand Down Expand Up @@ -1861,13 +1840,13 @@ describe("socket.io", () => {
socket1.on("a", () => {
done();
});
socket1.emit("join", "woot", () => {
socket1.emit("emit", "woot");
});
socket1.emit("join", "woot");
socket1.emit("emit", "woot");

sio.on("connection", socket => {
socket.on("join", (room, fn) => {
socket.join(room, fn);
socket.join(room);
fn && fn();
});

socket.on("emit", room => {
Expand Down Expand Up @@ -1904,7 +1883,8 @@ describe("socket.io", () => {

sio.on("connection", socket => {
socket.on("join", (room, fn) => {
socket.join(room, fn);
socket.join(room);
fn && fn();
});

socket.on("emit", room => {
Expand Down Expand Up @@ -1949,7 +1929,8 @@ describe("socket.io", () => {

sio.on("connection", socket => {
socket.on("join", (room, fn) => {
socket.join(room, fn);
socket.join(room);
fn && fn();
});

socket.on("broadcast", () => {
Expand Down Expand Up @@ -1996,7 +1977,8 @@ describe("socket.io", () => {

sio.on("connection", socket => {
socket.on("join", (room, fn) => {
socket.join(room, fn);
socket.join(room);
fn && fn();
});
socket.on("broadcast", () => {
socket.broadcast.to("test").emit("bin", Buffer.alloc(5));
Expand All @@ -2013,21 +1995,17 @@ describe("socket.io", () => {
srv.listen(() => {
const socket = client(srv);
sio.on("connection", s => {
s.join("a", () => {
expect(s.rooms).to.contain(s.id, "a");
s.join("b", () => {
expect(s.rooms).to.contain(s.id, "a", "b");
s.join("c", () => {
expect(s.rooms).to.contain(s.id, "a", "b", "c");
s.leave("b", () => {
expect(s.rooms).to.contain(s.id, "a", "c");
s.leaveAll();
expect(s.rooms.size).to.eql(0);
done();
});
});
});
});
s.join("a");
expect(s.rooms).to.contain(s.id, "a");
s.join("b");
expect(s.rooms).to.contain(s.id, "a", "b");
s.join("c");
expect(s.rooms).to.contain(s.id, "a", "b", "c");
s.leave("b");
expect(s.rooms).to.contain(s.id, "a", "c");
s.leaveAll();
expect(s.rooms.size).to.eql(0);
done();
});
});
});
Expand All @@ -2039,13 +2017,11 @@ describe("socket.io", () => {
srv.listen(() => {
const socket = client(srv);
sio.on("connection", s => {
s.join("a", () => {
expect(s.nsp.adapter.rooms).to.contain("a");
s.leave("a", () => {
expect(s.nsp.adapter.rooms).to.not.contain("a");
done();
});
});
s.join("a");
expect(s.nsp.adapter.rooms).to.contain("a");
s.leave("a");
expect(s.nsp.adapter.rooms).to.not.contain("a");
done();
});
});
});
Expand All @@ -2057,18 +2033,15 @@ describe("socket.io", () => {
srv.listen(() => {
const socket = client(srv);
sio.on("connection", s => {
s.join("a", () => {
expect(s.rooms).to.contain(s.id, "a");
s.join("b", () => {
expect(s.rooms).to.contain(s.id, "a", "b");
s.leave("unknown", () => {
expect(s.rooms).to.contain(s.id, "a", "b");
s.leaveAll();
expect(s.rooms.size).to.eql(0);
done();
});
});
});
s.join("a");
expect(s.rooms).to.contain(s.id, "a");
s.join("b");
expect(s.rooms).to.contain(s.id, "a", "b");
s.leave("unknown");
expect(s.rooms).to.contain(s.id, "a", "b");
s.leaveAll();
expect(s.rooms.size).to.eql(0);
done();
});
});
});
Expand All @@ -2080,10 +2053,9 @@ describe("socket.io", () => {
srv.listen(() => {
const socket = client(srv);
sio.on("connection", s => {
s.join(["a", "b", "c"], () => {
expect(s.rooms).to.contain(s.id, "a", "b", "c");
done();
});
s.join(["a", "b", "c"]);
expect(s.rooms).to.contain(s.id, "a", "b", "c");
done();
});
});
});
Expand Down

0 comments on commit 129c641

Please sign in to comment.