#7675: Reuse SimpleEventTarget wherever possible

src/bricks/hooks/useAllBricks.ts

@@ -17,20 +17,15 @@
 
 import brickRegistry, { type TypedBrickMap } from "@/bricks/registry";
 import { isEmpty } from "lodash";
-import { type RegistryChangeListener } from "@/registry/memoryRegistry";
 import { useSyncExternalStore } from "use-sync-external-store/shim";
 import { useState } from "react";
 import { useAsyncEffect } from "use-async-effect";
 
 const subscribe = (callback: () => void) => {
-  const listener: RegistryChangeListener = {
-    onCacheChanged: callback,
-  };
-
-  brickRegistry.addListener(listener);
+  brickRegistry.onCacheChanged.add(callback);
 
   return () => {
-    brickRegistry.removeListener(listener);
+    brickRegistry.onCacheChanged.remove(callback);
   };
 };
 

src/bricks/registry.ts

@@ -43,10 +43,8 @@ class BrickRegistry extends MemoryRegistry<RegistryId, Brick> {
     // Can't reference "this" before the call to "super"
     this.setDeserialize(partial(fromJS, this));
 
-    this.addListener({
-      onCacheChanged: () => {
-        this.typeCachePromise = null;
-      },
+    this.onCacheChanged.add(() => {
+      this.typeCachePromise = null;
     });
   }
 

src/contentScript/lifecycle.ts

@@ -47,6 +47,7 @@ import {
 import { $safeFind } from "@/utils/domUtils";
 import { onContextInvalidated } from "webext-events";
 import { ContextMenuStarterBrickABC } from "@/starterBricks/contextMenu";
+import { RepeatableAbortController } from "abort-utils";
 
 /**
  * True if handling the initial page load.
@@ -92,10 +93,9 @@ const _activeExtensionPoints = new Set<StarterBrick>();
 let lastUrl: string | undefined;
 
 /**
- * Abort controllers for navigation events for Single Page Applications (SPAs).
+ * Abort controller for navigation events for Single Page Applications (SPAs).
  */
-// eslint-disable-next-line local-rules/persistBackgroundData -- Functions
-const _navigationListeners = new Set<() => void>();
+const navigationListeners = new RepeatableAbortController();
 
 const WAIT_LOADED_INTERVAL_MS = 25;
 
@@ -324,23 +324,10 @@ export function clearEditorExtension(
 }
 
 /**
- * Return an AbortSignal that's aborted when the user navigates.
- */
-function createNavigationAbortSignal(): AbortSignal {
-  const controller = new AbortController();
-  _navigationListeners.add(controller.abort.bind(controller));
-  return controller.signal;
-}
-
-/**
- * Notifies all navigation listeners that the user has navigated in a way that changed the URL.
+ * Notifies all the listeners that the user has navigated in a way that changed the URL.
  */
 function notifyNavigationListeners(): void {
-  for (const listener of _navigationListeners) {
-    listener();
-  }
-
-  _navigationListeners.clear();
+  navigationListeners.abortAndReset();
 }
 
 /**
@@ -372,7 +359,7 @@ export async function runEditorExtension(
     // The Page Editor is the only caller for runDynamic
     reason: RunReason.PAGE_EDITOR,
     extensionIds: [extensionId],
-    abortSignal: createNavigationAbortSignal(),
+    abortSignal: navigationListeners.signal,
   });
 
   checkLifecycleInvariants();
@@ -587,14 +574,12 @@ export async function handleNavigate({
   updateNavigationId();
   notifyNavigationListeners();
 
-  const abortSignal = createNavigationAbortSignal();
-
   const extensionPoints = await loadPersistedExtensionsOnce();
   if (extensionPoints.length > 0) {
     // Wait for document to load, to ensure any selector-based availability rules are ready to be applied.
     await logPromiseDuration(
       "handleNavigate:waitDocumentLoad",
-      waitDocumentLoad(abortSignal),
+      waitDocumentLoad(navigationListeners.signal),
     );
 
     // Safe to use Promise.all because the inner method can't throw
@@ -606,7 +591,7 @@ export async function handleNavigate({
           // extension point that runs on the contact information modal on LinkedIn
           const runPromise = runExtensionPoint(extensionPoint, {
             reason: runReason,
-            abortSignal,
+            abortSignal: navigationListeners.signal,
           }).catch((error) => {
             console.error("Error installing/running: %s", extensionPoint.id, {
               error,

src/hooks/useAsyncExternalStore.ts

@@ -26,11 +26,12 @@ import {
 import { type UUID } from "@/types/stringTypes";
 import { uuidv4, validateUUID } from "@/types/helpers";
 import deepEquals from "fast-deep-equal";
+import { SimpleEventTarget } from "@/utils/SimpleEventTarget";
 
 type Subscribe = (callback: () => void) => () => void;
 
 class StateController<T = unknown> {
-  private readonly stateListeners = new Set<() => void>();
+  private readonly stateListeners = new SimpleEventTarget();
   private state: AsyncState<T> = uninitializedAsyncStateFactory();
   private nonce: UUID = validateUUID(null); // TODO: Drop after strictNullCheck transition; Silences TS bug
 
@@ -48,16 +49,10 @@ class StateController<T = unknown> {
     return () => {
       // Theoretically, we could also try unsubscribing from the external source when the last listener is removed.
       // However, in practice that was causing some bugs with component lifecycle.
-      this.stateListeners.delete(callback);
+      this.stateListeners.remove(callback);
     };
   };
 
-  notifyAll(): void {
-    for (const listener of this.stateListeners) {
-      listener();
-    }
-  }
-
   getSnapshot = (): AsyncState<T> => this.state;
 
   updateSnapshot = async (): Promise<void> => {
@@ -73,7 +68,7 @@ class StateController<T = unknown> {
     this.nonce = nonce;
 
     // Inform subscribers of loading/fetching state
-    this.notifyAll();
+    this.stateListeners.emit();
 
     try {
       const data = await this.factory();
@@ -97,7 +92,7 @@ class StateController<T = unknown> {
       this.state = errorToAsyncState(error);
     }
 
-    this.notifyAll();
+    this.stateListeners.emit();
   };
 }
 

src/registry/memoryRegistry.ts

@@ -22,6 +22,7 @@ import { expectContext } from "@/utils/expectContext";
 import { type RegistryId } from "@/types/registryTypes";
 import { isInnerDefinitionRegistryId } from "@/types/helpers";
 import { memoizeUntilSettled } from "@/utils/promiseUtils";
+import { SimpleEventTarget } from "@/utils/SimpleEventTarget";
 
 type Source =
   // From the remote brick registry
@@ -45,26 +46,10 @@ export class DoesNotExistError extends Error {
   }
 }
 
-export type RegistryChangeListener = {
-  onCacheChanged: () => void;
-};
-
-type DatabaseChangeListener = {
-  onChanged: () => void;
-};
-
 /**
  * `backgroundRegistry` database change listeners.
  */
-// TODO: Use SimpleEventTarget instead
-// eslint-disable-next-line local-rules/persistBackgroundData -- Functions
-const databaseChangeListeners: DatabaseChangeListener[] = [];
-
-function notifyDatabaseListeners() {
-  for (const listener of databaseChangeListeners) {
-    listener.onChanged();
-  }
-}
+const databaseChangeListeners = new SimpleEventTarget();
 
 /**
  * Replace IDB with remote packages and notify listeners.
@@ -73,7 +58,7 @@ export const syncRemotePackages = memoizeUntilSettled(async () => {
   expectContext("extension");
 
   await backgroundRegistry.syncRemote();
-  notifyDatabaseListeners();
+  databaseChangeListeners.emit();
 });
 
 /**
@@ -83,7 +68,7 @@ export const clearPackages = async () => {
   expectContext("extension");
 
   await backgroundRegistry.clear();
-  notifyDatabaseListeners();
+  databaseChangeListeners.emit();
 };
 
 /**
@@ -134,17 +119,15 @@ class MemoryRegistry<
 
   private deserialize: ((raw: unknown) => Item) | null;
 
-  private listeners: RegistryChangeListener[] = [];
+  onCacheChanged = new SimpleEventTarget();
 
   constructor(kinds: Kind[], deserialize: ((raw: unknown) => Item) | null) {
     this.kinds = new Set(kinds);
     this.deserialize = deserialize;
 
-    databaseChangeListeners.push({
-      onChanged: () => {
-        // If database changes, clear the cache to force reloading user-defined bricks
-        this.clear();
-      },
+    databaseChangeListeners.add(() => {
+      // If database changes, clear the cache to force reloading user-defined bricks
+      this.clear();
     });
   }
 
@@ -160,28 +143,6 @@ class MemoryRegistry<
     this.deserialize = deserialize;
   }
 
-  /**
-   * Add a change listener
-   * @param listener the change listener
-   */
-  addListener(listener: RegistryChangeListener): void {
-    this.listeners.push(listener);
-  }
-
-  /**
-   * Remove a change listener
-   * @param listener the change listener
-   */
-  removeListener(listener: RegistryChangeListener): void {
-    this.listeners = this.listeners.filter((x) => x !== listener);
-  }
-
-  private notifyAll() {
-    for (const listener of this.listeners) {
-      listener.onCacheChanged();
-    }
-  }
-
   /**
    * Return true if the registry contains the given item
    * @param id the registry id
@@ -304,7 +265,7 @@ class MemoryRegistry<
       source: "internal",
       notify: false,
     });
-    this.notifyAll();
+    this.onCacheChanged.emit();
 
     this._cacheInitialized = true;
 
@@ -343,7 +304,7 @@ class MemoryRegistry<
     }
 
     if (changed && notify) {
-      this.notifyAll();
+      this.onCacheChanged.emit();
     }
   }
 
@@ -371,7 +332,7 @@ class MemoryRegistry<
     // Need to clear the whole thing, including built-ins. Listeners will often can all() to repopulate the cache.
     this._cacheInitialized = false;
     this._cache.clear();
-    this.notifyAll();
+    this.onCacheChanged.emit();
   }
 
   /**

src/starterBricks/menuItemExtension.ts

@@ -206,7 +206,7 @@ export abstract class MenuItemStarterBrickABC extends StarterBrickABC<MenuItemSt
    * @see MenuItemStarterBrickConfig.dependencies
    * @private
    */
-  private readonly cancelDependencyObservers: Map<UUID, () => void>;
+  private readonly cancelDependencyObservers = new EventTarget();
 
   /**
    * True if the extension point has been uninstalled
@@ -255,7 +255,6 @@ export abstract class MenuItemStarterBrickABC extends StarterBrickABC<MenuItemSt
     super(platform, metadata);
     this.menus = new Map<UUID, HTMLElement>();
     this.removed = new Set<UUID>();
-    this.cancelDependencyObservers = new Map<UUID, () => void>();
   }
 
   inputSchema: Schema = propertiesToSchema(
@@ -353,16 +352,13 @@ export abstract class MenuItemStarterBrickABC extends StarterBrickABC<MenuItemSt
     }
 
     for (const extension of extensions) {
-      const clear = this.cancelDependencyObservers.get(extension.id);
-      if (clear) {
-        try {
-          clear();
-        } catch {
-          console.error("Error cancelling dependency observer");
-        }
+      try {
+        this.cancelDependencyObservers.dispatchEvent(
+          new CustomEvent(extension.id),
+        );
+      } catch {
+        console.error("Error cancelling dependency observer");
       }
-
-      this.cancelDependencyObservers.delete(extension.id);
     }
   }
 
@@ -784,10 +780,7 @@ export abstract class MenuItemStarterBrickABC extends StarterBrickABC<MenuItemSt
     const { dependencies = [] } = extension.config;
 
     // Clean up old observers
-    if (this.cancelDependencyObservers.has(extension.id)) {
-      this.cancelDependencyObservers.get(extension.id)();
-      this.cancelDependencyObservers.delete(extension.id);
-    }
+    this.cancelDependencyObservers.dispatchEvent(new CustomEvent(extension.id));
 
     if (dependencies.length > 0) {
       const rerun = once(() => {
@@ -799,8 +792,8 @@ export abstract class MenuItemStarterBrickABC extends StarterBrickABC<MenuItemSt
       });
 
       const observer = new MutationObserver(rerun);
-
       const abortController = new AbortController();
+      onAbort(abortController, observer);
 
       let elementCount = 0;
       for (const dependency of dependencies) {
@@ -828,15 +821,13 @@ export abstract class MenuItemStarterBrickABC extends StarterBrickABC<MenuItemSt
         `Observing ${elementCount} element(s) for extension: ${extension.id}`,
       );
 
-      this.cancelDependencyObservers.set(extension.id, () => {
-        try {
-          observer.disconnect();
-        } catch (error) {
-          console.error("Error cancelling mutation observer", error);
-        }
-
-        abortController.abort();
-      });
+      this.cancelDependencyObservers.addEventListener(
+        extension.id,
+        () => {
+          abortController.abort();
+        },
+        { once: true },
+      );
     } else {
       console.debug(`Extension has no dependencies: ${extension.id}`);
     }