Repository: denoland/deno
Pull request: fix(ext/node): fix events.once

This PR aligns the behavior of `events.once` to Node.js and enables `parallel/test-events-once.js`.

Details:
- Added validation of `options` object.
- Added patching of `kEvents` field for passed `signal` (necessary for passing compat test case)
- Added `kResistStopImmediatePropagation` symbol. That works as private option for event listener. If that option is specified, the event listener ignores `e.stopImmediatePropagation()` call from earlier listeners (Node.js also has this internal option https://github.com/nodejs/node/commit/bcd35c334ec75402ee081f1c4da128c339f70c24 and use it for internal abort listeners)

towards #29595 

Changed files:
filename: ext/node/polyfills/_events.mjs
patch: @@ -28,6 +28,7 @@
 import { primordials } from "ext:core/mod.js";
 const {
   ArrayPrototypeMap,
+  ArrayPrototypeFilter,
   ObjectDefineProperty,
   ObjectEntries,
   SafeMap,
@@ -57,7 +58,10 @@ import {
 } from "ext:deno_node/internal/validators.mjs";
 import { spliceOne } from "ext:deno_node/_utils.ts";
 import { nextTick } from "ext:deno_node/_process/process.ts";
-import { eventTargetData } from "ext:deno_web/02_event.js";
+import {
+  eventTargetData,
+  kResistStopImmediatePropagation,
+} from "ext:deno_web/02_event.js";
 
 export { addAbortListener } from "./internal/events/abort_listener.mjs";
 
@@ -874,12 +878,22 @@ export function getEventListeners(emitterOrTarget, type) {
  * @returns {Promise}
  */
 // deno-lint-ignore require-await
-export async function once(emitter, name, options = {}) {
+export async function once(emitter, name, options = kEmptyObject) {
+  validateObject(options, "options");
   const signal = options?.signal;
   validateAbortSignal(signal, "options.signal");
   if (signal?.aborted) {
     throw new AbortError();
   }
+
+  if (signal) {
+    // Patches [kEvents] field of AbortSignal to simulate Node.js EventTarget
+    // This is necessary to pass `paralle/test-events-once.js` test
+    // TODO(kt3k): This can be removed if events.getEventListeners() is used
+    // instead of `signal[kEvents]` in upstream.
+    ObjectDefineProperty(signal, kEvents, kEventsGetter);
+  }
+
   return new Promise((resolve, reject) => {
     const errorListener = (err) => {
       emitter.removeListener(name, resolver);
@@ -911,7 +925,7 @@ export async function once(emitter, name, options = {}) {
         signal,
         "abort",
         abortListener,
-        { once: true },
+        { once: true, [kResistStopImmediatePropagation]: true },
       );
     }
   });
@@ -958,9 +972,13 @@ const kEventsGetter = {
       return new SafeMap();
     }
     return new SafeMap(
-      ArrayPrototypeMap(ObjectEntries(data.listeners), (
-        [key, listeners],
-      ) => [key, new SafeSet(listeners)]),
+      ArrayPrototypeMap(
+        ArrayPrototypeFilter(
+          ObjectEntries(data.listeners),
+          ([_, listeners]) => listeners.length > 0,
+        ),
+        ([key, listeners]) => [key, new SafeSet(listeners)],
+      ),
     );
   },
 };

filename: ext/web/02_event.js
patch: @@ -728,6 +728,13 @@ function innerInvokeEventListeners(
   for (let i = 0; i < handlersLength; i++) {
     const listener = handlers[i];
 
+    if (
+      getStopImmediatePropagation(eventImpl) &&
+      !listener.options[kResistStopImmediatePropagation]
+    ) {
+      continue;
+    }
+
     let capture, once, passive;
     if (typeof listener.options === "boolean") {
       capture = listener.options;
@@ -778,10 +785,6 @@ function innerInvokeEventListeners(
     }
 
     setInPassiveListener(eventImpl, false);
-
-    if (getStopImmediatePropagation(eventImpl)) {
-      return found;
-    }
   }
 
   return found;
@@ -862,6 +865,9 @@ function retarget(a, b) {
 // Accessors for non-public data
 
 export const eventTargetData = Symbol();
+export const kResistStopImmediatePropagation = Symbol(
+  "kResistStopImmediatePropagation",
+);
 
 function setEventTargetData(target) {
   target[eventTargetData] = getDefaultTargetData();
@@ -910,6 +916,10 @@ function addEventListenerOptionsConverter(V, prefix) {
     capture: !!V.capture,
     once: !!V.once,
     passive: !!V.passive,
+    // This field exists for simulating Node.js behavior, implemented in https://github.com/nodejs/node/commit/bcd35c334ec75402ee081f1c4da128c339f70c24
+    // Some internal event listeners in Node.js can ignore `e.stopImmediatePropagation()` calls
+    // from the earlier event listeners.
+    [kResistStopImmediatePropagation]: !!V[kResistStopImmediatePropagation],
   };
 
   const signal = V.signal;

filename: tests/node_compat/config.jsonc
patch: @@ -70,7 +70,6 @@
       "test-error-aggregateTwoErrors.js",
       "test-error-prepare-stack-trace.js",
       "test-event-emitter-prepend.js",
-      "test-events-once.js",
       "test-fs-access.js",
       "test-fs-append-file.js",
       "test-fs-chmod-mask.js",

filename: tests/node_compat/test/parallel/test-events-once.js
patch: @@ -2,23 +2,21 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 18.8.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
-
-// TODO(cjihrig): kEvents is an internally used symbol in Node.js. It is not
-// implemented in Deno, so parts of this test must be removed in order to pass.
+// Taken from Node 23.9.0
+// This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.
 
 'use strict';
-// Flags: --no-warnings
+// Flags: --expose-internals --no-warnings
 
 const common = require('../common');
 const { once, EventEmitter } = require('events');
 const {
-  strictEqual,
   deepStrictEqual,
   fail,
   rejects,
+  strictEqual,
 } = require('assert');
+const { kEvents } = require('internal/event_target');
 
 async function onceAnEvent() {
   const ee = new EventEmitter();
@@ -33,18 +31,16 @@ async function onceAnEvent() {
   strictEqual(ee.listenerCount('myevent'), 0);
 }
 
-async function onceAnEventWithNullOptions() {
+async function onceAnEventWithInvalidOptions() {
   const ee = new EventEmitter();
 
-  process.nextTick(() => {
-    ee.emit('myevent', 42);
-  });
-
-  const [value] = await once(ee, 'myevent', null);
-  strictEqual(value, 42);
+  await Promise.all([1, 'hi', null, false, () => {}, Symbol(), 1n].map((options) => {
+    return rejects(once(ee, 'myevent', options), {
+      code: 'ERR_INVALID_ARG_TYPE',
+    });
+  }));
 }
 
-
 async function onceAnEventWithTwoArgs() {
   const ee = new EventEmitter();
 
@@ -89,6 +85,7 @@ async function catchesErrorsWithAbortSignal() {
   try {
     const promise = once(ee, 'myevent', { signal });
     strictEqual(ee.listenerCount('error'), 1);
+    strictEqual(signal[kEvents].size, 1);
 
     await promise;
   } catch (e) {
@@ -97,6 +94,7 @@ async function catchesErrorsWithAbortSignal() {
   strictEqual(err, expected);
   strictEqual(ee.listenerCount('error'), 0);
   strictEqual(ee.listenerCount('myevent'), 0);
+  strictEqual(signal[kEvents].size, 0);
 }
 
 async function stopListeningAfterCatchingError() {
@@ -157,6 +155,13 @@ async function onceWithEventTargetError() {
   strictEqual(err, error);
 }
 
+async function onceWithInvalidEventEmmiter() {
+  const ac = new AbortController();
+  return rejects(once(ac, 'myevent'), {
+    code: 'ERR_INVALID_ARG_TYPE',
+  });
+}
+
 async function prioritizesEventEmitter() {
   const ee = new EventEmitter();
   ee.addEventListener = fail;
@@ -172,12 +177,12 @@ async function abortSignalBefore() {
 
   await Promise.all([1, {}, 'hi', null, false].map((signal) => {
     return rejects(once(ee, 'foo', { signal }), {
-      code: 'ERR_INVALID_ARG_TYPE'
+      code: 'ERR_INVALID_ARG_TYPE',
     });
   }));
 
   return rejects(once(ee, 'foo', { signal: abortedSignal }), {
-    name: 'AbortError'
+    name: 'AbortError',
   });
 }
 
@@ -186,7 +191,7 @@ async function abortSignalAfter() {
   const ac = new AbortController();
   ee.on('error', common.mustNotCall());
   const r = rejects(once(ee, 'foo', { signal: ac.signal }), {
-    name: 'AbortError'
+    name: 'AbortError',
   });
   process.nextTick(() => ac.abort());
   return r;
@@ -200,7 +205,9 @@ async function abortSignalAfterEvent() {
     ac.abort();
   });
   const promise = once(ee, 'foo', { signal: ac.signal });
+  strictEqual(ac.signal[kEvents].size, 1);
   await promise;
+  strictEqual(ac.signal[kEvents].size, 0);
 }
 
 async function abortSignalRemoveListener() {
@@ -222,20 +229,32 @@ async function eventTargetAbortSignalBefore() {
 
   await Promise.all([1, {}, 'hi', null, false].map((signal) => {
     return rejects(once(et, 'foo', { signal }), {
-      code: 'ERR_INVALID_ARG_TYPE'
+      code: 'ERR_INVALID_ARG_TYPE',
     });
   }));
 
   return rejects(once(et, 'foo', { signal: abortedSignal }), {
-    name: 'AbortError'
+    name: 'AbortError',
+  });
+}
+
+async function eventTargetAbortSignalBeforeEvenWhenSignalPropagationStopped() {
+  const et = new EventTarget();
+  const ac = new AbortController();
+  const { signal } = ac;
+  signal.addEventListener('abort', (e) => e.stopImmediatePropagation(), { once: true });
+
+  process.nextTick(() => ac.abort());
+  return rejects(once(et, 'foo', { signal }), {
+    name: 'AbortError',
   });
 }
 
 async function eventTargetAbortSignalAfter() {
   const et = new EventTarget();
   const ac = new AbortController();
   const r = rejects(once(et, 'foo', { signal: ac.signal }), {
-    name: 'AbortError'
+    name: 'AbortError',
   });
   process.nextTick(() => ac.abort());
   return r;
@@ -253,20 +272,22 @@ async function eventTargetAbortSignalAfterEvent() {
 
 Promise.all([
   onceAnEvent(),
-  onceAnEventWithNullOptions(),
+  onceAnEventWithInvalidOptions(),
   onceAnEventWithTwoArgs(),
   catchesErrors(),
   catchesErrorsWithAbortSignal(),
   stopListeningAfterCatchingError(),
   onceError(),
   onceWithEventTarget(),
   onceWithEventTargetError(),
+  onceWithInvalidEventEmmiter(),
   prioritizesEventEmitter(),
   abortSignalBefore(),
   abortSignalAfter(),
   abortSignalAfterEvent(),
   abortSignalRemoveListener(),
   eventTargetAbortSignalBefore(),
+  eventTargetAbortSignalBeforeEvenWhenSignalPropagationStopped(),
   eventTargetAbortSignalAfter(),
   eventTargetAbortSignalAfterEvent(),
 ]).then(common.mustCall());