#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.onChange.add(callback);
 
   return () => {
-    brickRegistry.removeListener(listener);
+    brickRegistry.onChange.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.onChange.add(() => {
+      this.typeCachePromise = null;
     });
   }
 

src/components/fields/schemaFields/widgets/varPopup/VarMenu.module.scss

@@ -42,21 +42,19 @@ $max-menu-height: 300px;
   top: 0;
   bottom: 0;
 
-  :global(.active:not(.hover)) {
+  :global(.active):not(:hover) {
     background-color: $color-active;
-
     ul {
       background-color: white;
     }
   }
 
-  :global(.hover) {
-    background-color: $color-hover !important;
-    cursor: pointer;
-
+  // Highlight the exact item being hovered, but
+  // don't highlight it when its children are hovered
+  li:hover:not(:has(li:hover)) {
+    background-color: $color-hover;
     ul {
       background-color: white;
-      cursor: none;
     }
   }
 

src/components/fields/schemaFields/widgets/varPopup/useTreeRow.ts

@@ -17,34 +17,6 @@
 
 import { type MutableRefObject, useEffect } from "react";
 
-type HoverListener = {
-  onMouseEnter: (element: HTMLElement) => void;
-  onMouseExit: (element: HTMLElement) => void;
-};
-
-// OK to use module-level tracking, because there will only ever be one VariablesTree rendered at a time
-const hoverListeners = new Set<HoverListener>();
-
-function addHoverListener(listener: HoverListener): void {
-  hoverListeners.add(listener);
-}
-
-function removeHoverListener(listener: HoverListener): void {
-  hoverListeners.delete(listener);
-}
-
-function notifyMouseEnter(element: HTMLElement): void {
-  for (const listener of hoverListeners) {
-    listener.onMouseEnter(element);
-  }
-}
-
-function notifyMouseExit(element: HTMLElement): void {
-  for (const listener of hoverListeners) {
-    listener.onMouseExit(element);
-  }
-}
-
 /**
  * A hack to make JSON Tree rows clickable/highlightable.
  * @param buttonRef ref for the label in the row
@@ -63,57 +35,25 @@ function useTreeRow({
   useEffect(() => {
     if (buttonRef.current) {
       // Find the containing row in the JSONTree
-      const $row = $(buttonRef.current).closest("li");
-      const row = $row.get(0);
-
-      $row.click((event) => {
-        if (event.target === row) {
-          event.preventDefault();
-          event.stopPropagation();
-
-          onSelect();
-        }
-      });
-
-      $row.mouseenter(() => {
-        notifyMouseEnter(row);
-      });
-
-      $row.mouseover((event) => {
-        if (event.target === row) {
-          notifyMouseEnter(row);
-        }
-
-        event.stopPropagation();
-      });
-
-      $row.mouseleave(() => {
-        notifyMouseExit(row);
-      });
-
-      const listener: HoverListener = {
-        onMouseEnter(element) {
-          if (element === row) {
-            $row.addClass("hover");
-          } else {
-            // Handle the case where user moves mouse over the nested properties. It's still over the parent, so
-            // we wouldn't have seen a mousexit event yet
-            $row.removeClass("hover");
-          }
-        },
-        onMouseExit(element) {
-          if (element === row) {
-            $row.removeClass("hover");
+      const row = buttonRef.current.closest("li");
+
+      const controller = new AbortController();
+      row.addEventListener(
+        "click",
+        (event) => {
+          // Only call onSelect if the click was on the row itself, not its children
+          if (event.target === row) {
+            event.preventDefault();
+            event.stopPropagation();
+
+            onSelect();
           }
         },
-      };
-
-      addHoverListener(listener);
+        { signal: controller.signal },
+      );
 
       return () => {
-        removeHoverListener(listener);
-        $row.off("click");
-        $row.off("mouseenter");
+        controller.abort();
       };
     }
   }, [buttonRef, onSelect]);

src/components/quickBar/quickBarRegistry.ts

@@ -20,7 +20,6 @@ import { type RegistryId } from "@/types/registryTypes";
 import { pull, remove } from "lodash";
 import {
   type ActionGenerator,
-  type ActionsChangeHandler,
   type GeneratorArgs,
 } from "@/components/quickBar/quickbarTypes";
 import { allSettled } from "@/utils/promiseUtils";
@@ -29,6 +28,7 @@ import type {
   CustomAction,
   QuickBarProtocol,
 } from "@/platform/platformTypes/quickBarProtocol";
+import { SimpleEventTarget } from "@/utils/SimpleEventTarget";
 
 class QuickBarRegistry implements QuickBarProtocol {
   /**
@@ -41,9 +41,8 @@ class QuickBarRegistry implements QuickBarProtocol {
 
   /**
    * Registry of action listeners, called when the set of actions changes.
-   * @private
    */
-  private readonly listeners: ActionsChangeHandler[] = [];
+  readonly changeEvent = new SimpleEventTarget<CustomAction[]>();
 
   /**
    * Registry of action generators. The generators are called when the user types in the Quick Bar.
@@ -74,15 +73,12 @@ class QuickBarRegistry implements QuickBarProtocol {
 
   /**
    * Helper method to notify all action listeners that the set of actions changed.
-   * @private
    */
   private notifyListeners() {
     // Need to copy the array because the registry mutates the array in-place, and listeners might be keeping a
     // reference to the argument passed to them
     const copy = [...this.actions];
-    for (const listener of this.listeners) {
-      listener(copy);
-    }
+    this.changeEvent.emit(copy);
   }
 
   /**
@@ -151,22 +147,6 @@ class QuickBarRegistry implements QuickBarProtocol {
     this.notifyListeners();
   }
 
-  /**
-   * Add an action change handler.
-   * @param handler the action change handler
-   */
-  addListener(handler: ActionsChangeHandler): void {
-    this.listeners.push(handler);
-  }
-
-  /**
-   * Remove an action change handler.
-   * @param handler the action change handler
-   */
-  removeListener(handler: ActionsChangeHandler): void {
-    pull(this.listeners, handler);
-  }
-
   /**
    * Register an action generator.
    * @param generator the action generator

src/components/quickBar/quickbarTypes.ts

@@ -15,16 +15,6 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import type { CustomAction } from "@/platform/platformTypes/quickBarProtocol";
-
-/**
- * Handler for when the set of registered actions changes
- *
- * @see QuickBarRegistry.addListener
- * @see QuickBarRegistry.removeListener
- */
-export type ActionsChangeHandler = (activeActions: CustomAction[]) => void;
-
 /**
  * Shape of arguments passed to action generators for dynamic QuickBar action generator.
  *

src/components/quickBar/useActions.ts

@@ -40,9 +40,9 @@ function useActions(): void {
     // from the initial mount
     handler(quickBarRegistry.currentActions);
 
-    quickBarRegistry.addListener(handler);
+    quickBarRegistry.changeEvent.add(handler);
     return () => {
-      quickBarRegistry.removeListener(handler);
+      quickBarRegistry.changeEvent.remove(handler);
     };
     // eslint-disable-next-line react-hooks/exhaustive-deps -- the query is available on initial mount
   }, []);

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();
   };
 }