Allow add/removeEventListener/dispatchEvent in modules, closes #207

cloudflare · Sep 3, 2022 · e606fca · e606fca
1 parent b3e187d
commit e606fca
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 52 deletions.
diff --git a/packages/core/src/standards/event.ts b/packages/core/src/standards/event.ts
@@ -5,7 +5,6 @@ import {
   MiniflareError,
   ThrowingEventTarget,
   TypedEventListener,
-  ValueOf,
   prefixError,
 } from "@miniflare/shared";
 import { Response as BaseResponse } from "undici";
@@ -177,6 +176,15 @@ export type WorkerGlobalScopeEventMap = {
   rejectionhandled: PromiseRejectionEvent;
 };
 
+function isSpecialEventType(type: string) {
+  return (
+    type === "fetch" ||
+    type === "scheduled" ||
+    type === "trace" ||
+    type === "queue"
+  );
+}
+
 export class WorkerGlobalScope extends ThrowingEventTarget<WorkerGlobalScopeEventMap> {}
 
 // true will be added to this set if #logUnhandledRejections is true so we
@@ -269,10 +277,9 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
     listener: TypedEventListener<WorkerGlobalScopeEventMap[Type]> | null,
     options?: AddEventListenerOptions | boolean
   ): void => {
-    if (this.#modules) {
-      throw new TypeError(
-        "Global addEventListener() cannot be used in modules. Instead, event " +
-          "handlers should be declared as exports on the root module."
+    if (this.#modules && isSpecialEventType(type)) {
+      return this.#log.warn(
+        `When using module syntax, the '${type}' event handler should be declared as an exported function on the root module as opposed to using the global addEventListener().`
       );
     }
 
@@ -295,12 +302,7 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
     listener: TypedEventListener<WorkerGlobalScopeEventMap[Type]> | null,
     options?: EventListenerOptions | boolean
   ): void => {
-    if (this.#modules) {
-      throw new TypeError(
-        "Global removeEventListener() cannot be used in modules. Instead, event " +
-          "handlers should be declared as exports on the root module."
-      );
-    }
+    if (this.#modules && isSpecialEventType(type)) return;
 
     // Unregister process wide rejectionHandled/unhandledRejection listeners if
     // no longer needed and not already done so
@@ -314,16 +316,6 @@ export class ServiceWorkerGlobalScope extends WorkerGlobalScope {
     super.removeEventListener(type, listener, options);
   };
 
-  dispatchEvent = (event: ValueOf<WorkerGlobalScopeEventMap>): boolean => {
-    if (this.#modules) {
-      throw new TypeError(
-        "Global dispatchEvent() cannot be used in modules. Instead, event " +
-          "handlers should be declared as exports on the root module."
-      );
-    }
-    return super.dispatchEvent(event);
-  };
-
   [kAddModuleFetchListener](listener: ModuleFetchListener): void {
     this.#calledAddFetchEventListener = true;
     super.addEventListener("fetch", (e) => {

diff --git a/packages/core/test/standards/event.spec.ts b/packages/core/test/standards/event.spec.ts
@@ -13,7 +13,7 @@ import {
   kDispatchFetch,
   kDispatchScheduled,
 } from "@miniflare/core";
-import { NoOpLog } from "@miniflare/shared";
+import { LogLevel, NoOpLog } from "@miniflare/shared";
 import {
   TestLog,
   getObjectProperties,
@@ -66,42 +66,38 @@ test("ServiceWorkerGlobalScope: includes environment in globals if modules disab
       /^env is not defined\.\nAttempted to access binding using global in modules/,
   });
 });
-test("ServiceWorkerGlobalScope: addEventListener: disabled if modules enabled", (t) => {
-  const globalScope = new ServiceWorkerGlobalScope(new NoOpLog(), {}, {}, true);
-  t.throws(() => globalScope.addEventListener("fetch", () => {}), {
-    instanceOf: TypeError,
-    message:
-      "Global addEventListener() cannot be used in modules. Instead, event " +
-      "handlers should be declared as exports on the root module.",
-  });
-});
-test("ServiceWorkerGlobalScope: removeEventListener: disabled if modules enabled", (t) => {
-  const globalScope = new ServiceWorkerGlobalScope(new NoOpLog(), {}, {}, true);
-  t.throws(() => globalScope.removeEventListener("fetch", () => {}), {
-    instanceOf: TypeError,
-    message:
-      "Global removeEventListener() cannot be used in modules. Instead, event " +
-      "handlers should be declared as exports on the root module.",
-  });
-});
-test("ServiceWorkerGlobalScope: dispatchEvent: disabled if modules enabled", (t) => {
-  const globalScope = new ServiceWorkerGlobalScope(new NoOpLog(), {}, {}, true);
-  const event = new FetchEvent("fetch", {
-    request: new Request("http://localhost"),
-  });
-  t.throws(() => globalScope.dispatchEvent(event), {
-    instanceOf: TypeError,
-    message:
-      "Global dispatchEvent() cannot be used in modules. Instead, event " +
-      "handlers should be declared as exports on the root module.",
-  });
+test("ServiceWorkerGlobalScope: addEventListener: warns for special events if modules enabled", async (t) => {
+  let customEvents = 0;
+  const customEventListener = () => customEvents++;
+
+  const log = new TestLog();
+  const globalScope = new ServiceWorkerGlobalScope(log, {}, {}, true);
+
+  globalScope.addEventListener("fetch", () => {});
+  globalScope.addEventListener("scheduled", () => {});
+  globalScope.addEventListener("trace" as any, () => {});
+  globalScope.addEventListener("queue" as any, () => {});
+  globalScope.addEventListener("custom" as any, customEventListener);
+  const warningMessage = (type: string) =>
+    `When using module syntax, the '${type}' event handler should be declared as an exported function on the root module as opposed to using the global addEventListener().`;
+  t.deepEqual(log.logsAtLevel(LogLevel.WARN), [
+    warningMessage("fetch"),
+    warningMessage("scheduled"),
+    warningMessage("trace"),
+    warningMessage("queue"),
+  ]);
+
+  globalScope.dispatchEvent(new Event("custom") as any);
+  t.is(customEvents, 1);
+  globalScope.removeEventListener("custom" as any, customEventListener);
+  globalScope.dispatchEvent(new Event("custom") as any);
+  t.is(customEvents, 1);
 });
 test("ServiceWorkerGlobalScope: hides implementation details", (t) => {
   const { globalScope } = t.context;
   t.deepEqual(getObjectProperties(globalScope), [
     "KEY", // binding
     "addEventListener",
-    "dispatchEvent",
     "global",
     "removeEventListener",
     "self",