Skip to content

Commit daad521

Browse files
benjamingrtargos
authored andcommitted
events: fire handlers in correct oder
PR-URL: #35931 Backport-PR-URL: #38386 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>
1 parent 7c95cfc commit daad521

File tree

2 files changed

+45
-9
lines changed

2 files changed

+45
-9
lines changed

lib/internal/event_target.js

+33-9
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
Object,
99
ObjectDefineProperty,
1010
ObjectGetOwnPropertyDescriptor,
11+
ReflectApply,
1112
SafeMap,
1213
String,
1314
Symbol,
@@ -577,24 +578,47 @@ function emitUnhandledRejectionOrErr(that, err, event) {
577578
process.emit('error', err, event);
578579
}
579580

581+
function makeEventHandler(handler) {
582+
// Event handlers are dispatched in the order they were first set
583+
// See https://github.com/nodejs/node/pull/35949#issuecomment-722496598
584+
function eventHandler(...args) {
585+
if (typeof eventHandler.handler !== 'function') {
586+
return;
587+
}
588+
return ReflectApply(eventHandler.handler, this, args);
589+
}
590+
eventHandler.handler = handler;
591+
return eventHandler;
592+
}
593+
580594
function defineEventHandler(emitter, name) {
581595
// 8.1.5.1 Event handlers - basically `on[eventName]` attributes
582596
ObjectDefineProperty(emitter, `on${name}`, {
583597
get() {
584-
return this[kHandlers]?.get(name);
598+
return this[kHandlers]?.get(name)?.handler;
585599
},
586600
set(value) {
587-
const oldValue = this[kHandlers]?.get(name);
588-
if (oldValue) {
589-
this.removeEventListener(name, oldValue);
590-
}
591-
if (typeof value === 'function') {
592-
this.addEventListener(name, value);
593-
}
594601
if (!this[kHandlers]) {
595602
this[kHandlers] = new SafeMap();
596603
}
597-
this[kHandlers].set(name, value);
604+
let wrappedHandler = this[kHandlers]?.get(name);
605+
if (wrappedHandler) {
606+
if (typeof wrappedHandler.handler === 'function') {
607+
this[kEvents].get(name).size--;
608+
const size = this[kEvents].get(name).size;
609+
this[kRemoveListener](size, name, wrappedHandler.handler, false);
610+
}
611+
wrappedHandler.handler = value;
612+
if (typeof wrappedHandler.handler === 'function') {
613+
this[kEvents].get(name).size++;
614+
const size = this[kEvents].get(name).size;
615+
this[kNewListener](size, name, value, false, false, false);
616+
}
617+
} else {
618+
wrappedHandler = makeEventHandler(value);
619+
this.addEventListener(name, wrappedHandler);
620+
}
621+
this[kHandlers].set(name, wrappedHandler);
598622
},
599623
configurable: true,
600624
enumerable: true

test/parallel/test-eventtarget.js

+12
Original file line numberDiff line numberDiff line change
@@ -524,3 +524,15 @@ let asyncTest = Promise.resolve();
524524
strictEqual(descriptor.configurable, true);
525525
strictEqual(descriptor.enumerable, true);
526526
}
527+
{
528+
const target = new EventTarget();
529+
defineEventHandler(target, 'foo');
530+
const output = [];
531+
target.addEventListener('foo', () => output.push(1));
532+
target.onfoo = common.mustNotCall();
533+
target.addEventListener('foo', () => output.push(3));
534+
target.onfoo = () => output.push(2);
535+
target.addEventListener('foo', () => output.push(4));
536+
target.dispatchEvent(new Event('foo'));
537+
deepStrictEqual(output, [1, 2, 3, 4]);
538+
}

0 commit comments

Comments
 (0)