From adf41edda12a26a84cb8b4252404aae2a9e7ae03 Mon Sep 17 00:00:00 2001 From: Koen <41021050+vrugtehagel@users.noreply.github.com> Date: Mon, 5 Jun 2023 02:03:44 +0200 Subject: [PATCH] fix(ext/web): Copy EventTarget list before dispatch (#19360) Related issue: https://github.com/denoland/deno/issues/19358. This is a regression that seems to have been introduced in https://github.com/denoland/deno/pull/18905. It looks to have been a performance optimization. The issue is probably easiest described with some code: ```ts const target = new EventTarget(); const event = new Event("foo"); target.addEventListener("foo", () => { console.log('base'); target.addEventListener("foo", () => { console.log('nested'); }); }); target.dispatchEvent(event); ``` Essentially, the second event listener is being attached while the `foo` event is still being dispatched. It should then not fire that second event listener, but Deno currently does. --- cli/tests/unit/event_target_test.ts | 14 ++++++++++++++ ext/web/02_event.js | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/cli/tests/unit/event_target_test.ts b/cli/tests/unit/event_target_test.ts index 49bd354aa2..c7acab364c 100644 --- a/cli/tests/unit/event_target_test.ts +++ b/cli/tests/unit/event_target_test.ts @@ -245,6 +245,20 @@ Deno.test(function eventTargetDispatchShouldSetTargetInListener() { assertEquals(called, true); }); +Deno.test(function eventTargetDispatchShouldFireCurrentListenersOnly() { + const target = new EventTarget(); + const event = new Event("foo"); + let callCount = 0; + target.addEventListener("foo", () => { + ++callCount; + target.addEventListener("foo", () => { + ++callCount; + }); + }); + target.dispatchEvent(event); + assertEquals(callCount, 1); +}); + Deno.test(function eventTargetAddEventListenerGlobalAbort() { return new Promise((resolve) => { const c = new AbortController(); diff --git a/ext/web/02_event.js b/ext/web/02_event.js index e7553ea89b..142fa66b2b 100644 --- a/ext/web/02_event.js +++ b/ext/web/02_event.js @@ -737,13 +737,14 @@ function innerInvokeEventListeners( } let handlers = targetListeners[type]; + const handlersLength = handlers.length; // Copy event listeners before iterating since the list can be modified during the iteration. - if (handlers.length > 1) { + if (handlersLength > 1) { handlers = ArrayPrototypeSlice(targetListeners[type]); } - for (let i = 0; i < handlers.length; i++) { + for (let i = 0; i < handlersLength; i++) { const listener = handlers[i]; let capture, once, passive;