diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 592e46e5320ce9..b387e5f4e2cea0 100644 --- a/lib/internal/abort_controller.js +++ b/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]; } diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 12e04098320bc3..51f295c3971151 100644 --- a/lib/internal/event_target.js +++ b/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 = { diff --git a/lib/internal/worker/io.js b/lib/internal/worker/io.js index 763d06856cbd42..73958dbc2d78f5 100644 --- a/lib/internal/worker/io.js +++ b/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, diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js index accacb8a328c03..fa280027c61f29 100644 --- a/test/parallel/test-eventtarget.js +++ b/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]); +} 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' ]); }