From 129c6417bd818bc8b4e1b831644323876e627c13 Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Thu, 22 Oct 2020 00:53:15 +0200 Subject: [PATCH] feat: make Socket#join() and Socket#leave() synchronous 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 - https://github.com/socketio/socket.io-redis/commit/05f926e13ea2beb3635c7426b9b165d034e8a96c Related: https://github.com/socketio/socket.io/issues/3662 --- lib/socket.ts | 23 +++----- package.json | 2 +- test/socket.io.ts | 146 +++++++++++++++++++--------------------------- 3 files changed, 67 insertions(+), 104 deletions(-) diff --git a/lib/socket.ts b/lib/socket.ts index d0d394f78b..1a51c99e81 100644 --- a/lib/socket.ts +++ b/lib/socket.ts @@ -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, fn?: (err: Error) => void): Socket { - debug("joining room %s", rooms); + public join(rooms: Room | Array): Promise | 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 { 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); } /** diff --git a/package.json b/package.json index 327321b85a..7287101255 100644 --- a/package.json +++ b/package.json @@ -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": { diff --git a/test/socket.io.ts b/test/socket.io.ts index 34b78eba77..0e0a823dcf 100644 --- a/test/socket.io.ts +++ b/test/socket.io.ts @@ -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 => { @@ -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() { @@ -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() { @@ -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); @@ -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 => { @@ -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 => { @@ -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", () => { @@ -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)); @@ -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(); }); }); }); @@ -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(); }); }); }); @@ -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(); }); }); }); @@ -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(); }); }); });