Skip to content

Commit

Permalink
refactor: remove duplicate _sockets map
Browse files Browse the repository at this point in the history
Both the "connected" and the "_sockets" maps were used to track the
Socket instances in the namespace.

Let's merge them into "sockets". It's a breaking change, but:

- the "sockets" object did already exist in Socket.IO v2 (and appears in some examples/tutorials)
- "sockets" makes more sense than "connected" in my opinion
- there was already a breaking change regarding the "connected" property (from object to Map)

Breaking change: the "connected" map is renamed to "sockets"
  • Loading branch information
darrachequesne committed Oct 15, 2020
1 parent 2a05042 commit 8a5db7f
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 11 deletions.
2 changes: 1 addition & 1 deletion lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ export class Server extends EventEmitter {
* @public
*/
public close(fn?: (err?: Error) => void): void {
for (const socket of this.sockets._sockets.values()) {
for (const socket of this.sockets.sockets.values()) {
socket._onclose("server shutting down");
}

Expand Down
11 changes: 4 additions & 7 deletions lib/namespace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const debug = debugModule("socket.io:namespace");

export class Namespace extends EventEmitter {
public readonly name: string;
public readonly connected: Map<SocketId, Socket> = new Map();
public readonly sockets: Map<SocketId, Socket> = new Map();

public adapter: Adapter;

Expand All @@ -29,9 +29,6 @@ export class Namespace extends EventEmitter {
/** @private */
_ids: number = 0;

/** @private */
_sockets: Map<SocketId, Socket> = new Map();

/**
* Namespace constructor.
*
Expand Down Expand Up @@ -135,7 +132,7 @@ export class Namespace extends EventEmitter {
if (err) return socket._error(err.message);

// track socket
this._sockets.set(socket.id, socket);
this.sockets.set(socket.id, socket);

// it's paramount that the internal `onconnect` logic
// fires before user-set events to prevent state order
Expand All @@ -161,8 +158,8 @@ export class Namespace extends EventEmitter {
* @private
*/
_remove(socket: Socket): void {
if (this._sockets.has(socket.id)) {
this._sockets.delete(socket.id);
if (this.sockets.has(socket.id)) {
this.sockets.delete(socket.id);
} else {
debug("ignoring remove for %s", socket.id);
}
Expand Down
2 changes: 0 additions & 2 deletions lib/socket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ export class Socket extends EventEmitter {
*/
_onconnect(): void {
debug("socket connected - writing packet");
this.nsp.connected.set(this.id, this);
this.join(this.id);
this.packet({ type: PacketType.CONNECT, data: { sid: this.id } });
}
Expand Down Expand Up @@ -430,7 +429,6 @@ export class Socket extends EventEmitter {
this.client._remove(this);
this.connected = false;
this.disconnected = true;
this.nsp.connected.delete(this.id);
super.emit("disconnect", reason);
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"base64id": "~2.0.0",
"debug": "~4.1.0",
"engine.io": "~4.0.0",
"socket.io-adapter": "~2.0.1",
"socket.io-adapter": "2.0.3-rc1",
"socket.io-client": "3.0.0-rc1",
"socket.io-parser": "4.0.1-rc2"
},
Expand Down

0 comments on commit 8a5db7f

Please sign in to comment.