events: fire handlers in correct oder

benjamingr · danielleadams · commit 9e673723e392 · 2020-11-09T13:35:03.000-05:00
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>
diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
@@ -9,6 +9,7 @@ const {
   ObjectDefineProperties,
   ObjectDefineProperty,
   ObjectGetOwnPropertyDescriptor,
+  ReflectApply,
   SafeMap,
   String,
   Symbol,
@@ -578,24 +579,47 @@ 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
   ObjectDefineProperty(emitter, `on${name}`, {
     get() {
-      return this[kHandlers]?.get(name);
+      return this[kHandlers]?.get(name)?.handler;
     },
     set(value) {
-      const oldValue = this[kHandlers]?.get(name);
-      if (oldValue) {
-        this.removeEventListener(name, oldValue);
-      }
-      if (typeof value === 'function') {
-        this.addEventListener(name, value);
-      }
       if (!this[kHandlers]) {
         this[kHandlers] = new SafeMap();
       }
-      this[kHandlers].set(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);
+      }
+      this[kHandlers].set(name, wrappedHandler);
     },
     configurable: true,
     enumerable: true
diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js
@@ -524,3 +524,15 @@ let asyncTest = Promise.resolve();
   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]);
+}
