diff --git a/src/bricks/hooks/useAllBricks.ts b/src/bricks/hooks/useAllBricks.ts index e9cdc4394f..aa5c8561cd 100644 --- a/src/bricks/hooks/useAllBricks.ts +++ b/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); }; }; diff --git a/src/bricks/registry.ts b/src/bricks/registry.ts index 2356929763..4211e585f3 100644 --- a/src/bricks/registry.ts +++ b/src/bricks/registry.ts @@ -43,10 +43,8 @@ class BrickRegistry extends MemoryRegistry { // 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; }); } diff --git a/src/components/fields/schemaFields/widgets/varPopup/VarMenu.module.scss b/src/components/fields/schemaFields/widgets/varPopup/VarMenu.module.scss index dc11e2811e..fe6572edcd 100644 --- a/src/components/fields/schemaFields/widgets/varPopup/VarMenu.module.scss +++ b/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; } } diff --git a/src/components/fields/schemaFields/widgets/varPopup/useTreeRow.ts b/src/components/fields/schemaFields/widgets/varPopup/useTreeRow.ts index 6553bbe5ef..713feb79a4 100644 --- a/src/components/fields/schemaFields/widgets/varPopup/useTreeRow.ts +++ b/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(); - -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]); diff --git a/src/components/quickBar/quickBarRegistry.ts b/src/components/quickBar/quickBarRegistry.ts index 68b9949a10..9bc7ed3524 100644 --- a/src/components/quickBar/quickBarRegistry.ts +++ b/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(); /** * 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 diff --git a/src/components/quickBar/quickbarTypes.ts b/src/components/quickBar/quickbarTypes.ts index 0ac6100ce8..8d69e2be3b 100644 --- a/src/components/quickBar/quickbarTypes.ts +++ b/src/components/quickBar/quickbarTypes.ts @@ -15,16 +15,6 @@ * along with this program. If not, see . */ -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. * diff --git a/src/components/quickBar/useActions.ts b/src/components/quickBar/useActions.ts index 18035aa8ff..903b86c584 100644 --- a/src/components/quickBar/useActions.ts +++ b/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 }, []); diff --git a/src/contentScript/lifecycle.ts b/src/contentScript/lifecycle.ts index b261afd77a..5d7e71c3a9 100644 --- a/src/contentScript/lifecycle.ts +++ b/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(); 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, diff --git a/src/hooks/useAsyncExternalStore.ts b/src/hooks/useAsyncExternalStore.ts index d3f6f83019..fa2891101c 100644 --- a/src/hooks/useAsyncExternalStore.ts +++ b/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 { - private readonly stateListeners = new Set<() => void>(); + private readonly stateListeners = new SimpleEventTarget(); private state: AsyncState = uninitializedAsyncStateFactory(); private nonce: UUID = validateUUID(null); // TODO: Drop after strictNullCheck transition; Silences TS bug @@ -48,16 +49,10 @@ class StateController { 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 => this.state; updateSnapshot = async (): Promise => { @@ -73,7 +68,7 @@ class StateController { 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 { this.state = errorToAsyncState(error); } - this.notifyAll(); + this.stateListeners.emit(); }; } diff --git a/src/registry/memoryRegistry.ts b/src/registry/memoryRegistry.ts index 8a6b2fb296..558e315a7b 100644 --- a/src/registry/memoryRegistry.ts +++ b/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 packageRegistryChange = 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(); + packageRegistryChange.emit(); }); /** @@ -83,7 +68,7 @@ export const clearPackages = async () => { expectContext("extension"); await backgroundRegistry.clear(); - notifyDatabaseListeners(); + packageRegistryChange.emit(); }; /** @@ -134,17 +119,15 @@ class MemoryRegistry< private deserialize: ((raw: unknown) => Item) | null; - private listeners: RegistryChangeListener[] = []; + onChange = 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(); - }, + packageRegistryChange.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.onChange.emit(); this._cacheInitialized = true; @@ -343,7 +304,7 @@ class MemoryRegistry< } if (changed && notify) { - this.notifyAll(); + this.onChange.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.onChange.emit(); } /** diff --git a/src/utils/SimpleEventTarget.ts b/src/utils/SimpleEventTarget.ts index eb0537a614..6c02858edf 100644 --- a/src/utils/SimpleEventTarget.ts +++ b/src/utils/SimpleEventTarget.ts @@ -62,6 +62,7 @@ export class SimpleEventTarget extends EventTarget { this.removeEventListener(this.coreEvent, this.getNativeListener(callback)); } + // TODO: Enforce detail, unless it's `undefined` emit(detail?: Detail): void { this.dispatchEvent(new CustomEvent(this.coreEvent, { detail })); } diff --git a/webpack.config.mjs b/webpack.config.mjs index 0d809cd19f..3c304209f4 100644 --- a/webpack.config.mjs +++ b/webpack.config.mjs @@ -286,7 +286,10 @@ const createConfig = (env, options) => }), new DiscardFilePlugin(), - isHMR && new ReactRefreshWebpackPlugin(), + isHMR && + new ReactRefreshWebpackPlugin({ + overlay: false, + }), ]), module: { rules: [