events: support defining event handlers on prototypes

lib/internal/abort_controller.js

@@ -48,6 +48,8 @@ ObjectDefineProperties(AbortSignal.prototype, {
   aborted: { enumerable: true }
 });
 
+defineEventHandler(AbortSignal.prototype, 'abort');
+
 function abortSignal(signal) {
   if (signal[kAborted]) return;
   signal[kAborted] = true;
@@ -65,7 +67,6 @@ class AbortController {
   constructor() {
     this[kSignal] = new AbortSignal();
     emitExperimentalWarning('AbortController');
-    defineEventHandler(this[kSignal], 'abort');
   }
 
   get signal() { return this[kSignal]; }

lib/internal/event_target.js

@@ -4,12 +4,13 @@ const {
   ArrayFrom,
   Boolean,
   Error,
-  Map,
   NumberIsInteger,
   ObjectAssign,
   ObjectDefineProperties,
   ObjectDefineProperty,
   ObjectGetOwnPropertyDescriptor,
+  ReflectApply,
+  SafeMap,
   String,
   Symbol,
   SymbolFor,
@@ -35,6 +36,7 @@ const kIsEventTarget = SymbolFor('nodejs.event_target');
 const kEvents = Symbol('kEvents');
 const kStop = Symbol('kStop');
 const kTarget = Symbol('kTarget');
+const kHandlers = Symbol('khandlers');
 
 const kHybridDispatch = Symbol.for('nodejs.internal.kHybridDispatch');
 const kCreateEvent = Symbol('kCreateEvent');
@@ -218,7 +220,7 @@ class Listener {
 }
 
 function initEventTarget(self) {
-  self[kEvents] = new Map();
+  self[kEvents] = new SafeMap();
 }
 
 class EventTarget {
@@ -577,22 +579,50 @@ function emitUnhandledRejectionOrErr(that, err, event) {
   process.emit('error', err, event);
 }
 
+function makeEventHandler(handler) {
+  // Event handlers are dispatched in the order they were first set
+  // See https://github.com/nodejs/node/pull/35949#issuecomment-722496598
+  function eventHandler(...args) {
+    if (typeof eventHandler.handler !== 'function') {
+      return;
+    }
+    return ReflectApply(eventHandler.handler, this, args);
+  }
+  eventHandler.handler = handler;
+  return eventHandler;
+}
+
 function defineEventHandler(emitter, name) {
   // 8.1.5.1 Event handlers - basically `on[eventName]` attributes
-  let eventHandlerValue;
   ObjectDefineProperty(emitter, `on${name}`, {
     get() {
-      return eventHandlerValue;
+      return this[kHandlers]?.get(name)?.handler;
     },
     set(value) {
-      if (eventHandlerValue) {
-        emitter.removeEventListener(name, eventHandlerValue);
+      if (!this[kHandlers]) {
+        this[kHandlers] = new SafeMap();
       }
-      if (typeof value === 'function') {
-        emitter.addEventListener(name, value);
+      let wrappedHandler = this[kHandlers]?.get(name);
+      if (wrappedHandler) {
+        if (typeof wrappedHandler.handler === 'function') {
+          this[kEvents].get(name).size--;
+          const size = this[kEvents].get(name).size;
+          this[kRemoveListener](size, name, wrappedHandler.handler, false);
+        }
+        wrappedHandler.handler = value;
+        if (typeof wrappedHandler.handler === 'function') {
+          this[kEvents].get(name).size++;
+          const size = this[kEvents].get(name).size;
+          this[kNewListener](size, name, value, false, false, false);
+        }
+      } else {
+        wrappedHandler = makeEventHandler(value);
+        this.addEventListener(name, wrappedHandler);
       }
-      eventHandlerValue = value;
-    }
+      this[kHandlers].set(name, wrappedHandler);
+    },
+    configurable: true,
+    enumerable: true
   });
 }
 module.exports = {

lib/internal/worker/io.js

@@ -146,13 +146,12 @@ 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,

test/parallel/test-eventtarget.js

@@ -517,3 +517,22 @@ let asyncTest = Promise.resolve();
   }));
   target.dispatchEvent(new Event('foo'));
 }
+{
+  const target = new EventTarget();
+  defineEventHandler(target, 'foo');
+  const descriptor = Object.getOwnPropertyDescriptor(target, 'onfoo');
+  strictEqual(descriptor.configurable, true);
+  strictEqual(descriptor.enumerable, true);
+}
+{
+  const target = new EventTarget();
+  defineEventHandler(target, 'foo');
+  const output = [];
+  target.addEventListener('foo', () => output.push(1));
+  target.onfoo = common.mustNotCall();
+  target.addEventListener('foo', () => output.push(3));
+  target.onfoo = () => output.push(2);
+  target.addEventListener('foo', () => output.push(4));
+  target.dispatchEvent(new Event('foo'));
+  deepStrictEqual(output, [1, 2, 3, 4]);
+}

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'
     ]);
 }