fix(node/events): make EventEmitter's public methods enumerable (deno…

…land#1530)
traceypooh · Nov 14, 2021 · a5578f0 · a5578f0
1 parent 95aca64
commit a5578f0
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 69 deletions.
diff --git a/node/_utils.ts b/node/_utils.ts
@@ -250,3 +250,17 @@ export async function assertCallbackErrorUncaught(
   assert(!status.success);
   assertStringIncludes(stderr, "Error: success");
 }
+
+export function makeMethodsEnumerable(klass: { new (): unknown }): void {
+  const proto = klass.prototype;
+  for (const key of Object.getOwnPropertyNames(proto)) {
+    const value = proto[key];
+    if (typeof value === "function") {
+      const desc = Reflect.getOwnPropertyDescriptor(proto, key);
+      if (desc) {
+        desc.enumerable = true;
+        Object.defineProperty(proto, key, desc);
+      }
+    }
+  }
+}
diff --git a/node/events.ts b/node/events.ts
@@ -22,7 +22,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 import { assert } from "../_util/assert.ts";
-import { notImplemented } from "./_utils.ts";
+import { makeMethodsEnumerable, notImplemented } from "./_utils.ts";
 import {
   ERR_INVALID_ARG_TYPE,
   ERR_OUT_OF_RANGE,
@@ -221,30 +221,9 @@ export class EventEmitter {
     return emitter.listenerCount(eventName);
   }
 
-  private _listeners(
-    target: EventEmitter,
-    eventName: string | symbol,
-    unwrap: boolean,
-  ): GenericFunction[] {
-    if (!hasListeners(target._events, eventName)) {
-      return [];
-    }
-
-    const eventListeners = target._events[eventName];
-    if (Array.isArray(eventListeners)) {
-      return unwrap
-        ? unwrapListeners(eventListeners)
-        : eventListeners.slice(0) as GenericFunction[];
-    } else {
-      return [
-        unwrap ? unwrapListener(eventListeners) : eventListeners,
-      ] as GenericFunction[];
-    }
-  }
-
   /** Returns a copy of the array of listeners for the event named eventName.*/
   public listeners(eventName: string | symbol): GenericFunction[] {
-    return this._listeners(this, eventName, true);
+    return listeners(this._events, eventName, true);
   }
 
   /**
@@ -254,7 +233,7 @@ export class EventEmitter {
   public rawListeners(
     eventName: string | symbol,
   ): Array<GenericFunction | WrappedFunction> {
-    return this._listeners(this, eventName, false);
+    return listeners(this._events, eventName, false);
   }
 
   /** Alias for emitter.removeListener(). */
@@ -294,54 +273,11 @@ export class EventEmitter {
    * time eventName is triggered, this listener is removed and then invoked.
    */
   public once(eventName: string | symbol, listener: GenericFunction): this {
-    const wrapped: WrappedFunction = this.onceWrap(eventName, listener);
+    const wrapped: WrappedFunction = onceWrap(this, eventName, listener);
     this.on(eventName, wrapped);
     return this;
   }
 
-  // Wrapped function that calls EventEmitter.removeListener(eventName, self) on execution.
-  private onceWrap(
-    eventName: string | symbol,
-    listener: GenericFunction,
-  ): WrappedFunction {
-    checkListenerArgument(listener);
-    const wrapper = function (
-      this: {
-        eventName: string | symbol;
-        listener: GenericFunction;
-        rawListener: GenericFunction | WrappedFunction;
-        context: EventEmitter;
-        isCalled?: boolean;
-      },
-      // deno-lint-ignore no-explicit-any
-      ...args: any[]
-    ): void {
-      // If `emit` is called in listeners, the same listener can be called multiple times.
-      // To prevent that, check the flag here.
-      if (this.isCalled) {
-        return;
-      }
-      this.context.removeListener(
-        this.eventName,
-        this.listener as GenericFunction,
-      );
-      this.isCalled = true;
-      return this.listener.apply(this.context, args);
-    };
-    const wrapperContext = {
-      eventName: eventName,
-      listener: listener,
-      rawListener: (wrapper as unknown) as WrappedFunction,
-      context: this,
-    };
-    const wrapped = (wrapper.bind(
-      wrapperContext,
-    ) as unknown) as WrappedFunction;
-    wrapperContext.rawListener = wrapped;
-    wrapped.listener = listener;
-    return wrapped as WrappedFunction;
-  }
-
   /**
    * Adds the listener function to the beginning of the listeners array for the
    *  event named eventName. No checks are made to see if the listener has
@@ -365,7 +301,7 @@ export class EventEmitter {
     eventName: string | symbol,
     listener: GenericFunction,
   ): this {
-    const wrapped: WrappedFunction = this.onceWrap(eventName, listener);
+    const wrapped: WrappedFunction = onceWrap(this, eventName, listener);
     this.prependListener(eventName, wrapped);
     return this;
   }
@@ -716,6 +652,27 @@ function hasListeners(
   return maybeEvents != null && Boolean(maybeEvents[eventName]);
 }
 
+function listeners(
+  events: EventMap,
+  eventName: string | symbol,
+  unwrap: boolean,
+): GenericFunction[] {
+  if (!hasListeners(events, eventName)) {
+    return [];
+  }
+
+  const eventListeners = events[eventName];
+  if (Array.isArray(eventListeners)) {
+    return unwrap
+      ? unwrapListeners(eventListeners)
+      : eventListeners.slice(0) as GenericFunction[];
+  } else {
+    return [
+      unwrap ? unwrapListener(eventListeners) : eventListeners,
+    ] as GenericFunction[];
+  }
+}
+
 function unwrapListeners(
   arr: (GenericFunction | WrappedFunction)[],
 ): GenericFunction[] {
@@ -732,6 +689,50 @@ function unwrapListener(
   return (listener as WrappedFunction)["listener"] ?? listener;
 }
 
+// Wrapped function that calls EventEmitter.removeListener(eventName, self) on execution.
+function onceWrap(
+  target: EventEmitter,
+  eventName: string | symbol,
+  listener: GenericFunction,
+): WrappedFunction {
+  checkListenerArgument(listener);
+  const wrapper = function (
+    this: {
+      eventName: string | symbol;
+      listener: GenericFunction;
+      rawListener: GenericFunction | WrappedFunction;
+      context: EventEmitter;
+      isCalled?: boolean;
+    },
+    // deno-lint-ignore no-explicit-any
+    ...args: any[]
+  ): void {
+    // If `emit` is called in listeners, the same listener can be called multiple times.
+    // To prevent that, check the flag here.
+    if (this.isCalled) {
+      return;
+    }
+    this.context.removeListener(
+      this.eventName,
+      this.listener as GenericFunction,
+    );
+    this.isCalled = true;
+    return this.listener.apply(this.context, args);
+  };
+  const wrapperContext = {
+    eventName: eventName,
+    listener: listener,
+    rawListener: (wrapper as unknown) as WrappedFunction,
+    context: target,
+  };
+  const wrapped = (wrapper.bind(
+    wrapperContext,
+  ) as unknown) as WrappedFunction;
+  wrapperContext.rawListener = wrapped;
+  wrapped.listener = listener;
+  return wrapped as WrappedFunction;
+}
+
 // EventEmitter#on should point to the same function as EventEmitter#addListener.
 EventEmitter.prototype.on = EventEmitter.prototype.addListener;
 // EventEmitter#off should point to the same function as EventEmitter#removeListener.
@@ -755,6 +756,8 @@ class MaxListenersExceededWarning extends Error {
   }
 }
 
+makeMethodsEnumerable(EventEmitter);
+
 export default Object.assign(EventEmitter, { EventEmitter, setMaxListeners });
 
 export const captureRejectionSymbol = EventEmitter.captureRejectionSymbol;

diff --git a/node/events_test.ts b/node/events_test.ts
@@ -1,6 +1,7 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 import {
   assert,
+  assertArrayIncludes,
   assertEquals,
   assertThrows,
   fail,
@@ -735,3 +736,14 @@ Deno.test("EventEmitter.setMaxListeners: it sets `n` as number of max listeners
     "defaultMaxListeners shouldn't be mutated.",
   );
 });
+
+// https://github.com/denoland/deno_std/issues/1511
+Deno.test("EventEmitter's public methods should be enumerable", () => {
+  const keys = Object.keys(EventEmitter.prototype);
+  assertArrayIncludes(keys, [
+    "emit",
+    "on",
+    "once",
+    "off",
+  ]);
+});