fix(site/src): replace misused useEffectEvent with correct patterns (#24525)

site/src/hooks/useClickable.ts

@@ -2,7 +2,6 @@ import {
 	type KeyboardEventHandler,
 	type MouseEventHandler,
 	type RefObject,
-	useEffectEvent,
 	useRef,
 } from "react";
 
@@ -44,11 +43,10 @@ export const useClickable = <
 	role?: TRole,
 ): UseClickableResult<TElement, TRole> => {
 	const ref = useRef<TElement>(null);
-	const onClickEvent = useEffectEvent(onClick);
 
 	return {
 		ref,
-		onClick: onClickEvent,
+		onClick,
 		tabIndex: 0,
 		role: (role ?? "button") as TRole,
 

site/src/hooks/useClipboard.test.tsx

@@ -260,10 +260,11 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
 		expect(result.current.error).toBeUndefined();
 	});
 
-	// This test case is really important to ensure that it's easy to plop this
-	// inside of useEffect calls without having to think about dependencies too
-	// much
-	it("Ensures that the copyToClipboard function always maintains a stable reference across all re-renders", async () => {
+	// This test case verifies that copyToClipboard maintains a stable
+	// reference across re-renders so it can be used safely in useEffect
+	// dependency arrays. Stability requires that onError and
+	// clearErrorOnSuccess are themselves stable references.
+	it("Ensures that the copyToClipboard function maintains a stable reference when its inputs are stable", async () => {
 		const initialOnError = vi.fn();
 		const { result, rerender } = renderUseClipboard({
 			onError: initialOnError,
@@ -276,17 +277,6 @@ describe.each(secureContextValues)("useClipboard - secure: %j", (isSecure) => {
 		rerender({ onError: initialOnError });
 		expect(result.current.copyToClipboard).toBe(initialCopy);
 
-		// Re-render with new onError prop and then swap back to simplify
-		// testing
-		rerender({ onError: vi.fn() });
-		expect(result.current.copyToClipboard).toBe(initialCopy);
-		rerender({ onError: initialOnError });
-
-		// Re-render with a new clear value then swap back to simplify testing
-		rerender({ onError: initialOnError, clearErrorOnSuccess: false });
-		expect(result.current.copyToClipboard).toBe(initialCopy);
-		rerender({ onError: initialOnError, clearErrorOnSuccess: true });
-
 		// Trigger a failed clipboard interaction
 		setSimulateFailure(true);
 		await act(() => result.current.copyToClipboard("dummy-text-2"));

site/src/hooks/useClipboard.ts

@@ -1,10 +1,4 @@
-import {
-	useCallback,
-	useEffect,
-	useEffectEvent,
-	useRef,
-	useState,
-} from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import { toast } from "sonner";
 
 const CLIPBOARD_TIMEOUT_MS = 1_000;
@@ -44,55 +38,50 @@ export type UseClipboardResult = Readonly<{
 export const useClipboard = (
 	input: UseClipboardInput = {},
 ): UseClipboardResult => {
-	const {
-		onError = (msg: string) => toast.error(msg),
-		clearErrorOnSuccess = true,
-	} = input;
+	const { onError = toast.error, clearErrorOnSuccess = true } = input;
 
 	const [showCopiedSuccess, setShowCopiedSuccess] = useState(false);
 	const [error, setError] = useState<Error>();
 	const timeoutIdRef = useRef<number | undefined>(undefined);
 
 	useEffect(() => {
-		const clearTimeoutOnUnmount = () => {
-			window.clearTimeout(timeoutIdRef.current);
-		};
-		return clearTimeoutOnUnmount;
+		return () => window.clearTimeout(timeoutIdRef.current);
 	}, []);
 
-	const onErrorEvent = useEffectEvent(() => onError(COPY_FAILED_MESSAGE));
-	const handleSuccessfulCopy = useEffectEvent(() => {
-		setShowCopiedSuccess(true);
-		if (clearErrorOnSuccess) {
-			setError(undefined);
-		}
+	const copyToClipboard = useCallback(
+		async (textToCopy: string) => {
+			const markSuccess = () => {
+				setShowCopiedSuccess(true);
+				if (clearErrorOnSuccess) {
+					setError(undefined);
+				}
+				timeoutIdRef.current = window.setTimeout(() => {
+					setShowCopiedSuccess(false);
+				}, CLIPBOARD_TIMEOUT_MS);
+			};
 
-		timeoutIdRef.current = window.setTimeout(() => {
-			setShowCopiedSuccess(false);
-		}, CLIPBOARD_TIMEOUT_MS);
-	});
+			try {
+				await window.navigator.clipboard.writeText(textToCopy);
+				markSuccess();
+			} catch (err) {
+				const fallbackCopySuccessful = simulateClipboardWrite(textToCopy);
+				if (fallbackCopySuccessful) {
+					markSuccess();
+					return;
+				}
 
-	const copyToClipboard = useCallback(async (textToCopy: string) => {
-		try {
-			await window.navigator.clipboard.writeText(textToCopy);
-			handleSuccessfulCopy();
-		} catch (err) {
-			const fallbackCopySuccessful = simulateClipboardWrite(textToCopy);
-			if (fallbackCopySuccessful) {
-				handleSuccessfulCopy();
-				return;
+				const wrappedErr = new Error(COPY_FAILED_MESSAGE);
+				if (err instanceof Error) {
+					wrappedErr.stack = err.stack;
+				}
+
+				console.error(wrappedErr);
+				setError(wrappedErr);
+				onError(COPY_FAILED_MESSAGE);
 			}
-
-			const wrappedErr = new Error(COPY_FAILED_MESSAGE);
-			if (err instanceof Error) {
-				wrappedErr.stack = err.stack;
-			}
-
-			console.error(wrappedErr);
-			setError(wrappedErr);
-			onErrorEvent();
-		}
-	}, []);
+		},
+		[onError, clearErrorOnSuccess],
+	);
 
 	return { showCopiedSuccess, error, copyToClipboard };
 };

site/src/modules/notifications/NotificationsInbox/NotificationsInbox.tsx

@@ -1,4 +1,4 @@
-import { type FC, useEffect, useEffectEvent } from "react";
+import { type FC, useCallback, useEffect } from "react";
 import { useMutation, useQuery, useQueryClient } from "react-query";
 import { toast } from "sonner";
 import { watchInboxNotifications } from "#/api/api";
@@ -40,7 +40,7 @@ export const NotificationsInbox: FC<NotificationsInboxProps> = ({
 		queryFn: () => fetchNotifications(),
 	});
 
-	const updateNotificationsCache = useEffectEvent(
+	const updateNotificationsCache = useCallback(
 		async (
 			callback: (
 				res: ListInboxNotificationsResponse,
@@ -57,6 +57,7 @@ export const NotificationsInbox: FC<NotificationsInboxProps> = ({
 				},
 			);
 		},
+		[queryClient],
 	);
 
 	useEffect(() => {
@@ -85,7 +86,7 @@ export const NotificationsInbox: FC<NotificationsInboxProps> = ({
 		});
 
 		return () => socket.close();
-	}, []);
+	}, [updateNotificationsCache]);
 
 	const {
 		mutate: loadMoreNotifications,

site/src/pages/AgentsPage/components/AgentCreateForm.tsx

@@ -310,26 +310,24 @@ export const AgentCreateForm: FC<AgentCreateFormProps> = ({
 			? selectedWorkspaceId
 			: null;
 
-	const handleSend = useEffectEvent(
-		async (message: string, fileIDs?: string[]) => {
-			submitDraft();
-			await onCreateChat({
-				message,
-				fileIDs,
-				workspaceId: effectiveWorkspaceId ?? undefined,
-				model: selectedModel || undefined,
-				organizationId,
-				mcpServerIds:
-					effectiveMCPServerIds.length > 0
-						? [...effectiveMCPServerIds]
-						: undefined,
-				planMode: planModeEnabled ? "plan" : undefined,
-			}).catch((err) => {
-				resetDraft();
-				throw err;
-			});
-		},
-	);
+	const handleSend = async (message: string, fileIDs?: string[]) => {
+		submitDraft();
+		await onCreateChat({
+			message,
+			fileIDs,
+			workspaceId: effectiveWorkspaceId ?? undefined,
+			model: selectedModel || undefined,
+			organizationId,
+			mcpServerIds:
+				effectiveMCPServerIds.length > 0
+					? [...effectiveMCPServerIds]
+					: undefined,
+			planMode: planModeEnabled ? "plan" : undefined,
+		}).catch((err) => {
+			resetDraft();
+			throw err;
+		});
+	};
 
 	const {
 		attachments,

site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts

@@ -1,4 +1,10 @@
-import { useEffect, useEffectEvent, useRef, useState } from "react";
+import {
+	useCallback,
+	useEffect,
+	useEffectEvent,
+	useRef,
+	useState,
+} from "react";
 import { type InfiniteData, useQueryClient } from "react-query";
 import { watchChat } from "#/api/api";
 import { chatMessagesKey, updateInfiniteChatsCache } from "#/api/queries/chats";
@@ -131,7 +137,7 @@ export const useChatStore = (
 	// last REST fetch, and structural sharing can suppress the
 	// refetch-driven store update when no new durable messages
 	// have been committed to the DB yet.
-	const upsertCacheMessages = useEffectEvent(
+	const upsertCacheMessages = useCallback(
 		(messages: readonly TypesGen.ChatMessage[]) => {
 			if (!chatID || messages.length === 0) {
 				return;
@@ -172,6 +178,7 @@ export const useChatStore = (
 				};
 			});
 		},
+		[chatID, queryClient],
 	);
 
 	useEffect(() => {
@@ -626,7 +633,7 @@ export const useChatStore = (
 			}
 			activeChatIDRef.current = null;
 		};
-	}, [chatID, initialDataLoaded, queryClient, store]);
+	}, [chatID, initialDataLoaded, queryClient, store, upsertCacheMessages]);
 	return {
 		store,
 		clearStreamError: () => {

site/src/pages/AgentsPage/components/ChatScrollContainer.tsx

@@ -120,20 +120,20 @@ function useStickToBottom(): StickToBottomInstance {
 	});
 
 	// Sync helpers — keep mutable state and React state in lockstep.
-	const syncIsAtBottom = useEffectEvent((v: boolean) => {
+	const syncIsAtBottom = (v: boolean) => {
 		stateRef.current.internalIsAtBottom = v;
 		setIsAtBottom(v);
-	});
+	};
 
-	const syncEscapedFromLock = useEffectEvent((v: boolean) => {
+	const syncEscapedFromLock = (v: boolean) => {
 		stateRef.current.escapedFromLock = v;
-	});
+	};
 
 	// -----------------------------------------------------------------------
 	// scrollToBottom
 	// -----------------------------------------------------------------------
 
-	const scrollToBottom = useEffectEvent((behavior?: ScrollBehavior) => {
+	const scrollToBottom = (behavior?: ScrollBehavior) => {
 		const s = stateRef.current;
 		if (!s.scrollElement) return;
 
@@ -153,7 +153,7 @@ function useStickToBottom(): StickToBottomInstance {
 			// scroll (currentScrollTop > lastScrollTop), which
 			// correctly clears escapedFromLock.
 		}
-	});
+	};
 
 	const suppressNextResize = () => {
 		stateRef.current.suppressNextResize = true;

site/src/pages/AgentsPage/hooks/useFileAttachments.ts

@@ -190,12 +190,13 @@ export function useFileAttachments(
 		() => new Map<File, string>(),
 	);
 
-	// Revoke blob URLs on unmount to prevent memory leaks.
 	const revokePreviewUrls = useEffectEvent(() => {
 		for (const [, url] of previewUrls) {
 			if (url.startsWith("blob:")) URL.revokeObjectURL(url);
 		}
 	});
+
+	// Revoke blob URLs on unmount to prevent memory leaks.
 	useEffect(() => {
 		return () => revokePreviewUrls();
 	}, []);
@@ -345,7 +346,9 @@ export function useFileAttachments(
 	};
 
 	const resetAttachments = () => {
-		revokePreviewUrls();
+		for (const [, url] of previewUrls) {
+			if (url.startsWith("blob:")) URL.revokeObjectURL(url);
+		}
 		setPreviewUrls(new Map());
 		setTextContents(new Map());
 		setUploadStates(new Map());

site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx

@@ -99,19 +99,20 @@ const CreateWorkspacePage: FC = () => {
 
 	const autofillParameters = getAutofillParameters(searchParams);
 
-	const sendMessage = useEffectEvent(
-		(formValues: Record<string, string>, ownerId?: string) => {
-			const request: DynamicParametersRequest = {
-				id: wsResponseId.current + 1,
-				owner_id: ownerId ?? owner.id,
-				inputs: formValues,
-			};
-			if (ws.current && ws.current.readyState === WebSocket.OPEN) {
-				ws.current.send(JSON.stringify(request));
-				wsResponseId.current = wsResponseId.current + 1;
-			}
-		},
-	);
+	const sendMessage = (
+		formValues: Record<string, string>,
+		ownerId?: string,
+	) => {
+		const request: DynamicParametersRequest = {
+			id: wsResponseId.current + 1,
+			owner_id: ownerId ?? owner.id,
+			inputs: formValues,
+		};
+		if (ws.current && ws.current.readyState === WebSocket.OPEN) {
+			ws.current.send(JSON.stringify(request));
+			wsResponseId.current = wsResponseId.current + 1;
+		}
+	};
 
 	// On page load, sends all initial parameter values to the websocket
 	// (including defaults and autofilled from the url)

site/src/pages/TemplatePage/TemplateEmbedPage/TemplateEmbedPageExperimental.tsx

@@ -42,19 +42,20 @@ const TemplateEmbedPageExperimental: FC = () => {
 	const ws = useRef<WebSocket | null>(null);
 	const [wsError, setWsError] = useState<Error | null>(null);
 
-	const sendMessage = useEffectEvent(
-		(formValues: Record<string, string>, _ownerId?: string) => {
-			const request: DynamicParametersRequest = {
-				id: wsResponseId.current + 1,
-				owner_id: me.id,
-				inputs: formValues,
-			};
-			if (ws.current && ws.current.readyState === WebSocket.OPEN) {
-				ws.current.send(JSON.stringify(request));
-				wsResponseId.current = wsResponseId.current + 1;
-			}
-		},
-	);
+	const sendMessage = (
+		formValues: Record<string, string>,
+		_ownerId?: string,
+	) => {
+		const request: DynamicParametersRequest = {
+			id: wsResponseId.current + 1,
+			owner_id: me.id,
+			inputs: formValues,
+		};
+		if (ws.current && ws.current.readyState === WebSocket.OPEN) {
+			ws.current.send(JSON.stringify(request));
+			wsResponseId.current = wsResponseId.current + 1;
+		}
+	};
 
 	const onMessage = useEffectEvent((response: DynamicParametersResponse) => {
 		if (latestResponse && latestResponse?.id >= response.id) {

site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx

@@ -60,7 +60,7 @@ const WorkspaceParametersPageExperimental: FC = () => {
 			source: "active_build",
 		})) ?? [];
 
-	const sendMessage = useEffectEvent((formValues: Record<string, string>) => {
+	const sendMessage = (formValues: Record<string, string>) => {
 		const request: DynamicParametersRequest = {
 			id: wsResponseId.current + 1,
 			owner_id: workspace.owner_id,
@@ -70,7 +70,7 @@ const WorkspaceParametersPageExperimental: FC = () => {
 			ws.current.send(JSON.stringify(request));
 			wsResponseId.current = wsResponseId.current + 1;
 		}
-	});
+	};
 
 	// On page load, sends initial workspace build parameters to the websocket.
 	// This ensures the backend has the form's complete initial state,

site/src/pages/WorkspacesPage/WorkspacesPage.tsx

@@ -1,4 +1,11 @@
-import { type FC, useEffectEvent, useMemo, useState } from "react";
+import {
+	type FC,
+	useCallback,
+	useLayoutEffect,
+	useMemo,
+	useRef,
+	useState,
+} from "react";
 import { useQuery, useQueryClient } from "react-query";
 import { useSearchParams } from "react-router";
 import { toast } from "sonner";
@@ -30,11 +37,17 @@ function useSafeSearchParams() {
 	// Have to wrap setSearchParams because React Router doesn't guarantee
 	// a stable reference for setSearchParams across renders.
 	const [searchParams, setSearchParams] = useSearchParams();
-	const setSearchParamsEvent = useEffectEvent(setSearchParams);
-
-	// Need this to be a tuple type, but can't use "as const", because that would
-	// make the whole array readonly and cause type mismatches downstream
-	return [searchParams, setSearchParamsEvent] as ReturnType<
+	const setterRef = useRef(setSearchParams);
+	useLayoutEffect(() => {
+		setterRef.current = setSearchParams;
+	}, [setSearchParams]);
+	const stableSetSearchParams = useCallback(
+		(...args: Parameters<typeof setSearchParams>) => setterRef.current(...args),
+		[],
+	);
+	// Need this to return a tuple, not a plain array. Using "as const"
+	// would make it readonly and cause type mismatches at call sites.
+	return [searchParams, stableSetSearchParams] as ReturnType<
 		typeof useSearchParams
 	>;
 }