From c4394c3c5ef2d0d1999bf6ef2cd85903dbe1057d Mon Sep 17 00:00:00 2001
From: Luigi Pinca <luigipinca@gmail.com>
Date: Tue, 20 Jul 2021 12:39:46 +0200
Subject: [PATCH] [major] Overhaul event classes

- Remove non-standard `OpenEvent` class.
- Make properties read-only.
- Update constructor signatures to match the ones defined by the HTML
  standard.
---
 lib/event-target.js       | 203 +++++++++++++++++++++---------
 lib/websocket.js          |   4 +-
 test/event-target.test.js | 253 ++++++++++++++++++++++++++++++++++++++
 test/websocket.test.js    |  61 +++++----
 4 files changed, 437 insertions(+), 84 deletions(-)
 create mode 100644 test/event-target.test.js

diff --git a/lib/event-target.js b/lib/event-target.js
index 4cf076477..d5abd83a0 100644
--- a/lib/event-target.js
+++ b/lib/event-target.js
@@ -2,112 +2,171 @@
 
 const { kForOnEventAttribute, kListener } = require('./constants');
 
+const kCode = Symbol('kCode');
+const kData = Symbol('kData');
+const kError = Symbol('kError');
+const kMessage = Symbol('kMessage');
+const kReason = Symbol('kReason');
+const kTarget = Symbol('kTarget');
+const kType = Symbol('kType');
+const kWasClean = Symbol('kWasClean');
+
 /**
  * Class representing an event.
- *
- * @private
  */
 class Event {
   /**
    * Create a new `Event`.
    *
    * @param {String} type The name of the event
-   * @param {Object} target A reference to the target to which the event was
-   *     dispatched
+   * @throws {TypeError} If the `type` argument is not specified
    */
-  constructor(type, target) {
-    this.target = target;
-    this.type = type;
+  constructor(type) {
+    this[kTarget] = null;
+    this[kType] = type;
   }
-}
 
-/**
- * Class representing a message event.
- *
- * @extends Event
- * @private
- */
-class MessageEvent extends Event {
   /**
-   * Create a new `MessageEvent`.
-   *
-   * @param {(String|Buffer|ArrayBuffer|Buffer[])} data The received data
-   * @param {WebSocket} target A reference to the target to which the event was
-   *     dispatched
+   * @type {*}
    */
-  constructor(data, target) {
-    super('message', target);
+  get target() {
+    return this[kTarget];
+  }
 
-    this.data = data;
+  /**
+   * @type {String}
+   */
+  get type() {
+    return this[kType];
   }
 }
 
+Object.defineProperty(Event.prototype, 'target', { enumerable: true });
+Object.defineProperty(Event.prototype, 'type', { enumerable: true });
+
 /**
  * Class representing a close event.
  *
  * @extends Event
- * @private
  */
 class CloseEvent extends Event {
   /**
    * Create a new `CloseEvent`.
    *
-   * @param {Number} code The status code explaining why the connection is being
-   *     closed
-   * @param {String} reason A human-readable string explaining why the
-   *     connection is closing
-   * @param {WebSocket} target A reference to the target to which the event was
-   *     dispatched
+   * @param {String} type The name of the event
+   * @param {Object} [options] A dictionary object that allows for setting
+   *     attributes via object members of the same name
+   * @param {Number} [options.code=0] The status code explaining why the
+   *     connection was closed
+   * @param {String} [options.reason=''] A human-readable string explaining why
+   *     the connection was closed
+   * @param {Boolean} [options.wasClean=false] Indicates whether or not the
+   *     connection was cleanly closed
    */
-  constructor(code, reason, target) {
-    super('close', target);
+  constructor(type, options = {}) {
+    super(type);
 
-    this.wasClean = target._closeFrameReceived && target._closeFrameSent;
-    this.reason = reason;
-    this.code = code;
+    this[kCode] = options.code === undefined ? 0 : options.code;
+    this[kReason] = options.reason === undefined ? '' : options.reason;
+    this[kWasClean] = options.wasClean === undefined ? false : options.wasClean;
+  }
+
+  /**
+   * @type {Number}
+   */
+  get code() {
+    return this[kCode];
+  }
+
+  /**
+   * @type {String}
+   */
+  get reason() {
+    return this[kReason];
+  }
+
+  /**
+   * @type {Boolean}
+   */
+  get wasClean() {
+    return this[kWasClean];
   }
 }
 
+Object.defineProperty(CloseEvent.prototype, 'code', { enumerable: true });
+Object.defineProperty(CloseEvent.prototype, 'reason', { enumerable: true });
+Object.defineProperty(CloseEvent.prototype, 'wasClean', { enumerable: true });
+
 /**
- * Class representing an open event.
+ * Class representing an error event.
  *
  * @extends Event
- * @private
  */
-class OpenEvent extends Event {
+class ErrorEvent extends Event {
   /**
-   * Create a new `OpenEvent`.
+   * Create a new `ErrorEvent`.
    *
-   * @param {WebSocket} target A reference to the target to which the event was
-   *     dispatched
+   * @param {String} type The name of the event
+   * @param {Object} [options] A dictionary object that allows for setting
+   *     attributes via object members of the same name
+   * @param {*} [options.error=null] The error that generated this event
+   * @param {String} [options.message=''] The error message
+   */
+  constructor(type, options = {}) {
+    super(type);
+
+    this[kError] = options.error === undefined ? null : options.error;
+    this[kMessage] = options.message === undefined ? '' : options.message;
+  }
+
+  /**
+   * @type {*}
+   */
+  get error() {
+    return this[kError];
+  }
+
+  /**
+   * @type {String}
    */
-  constructor(target) {
-    super('open', target);
+  get message() {
+    return this[kMessage];
   }
 }
 
+Object.defineProperty(ErrorEvent.prototype, 'error', { enumerable: true });
+Object.defineProperty(ErrorEvent.prototype, 'message', { enumerable: true });
+
 /**
- * Class representing an error event.
+ * Class representing a message event.
  *
  * @extends Event
- * @private
  */
-class ErrorEvent extends Event {
+class MessageEvent extends Event {
   /**
-   * Create a new `ErrorEvent`.
+   * Create a new `MessageEvent`.
    *
-   * @param {Object} error The error that generated this event
-   * @param {WebSocket} target A reference to the target to which the event was
-   *     dispatched
+   * @param {String} type The name of the event
+   * @param {Object} [options] A dictionary object that allows for setting
+   *     attributes via object members of the same name
+   * @param {*} [options.data=null] The message content
    */
-  constructor(error, target) {
-    super('error', target);
+  constructor(type, options = {}) {
+    super(type);
+
+    this[kData] = options.data === undefined ? null : options.data;
+  }
 
-    this.message = error.message;
-    this.error = error;
+  /**
+   * @type {*}
+   */
+  get data() {
+    return this[kData];
   }
 }
 
+Object.defineProperty(MessageEvent.prototype, 'data', { enumerable: true });
+
 /**
  * This provides methods for emulating the `EventTarget` interface. It's not
  * meant to be used directly.
@@ -132,20 +191,40 @@ const EventTarget = {
 
     if (type === 'message') {
       wrapper = function onMessage(data, isBinary) {
-        const event = new MessageEvent(isBinary ? data : data.toString(), this);
+        const event = new MessageEvent('message', {
+          data: isBinary ? data : data.toString()
+        });
+
+        event[kTarget] = this;
         listener.call(this, event);
       };
     } else if (type === 'close') {
       wrapper = function onClose(code, message) {
-        listener.call(this, new CloseEvent(code, message.toString(), this));
+        const event = new CloseEvent('close', {
+          code,
+          reason: message.toString(),
+          wasClean: this._closeFrameReceived && this._closeFrameSent
+        });
+
+        event[kTarget] = this;
+        listener.call(this, event);
       };
     } else if (type === 'error') {
       wrapper = function onError(error) {
-        listener.call(this, new ErrorEvent(error, this));
+        const event = new ErrorEvent('error', {
+          error,
+          message: error.message
+        });
+
+        event[kTarget] = this;
+        listener.call(this, event);
       };
     } else if (type === 'open') {
       wrapper = function onOpen() {
-        listener.call(this, new OpenEvent(this));
+        const event = new Event('open');
+
+        event[kTarget] = this;
+        listener.call(this, event);
       };
     } else {
       return;
@@ -178,4 +257,10 @@ const EventTarget = {
   }
 };
 
-module.exports = EventTarget;
+module.exports = {
+  CloseEvent,
+  ErrorEvent,
+  Event,
+  EventTarget,
+  MessageEvent
+};
diff --git a/lib/websocket.js b/lib/websocket.js
index 45a643f22..3b2bacd28 100644
--- a/lib/websocket.js
+++ b/lib/websocket.js
@@ -21,7 +21,9 @@ const {
   kWebSocket,
   NOOP
 } = require('./constants');
-const { addEventListener, removeEventListener } = require('./event-target');
+const {
+  EventTarget: { addEventListener, removeEventListener }
+} = require('./event-target');
 const { format, parse } = require('./extension');
 const { toBuffer } = require('./buffer-util');
 
diff --git a/test/event-target.test.js b/test/event-target.test.js
new file mode 100644
index 000000000..5caaa5c27
--- /dev/null
+++ b/test/event-target.test.js
@@ -0,0 +1,253 @@
+'use strict';
+
+const assert = require('assert');
+
+const {
+  CloseEvent,
+  ErrorEvent,
+  Event,
+  MessageEvent
+} = require('../lib/event-target');
+
+describe('Event', () => {
+  describe('#ctor', () => {
+    it('takes a `type` argument', () => {
+      const event = new Event('foo');
+
+      assert.strictEqual(event.type, 'foo');
+    });
+  });
+
+  describe('Properties', () => {
+    describe('`target`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          Event.prototype,
+          'target'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+
+      it('defaults to `null`', () => {
+        const event = new Event('foo');
+
+        assert.strictEqual(event.target, null);
+      });
+    });
+
+    describe('`type`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          Event.prototype,
+          'type'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+    });
+  });
+});
+
+describe('CloseEvent', () => {
+  it('inherits from `Event`', () => {
+    assert.ok(CloseEvent.prototype instanceof Event);
+  });
+
+  describe('#ctor', () => {
+    it('takes a `type` argument', () => {
+      const event = new CloseEvent('foo');
+
+      assert.strictEqual(event.type, 'foo');
+    });
+
+    it('takes an optional `options` argument', () => {
+      const event = new CloseEvent('close', {
+        code: 1000,
+        reason: 'foo',
+        wasClean: true
+      });
+
+      assert.strictEqual(event.type, 'close');
+      assert.strictEqual(event.code, 1000);
+      assert.strictEqual(event.reason, 'foo');
+      assert.strictEqual(event.wasClean, true);
+    });
+  });
+
+  describe('Properties', () => {
+    describe('`code`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          CloseEvent.prototype,
+          'code'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+
+      it('defaults to 0', () => {
+        const event = new CloseEvent('close');
+
+        assert.strictEqual(event.code, 0);
+      });
+    });
+
+    describe('`reason`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          CloseEvent.prototype,
+          'reason'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+
+      it('defaults to an empty string', () => {
+        const event = new CloseEvent('close');
+
+        assert.strictEqual(event.reason, '');
+      });
+    });
+
+    describe('`wasClean`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          CloseEvent.prototype,
+          'wasClean'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+
+      it('defaults to false', () => {
+        const event = new CloseEvent('close');
+
+        assert.strictEqual(event.wasClean, false);
+      });
+    });
+  });
+});
+
+describe('ErrorEvent', () => {
+  it('inherits from `Event`', () => {
+    assert.ok(ErrorEvent.prototype instanceof Event);
+  });
+
+  describe('#ctor', () => {
+    it('takes a `type` argument', () => {
+      const event = new ErrorEvent('foo');
+
+      assert.strictEqual(event.type, 'foo');
+    });
+
+    it('takes an optional `options` argument', () => {
+      const error = new Error('Oops');
+      const event = new ErrorEvent('error', { error, message: error.message });
+
+      assert.strictEqual(event.type, 'error');
+      assert.strictEqual(event.error, error);
+      assert.strictEqual(event.message, error.message);
+    });
+  });
+
+  describe('Properties', () => {
+    describe('`error`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          ErrorEvent.prototype,
+          'error'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+
+      it('defaults to `null`', () => {
+        const event = new ErrorEvent('error');
+
+        assert.strictEqual(event.error, null);
+      });
+    });
+
+    describe('`message`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          ErrorEvent.prototype,
+          'message'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+
+      it('defaults to an empty string', () => {
+        const event = new ErrorEvent('error');
+
+        assert.strictEqual(event.message, '');
+      });
+    });
+  });
+});
+
+describe('MessageEvent', () => {
+  it('inherits from `Event`', () => {
+    assert.ok(MessageEvent.prototype instanceof Event);
+  });
+
+  describe('#ctor', () => {
+    it('takes a `type` argument', () => {
+      const event = new MessageEvent('foo');
+
+      assert.strictEqual(event.type, 'foo');
+    });
+
+    it('takes an optional `options` argument', () => {
+      const event = new MessageEvent('message', { data: 'bar' });
+
+      assert.strictEqual(event.type, 'message');
+      assert.strictEqual(event.data, 'bar');
+    });
+  });
+
+  describe('Properties', () => {
+    describe('`data`', () => {
+      it('is enumerable and configurable', () => {
+        const descriptor = Object.getOwnPropertyDescriptor(
+          MessageEvent.prototype,
+          'data'
+        );
+
+        assert.strictEqual(descriptor.configurable, true);
+        assert.strictEqual(descriptor.enumerable, true);
+        assert.ok(descriptor.get !== undefined);
+        assert.ok(descriptor.set === undefined);
+      });
+
+      it('defaults to `null`', () => {
+        const event = new MessageEvent('message');
+
+        assert.strictEqual(event.data, null);
+      });
+    });
+  });
+});
diff --git a/test/websocket.test.js b/test/websocket.test.js
index 0841a97ae..82a25bfce 100644
--- a/test/websocket.test.js
+++ b/test/websocket.test.js
@@ -11,6 +11,12 @@ const fs = require('fs');
 const { URL } = require('url');
 
 const WebSocket = require('..');
+const {
+  CloseEvent,
+  ErrorEvent,
+  Event,
+  MessageEvent
+} = require('../lib/event-target');
 const { EMPTY_BUFFER, GUID, kListener, NOOP } = require('../lib/constants');
 
 class CustomAgent extends http.Agent {
@@ -2315,8 +2321,9 @@ describe('WebSocket', () => {
           ws.close();
         });
 
-        ws.addEventListener('message', (messageEvent) => {
-          assert.strictEqual(messageEvent.data, 'hi');
+        ws.addEventListener('message', (event) => {
+          assert.ok(event instanceof MessageEvent);
+          assert.strictEqual(event.data, 'hi');
           wss.close(done);
         });
       });
@@ -2332,10 +2339,11 @@ describe('WebSocket', () => {
       const wss = new WebSocket.Server({ port: 0 }, () => {
         const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
 
-        ws.addEventListener('close', (closeEvent) => {
-          assert.ok(closeEvent.wasClean);
-          assert.strictEqual(closeEvent.reason, '');
-          assert.strictEqual(closeEvent.code, 1000);
+        ws.addEventListener('close', (event) => {
+          assert.ok(event instanceof CloseEvent);
+          assert.ok(event.wasClean);
+          assert.strictEqual(event.reason, '');
+          assert.strictEqual(event.code, 1000);
           wss.close(done);
         });
       });
@@ -2347,10 +2355,11 @@ describe('WebSocket', () => {
       const wss = new WebSocket.Server({ port: 0 }, () => {
         const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
 
-        ws.addEventListener('close', (closeEvent) => {
-          assert.ok(closeEvent.wasClean);
-          assert.strictEqual(closeEvent.reason, 'some daft reason');
-          assert.strictEqual(closeEvent.code, 4000);
+        ws.addEventListener('close', (event) => {
+          assert.ok(event instanceof CloseEvent);
+          assert.ok(event.wasClean);
+          assert.strictEqual(event.reason, 'some daft reason');
+          assert.strictEqual(event.code, 4000);
           wss.close(done);
         });
       });
@@ -2363,25 +2372,29 @@ describe('WebSocket', () => {
         const err = new Error('forced');
         const ws = new WebSocket(`ws://localhost:${wss.address().port}`);
 
-        ws.addEventListener('open', (openEvent) => {
-          assert.strictEqual(openEvent.type, 'open');
-          assert.strictEqual(openEvent.target, ws);
+        ws.addEventListener('open', (event) => {
+          assert.ok(event instanceof Event);
+          assert.strictEqual(event.type, 'open');
+          assert.strictEqual(event.target, ws);
         });
-        ws.addEventListener('message', (messageEvent) => {
-          assert.strictEqual(messageEvent.type, 'message');
-          assert.strictEqual(messageEvent.target, ws);
+        ws.addEventListener('message', (event) => {
+          assert.ok(event instanceof MessageEvent);
+          assert.strictEqual(event.type, 'message');
+          assert.strictEqual(event.target, ws);
           ws.close();
         });
-        ws.addEventListener('close', (closeEvent) => {
-          assert.strictEqual(closeEvent.type, 'close');
-          assert.strictEqual(closeEvent.target, ws);
+        ws.addEventListener('close', (event) => {
+          assert.ok(event instanceof CloseEvent);
+          assert.strictEqual(event.type, 'close');
+          assert.strictEqual(event.target, ws);
           ws.emit('error', err);
         });
-        ws.addEventListener('error', (errorEvent) => {
-          assert.strictEqual(errorEvent.message, 'forced');
-          assert.strictEqual(errorEvent.type, 'error');
-          assert.strictEqual(errorEvent.target, ws);
-          assert.strictEqual(errorEvent.error, err);
+        ws.addEventListener('error', (event) => {
+          assert.ok(event instanceof ErrorEvent);
+          assert.strictEqual(event.message, 'forced');
+          assert.strictEqual(event.type, 'error');
+          assert.strictEqual(event.target, ws);
+          assert.strictEqual(event.error, err);
 
           wss.close(done);
         });