From bdad1bc4fc8efa6176cab57ad05e9989040cc804 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Mon, 2 Nov 2020 22:53:59 +0200 Subject: [PATCH] events: support event handlers on prototypes PR-URL: https://github.com/nodejs/node/pull/35931 Backport-PR-URL: https://github.com/nodejs/node/pull/38386 Reviewed-By: Anna Henningsen Reviewed-By: Antoine du Hamel Reviewed-By: Daijiro Wachi Reviewed-By: Rich Trott --- lib/internal/event_target.js | 21 ++++++++++++++------- lib/internal/worker/io.js | 5 +++-- test/parallel/test-worker-message-port.js | 6 ++---- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 5f84ffbccf449d..30ee46909e6274 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -13,6 +13,7 @@ const { Symbol, SymbolFor, SymbolToStringTag, + SafeWeakMap, SafeWeakSet, } = primordials; @@ -576,21 +577,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; - Object.defineProperty(emitter, `on${name}`, { + 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 index 319283852ce2ea..b2fed9f06ff35c 100644 --- a/lib/internal/worker/io.js +++ b/lib/internal/worker/io.js @@ -99,13 +99,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 index 6f4e09d029d168..51618e4fab1850 100644 --- 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' ]); }