From 78632a128dbebe208e89fe7a5aa17fcb287aa44f Mon Sep 17 00:00:00 2001 From: Olaf Tomalka Date: Tue, 7 Jun 2022 08:49:46 +0200 Subject: [PATCH] Fix #242 (#362) * Fix #242 * Changes after code review --- .eslintrc | 3 +- src/websocket.js | 25 +++++++---- tests/issues/242.test.js | 90 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 tests/issues/242.test.js diff --git a/.eslintrc b/.eslintrc index e4db4868..a2fc9ca5 100644 --- a/.eslintrc +++ b/.eslintrc @@ -13,6 +13,7 @@ "no-param-reassign": 0, "comma-dangle": ["error", "never"], "arrow-parens": 0, - "import/no-extraneous-dependencies": 0 // all deps are resolved with "node-resolve" + "import/no-extraneous-dependencies": 0, // all deps are resolved with "node-resolve" + "no-underscore-dangle": ["error", { "allowAfterThis": true }] } } diff --git a/src/websocket.js b/src/websocket.js index ae4d2447..112193c9 100644 --- a/src/websocket.js +++ b/src/websocket.js @@ -21,6 +21,11 @@ class WebSocket extends EventTarget { constructor(url, protocols) { super(); + this._onopen = null; + this._onmessage = null; + this._onerror = null; + this._onclose = null; + this.url = urlVerification(url); protocols = protocolVerification(protocols); this.protocol = protocols[0] || ''; @@ -94,38 +99,42 @@ class WebSocket extends EventTarget { } get onopen() { - return this.listeners.open; + return this._onopen; } get onmessage() { - return this.listeners.message; + return this._onmessage; } get onclose() { - return this.listeners.close; + return this._onclose; } get onerror() { - return this.listeners.error; + return this._onerror; } set onopen(listener) { - delete this.listeners.open; + this.removeEventListener('open', this._onopen); + this._onopen = listener; this.addEventListener('open', listener); } set onmessage(listener) { - delete this.listeners.message; + this.removeEventListener('message', this._onmessage); + this._onmessage = listener; this.addEventListener('message', listener); } set onclose(listener) { - delete this.listeners.close; + this.removeEventListener('close', this._onclose); + this._onclose = listener; this.addEventListener('close', listener); } set onerror(listener) { - delete this.listeners.error; + this.removeEventListener('error', this._onerror); + this._onerror = listener; this.addEventListener('error', listener); } diff --git a/tests/issues/242.test.js b/tests/issues/242.test.js new file mode 100644 index 00000000..647a706c --- /dev/null +++ b/tests/issues/242.test.js @@ -0,0 +1,90 @@ +import test from 'ava'; +import Server from '../../src/server'; +import WebSocket from '../../src/websocket'; +import Event from '../../src/event/event'; + +test('websocket on* methods family returns a single listener', t => { + const socketUrl = 'ws://localhost:8080'; + const mockServer = new Server(socketUrl); + const mockSocket = new WebSocket(socketUrl); + + const listener = () => { + /* do nothing */ + }; + + mockSocket.onopen = listener; + mockSocket.onmessage = listener; + mockSocket.onerror = listener; + mockSocket.onclose = listener; + + t.is(mockSocket.onopen, listener); + t.is(mockSocket.onmessage, listener); + t.is(mockSocket.onerror, listener); + t.is(mockSocket.onclose, listener); + + mockServer.close(); +}); + +test("websocket on* methods family doesn't delete other listeners", async t => { + const socketUrl = 'ws://localhost:8080'; + const mockServer = new Server(socketUrl); + const mockSocket = new WebSocket(socketUrl); + + mockServer.on('connection', socket => { + socket.send('test message'); + }); + + let onOpenCalled = 0; + let onMessageCalled = 0; + let onErrorCalled = 0; + + let onCloseEventResolve; + let onCloseResolve; + const allClosed = Promise.all([ + new Promise(r => { + onCloseEventResolve = r; + }), + new Promise(r => { + onCloseResolve = r; + }) + ]); + + mockSocket.addEventListener('open', () => { + onOpenCalled += 1; + }); + mockSocket.addEventListener('message', () => { + onMessageCalled += 1; + mockSocket.dispatchEvent(new Event('error')); + }); + mockSocket.addEventListener('error', () => { + onErrorCalled += 1; + mockSocket.close(); + }); + mockSocket.addEventListener('close', () => onCloseEventResolve()); + + const throwCb = () => { + throw new Error('this call should have been replaced'); + }; + mockSocket.onopen = throwCb; + mockSocket.onopen = () => { + onOpenCalled += 1; + }; + mockSocket.onmessage = throwCb; + mockSocket.onmessage = () => { + onMessageCalled += 1; + }; + mockSocket.onerror = throwCb; + mockSocket.onerror = () => { + onErrorCalled += 1; + }; + mockSocket.onclose = throwCb; + mockSocket.onclose = () => onCloseResolve(); + + await allClosed; + + t.is(onOpenCalled, 2); + t.is(onMessageCalled, 2); + t.is(onErrorCalled, 2); + + mockServer.close(); +});