events: support event handlers on prototypes

PR-URL: #35931 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Daijiro Wachi <[email protected]> Reviewed-By: Rich Trott <[email protected]>
nodejs · Nov 9, 2020 · ab0eb4f · ab0eb4f
1 parent 33e2ee5
commit ab0eb4f
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 12 deletions.
diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
@@ -14,6 +14,7 @@ const {
   Symbol,
   SymbolFor,
   SymbolToStringTag,
+  SafeWeakMap,
   SafeWeakSet,
 } = primordials;
 
@@ -577,21 +578,27 @@ function emitUnhandledRejectionOrErr(that, err, event) {
   process.emit('error', err, event);
 }
 
+// A map of emitter -> map of name -> handler
+const eventHandlerValueMap = new SafeWeakMap();
+
 function defineEventHandler(emitter, name) {
   // 8.1.5.1 Event handlers - basically `on[eventName]` attributes
-  let eventHandlerValue;
   ObjectDefineProperty(emitter, `on${name}`, {
     get() {
-      return eventHandlerValue;
+      return eventHandlerValueMap.get(this)?.get(name);
     },
     set(value) {
-      if (eventHandlerValue) {
-        emitter.removeEventListener(name, eventHandlerValue);
+      const oldValue = eventHandlerValueMap.get(this)?.get(name);
+      if (oldValue) {
+        this.removeEventListener(name, oldValue);
       }
       if (typeof value === 'function') {
-        emitter.addEventListener(name, value);
+        this.addEventListener(name, value);
+      }
+      if (!eventHandlerValueMap.has(this)) {
+        eventHandlerValueMap.set(this, new Map());
       }
-      eventHandlerValue = value;
+      eventHandlerValueMap.get(this).set(name, value);
     },
     configurable: true,
     enumerable: true

diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js
@@ -146,13 +146,14 @@ ObjectDefineProperty(
 // This is called from inside the `MessagePort` constructor.
 function oninit() {
   initNodeEventTarget(this);
-  // TODO(addaleax): This should be on MessagePort.prototype, but
-  // defineEventHandler() does not support that.
   defineEventHandler(this, 'message');
   defineEventHandler(this, 'messageerror');
   setupPortReferencing(this, this, 'message');
 }
 
+defineEventHandler(MessagePort.prototype, 'message');
+defineEventHandler(MessagePort.prototype, 'messageerror');
+
 ObjectDefineProperty(MessagePort.prototype, onInitSymbol, {
   enumerable: true,
   writable: false,

diff --git a/test/parallel/test-worker-message-port.js b/test/parallel/test-worker-message-port.js
@@ -165,9 +165,7 @@ const { MessageChannel, MessagePort } = require('worker_threads');
   assert.deepStrictEqual(
     Object.getOwnPropertyNames(MessagePort.prototype).sort(),
     [
-      // TODO(addaleax): This should include onmessage (and eventually
-      // onmessageerror).
-      'close', 'constructor', 'postMessage', 'ref', 'start',
-      'unref'
+      'close', 'constructor', 'onmessage', 'onmessageerror', 'postMessage',
+      'ref', 'start', 'unref'
     ]);
 }