events: support event handlers on prototypes

benjamingr · danielleadams · commit ab0eb4f2c903 · 2020-11-09T13:35:02.000-05:00
PR-URL: #35931 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
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'
     ]);
 }

Original file line number	Diff line number	Diff line change
`@@ -165,9 +165,7 @@ const { MessageChannel, MessagePort } = require('worker_threads');`
`165`	`165`	`assert.deepStrictEqual(`
`166`	`166`	`Object.getOwnPropertyNames(MessagePort.prototype).sort(),`
`167`	`167`	`[`
`168`		`- // TODO(addaleax): This should include onmessage (and eventually`
`169`		`- // onmessageerror).`
`170`		`- 'close', 'constructor', 'postMessage', 'ref', 'start',`
`171`		`- 'unref'`
	`168`	`+ 'close', 'constructor', 'onmessage', 'onmessageerror', 'postMessage',`
	`169`	`+ 'ref', 'start', 'unref'`
`172`	`170`	`]);`
`173`	`171`	`}`