diff --git a/doc/ws.md b/doc/ws.md index 390596270..110980816 100644 --- a/doc/ws.md +++ b/doc/ws.md @@ -370,7 +370,9 @@ handshake. This allows you to read headers from the server, for example at most once after being added. If `true`, the listener would be automatically removed when invoked. -Register an event listener emulating the `EventTarget` interface. +Register an event listener emulating the `EventTarget` interface. This method +does nothing if `type` is not one of `'close'`, `'error'`, `'message'`, or +`'open'`. ### websocket.binaryType diff --git a/lib/constants.js b/lib/constants.js index 4082981f8..dce5dd21a 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -2,9 +2,10 @@ module.exports = { BINARY_TYPES: ['nodebuffer', 'arraybuffer', 'fragments'], + EMPTY_BUFFER: Buffer.alloc(0), GUID: '258EAFA5-E914-47DA-95CA-C5AB0DC85B11', + kListener: Symbol('kListener'), kStatusCode: Symbol('status-code'), kWebSocket: Symbol('websocket'), - EMPTY_BUFFER: Buffer.alloc(0), NOOP: () => {} }; diff --git a/lib/event-target.js b/lib/event-target.js index cc4f3ba97..6987d5b5c 100644 --- a/lib/event-target.js +++ b/lib/event-target.js @@ -1,5 +1,7 @@ 'use strict'; +const { kListener } = require('./constants'); + /** * Class representing an event. * @@ -126,8 +128,6 @@ const EventTarget = { * @public */ addEventListener(type, listener, options) { - if (typeof listener !== 'function') return; - function onMessage(data, isBinary) { listener.call( this, @@ -150,19 +150,17 @@ const EventTarget = { const method = options && options.once ? 'once' : 'on'; if (type === 'message') { - onMessage._listener = listener; + onMessage[kListener] = listener; this[method](type, onMessage); } else if (type === 'close') { - onClose._listener = listener; + onClose[kListener] = listener; this[method](type, onClose); } else if (type === 'error') { - onError._listener = listener; + onError[kListener] = listener; this[method](type, onError); } else if (type === 'open') { - onOpen._listener = listener; + onOpen[kListener] = listener; this[method](type, onOpen); - } else { - this[method](type, listener); } }, @@ -170,15 +168,13 @@ const EventTarget = { * Remove an event listener. * * @param {String} type A string representing the event type to remove - * @param {Function} listener The listener to remove + * @param {Function} handler The listener to remove * @public */ - removeEventListener(type, listener) { - const listeners = this.listeners(type); - - for (let i = 0; i < listeners.length; i++) { - if (listeners[i] === listener || listeners[i]._listener === listener) { - this.removeListener(type, listeners[i]); + removeEventListener(type, handler) { + for (const listener of this.listeners(type)) { + if (listener === handler || listener[kListener] === handler) { + this.removeListener(type, listener); } } } diff --git a/lib/websocket.js b/lib/websocket.js index cb8077de5..bf33b6ce8 100644 --- a/lib/websocket.js +++ b/lib/websocket.js @@ -15,6 +15,7 @@ const { BINARY_TYPES, EMPTY_BUFFER, GUID, + kListener, kStatusCode, kWebSocket, NOOP @@ -522,22 +523,23 @@ Object.defineProperty(WebSocket.prototype, 'CLOSED', { Object.defineProperty(WebSocket.prototype, `on${method}`, { enumerable: true, get() { - const listeners = this.listeners(method); - for (let i = 0; i < listeners.length; i++) { - if (listeners[i]._listener) return listeners[i]._listener; + for (const listener of this.listeners(method)) { + if (listener[kListener]) return listener[kListener]; } return undefined; }, - set(listener) { - const listeners = this.listeners(method); - for (let i = 0; i < listeners.length; i++) { + set(handler) { + for (const listener of this.listeners(method)) { // // Remove only the listeners added via `addEventListener`. // - if (listeners[i]._listener) this.removeListener(method, listeners[i]); + if (listener[kListener]) this.removeListener(method, listener); } - this.addEventListener(method, listener); + + if (typeof handler !== 'function') return; + + this.addEventListener(method, handler); } }); }); diff --git a/test/websocket.test.js b/test/websocket.test.js index feb1d8c50..e409e8fca 100644 --- a/test/websocket.test.js +++ b/test/websocket.test.js @@ -11,7 +11,7 @@ const fs = require('fs'); const { URL } = require('url'); const WebSocket = require('..'); -const { EMPTY_BUFFER, GUID, NOOP } = require('../lib/constants'); +const { EMPTY_BUFFER, GUID, kListener, NOOP } = require('../lib/constants'); class CustomAgent extends http.Agent { addRequest() {} @@ -2157,7 +2157,7 @@ describe('WebSocket', () => { assert.strictEqual(listeners.length, 2); assert.strictEqual(listeners[0], NOOP); - assert.strictEqual(listeners[1]._listener, NOOP); + assert.strictEqual(listeners[1][kListener], NOOP); ws.onclose = NOOP; @@ -2165,36 +2165,38 @@ describe('WebSocket', () => { assert.strictEqual(listeners.length, 2); assert.strictEqual(listeners[0], NOOP); - assert.strictEqual(listeners[1]._listener, NOOP); + assert.strictEqual(listeners[1][kListener], NOOP); }); - it('adds listeners for custom events with `addEventListener`', () => { + it('supports the `addEventListener` method', () => { + const events = []; const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() }); - ws.addEventListener('foo', NOOP); - assert.strictEqual(ws.listeners('foo')[0], NOOP); + ws.addEventListener('foo', () => {}); + assert.strictEqual(ws.listenerCount('foo'), 0); - // - // Fails silently when the `listener` is not a function. - // - ws.addEventListener('bar', {}); - assert.strictEqual(ws.listeners('bar').length, 0); - }); + ws.addEventListener('open', () => { + events.push('open'); + assert.strictEqual(ws.listenerCount('open'), 1); + }); - it('allows to add one time listeners with `addEventListener`', (done) => { - const ws = new WebSocket('ws://localhost', { agent: new CustomAgent() }); + assert.strictEqual(ws.listenerCount('open'), 1); ws.addEventListener( - 'foo', + 'message', () => { - assert.strictEqual(ws.listenerCount('foo'), 0); - done(); + events.push('message'); + assert.strictEqual(ws.listenerCount('message'), 0); }, { once: true } ); - assert.strictEqual(ws.listenerCount('foo'), 1); - ws.emit('foo'); + assert.strictEqual(ws.listenerCount('message'), 1); + + ws.emit('open'); + ws.emit('message', EMPTY_BUFFER, false); + + assert.deepStrictEqual(events, ['open', 'message']); }); it('supports the `removeEventListener` method', () => { @@ -2202,43 +2204,35 @@ describe('WebSocket', () => { ws.addEventListener('message', NOOP); ws.addEventListener('open', NOOP); - ws.addEventListener('foo', NOOP); - assert.strictEqual(ws.listeners('message')[0]._listener, NOOP); - assert.strictEqual(ws.listeners('open')[0]._listener, NOOP); - assert.strictEqual(ws.listeners('foo')[0], NOOP); + assert.strictEqual(ws.listeners('message')[0][kListener], NOOP); + assert.strictEqual(ws.listeners('open')[0][kListener], NOOP); ws.removeEventListener('message', () => {}); - assert.strictEqual(ws.listeners('message')[0]._listener, NOOP); + assert.strictEqual(ws.listeners('message')[0][kListener], NOOP); ws.removeEventListener('message', NOOP); ws.removeEventListener('open', NOOP); - ws.removeEventListener('foo', NOOP); assert.strictEqual(ws.listenerCount('message'), 0); assert.strictEqual(ws.listenerCount('open'), 0); - assert.strictEqual(ws.listenerCount('foo'), 0); ws.addEventListener('message', NOOP, { once: true }); ws.addEventListener('open', NOOP, { once: true }); - ws.addEventListener('foo', NOOP, { once: true }); - assert.strictEqual(ws.listeners('message')[0]._listener, NOOP); - assert.strictEqual(ws.listeners('open')[0]._listener, NOOP); - assert.strictEqual(ws.listeners('foo')[0], NOOP); + assert.strictEqual(ws.listeners('message')[0][kListener], NOOP); + assert.strictEqual(ws.listeners('open')[0][kListener], NOOP); ws.removeEventListener('message', () => {}); - assert.strictEqual(ws.listeners('message')[0]._listener, NOOP); + assert.strictEqual(ws.listeners('message')[0][kListener], NOOP); ws.removeEventListener('message', NOOP); ws.removeEventListener('open', NOOP); - ws.removeEventListener('foo', NOOP); assert.strictEqual(ws.listenerCount('message'), 0); assert.strictEqual(ws.listenerCount('open'), 0); - assert.strictEqual(ws.listenerCount('foo'), 0); }); it('wraps text data in a `MessageEvent`', (done) => {