Skip to content

Commit 9e67372

Browse files
benjamingrdanielleadams
authored andcommitted
events: fire handlers in correct oder
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>
1 parent ff59fcd commit 9e67372

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
@@ -9,6 +9,7 @@ const {
99
ObjectDefineProperties,
1010
ObjectDefineProperty,
1111
ObjectGetOwnPropertyDescriptor,
12+
ReflectApply,
1213
SafeMap,
1314
String,
1415
Symbol,
@@ -578,24 +579,47 @@ function emitUnhandledRejectionOrErr(that, err, event) {
578579
process.emit('error', err, event);
579580
}
580581

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