diff --git a/site/src/components/SearchField/SearchField.tsx b/site/src/components/SearchField/SearchField.tsx index 9477c547e4..56a3df85e8 100644 --- a/site/src/components/SearchField/SearchField.tsx +++ b/site/src/components/SearchField/SearchField.tsx @@ -1,5 +1,5 @@ import { SearchIcon, XIcon } from "lucide-react"; -import { type Ref, useLayoutEffect, useRef } from "react"; +import { type Ref, useEffectEvent, useLayoutEffect, useRef } from "react"; import { InputGroup, InputGroupAddon, @@ -11,7 +11,6 @@ import { TooltipContent, TooltipTrigger, } from "#/components/Tooltip/Tooltip"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; export type SearchFieldProps = { value: string; @@ -45,7 +44,7 @@ export const SearchField: React.FC = ({ }); useLayoutEffect(() => { focusOnMount(); - }, [focusOnMount]); + }, []); const handleClear = () => { if (onClear) { diff --git a/site/src/hooks/hookPolyfills.test.ts b/site/src/hooks/hookPolyfills.test.ts deleted file mode 100644 index 23342bb59c..0000000000 --- a/site/src/hooks/hookPolyfills.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { renderHook } from "@testing-library/react"; -import { useEffectEvent } from "./hookPolyfills"; - -describe(useEffectEvent.name, () => { - function renderEffectEvent( - callbackArg: (...args: TArgs) => TReturn, - ) { - type Callback = typeof callbackArg; - type Props = Readonly<{ callback: Callback }>; - - return renderHook( - ({ callback }) => useEffectEvent(callback), - { initialProps: { callback: callbackArg } }, - ); - } - - it("Should maintain a stable reference across all renders", () => { - const callback = vi.fn(); - const { result, rerender } = renderEffectEvent(callback); - - const firstResult = result.current; - for (let i = 0; i < 5; i++) { - rerender({ callback }); - } - - expect(result.current).toBe(firstResult); - expect.hasAssertions(); - }); - - it("Should always call the most recent callback passed in", () => { - const mockCallback1 = vi.fn(); - const mockCallback2 = vi.fn(); - - const { result, rerender } = renderEffectEvent(mockCallback1); - rerender({ callback: mockCallback2 }); - - result.current(); - expect(mockCallback1).not.toBeCalled(); - expect(mockCallback2).toBeCalledTimes(1); - }); -}); diff --git a/site/src/hooks/hookPolyfills.ts b/site/src/hooks/hookPolyfills.ts deleted file mode 100644 index 171ff2c053..0000000000 --- a/site/src/hooks/hookPolyfills.ts +++ /dev/null @@ -1,49 +0,0 @@ -/** - * @file For defining DIY versions of official React hooks that have not been - * released yet. - * - * These hooks should be deleted as soon as the official versions are available. - * They do not have the same ESLinter exceptions baked in that the official - * hooks do, especially for dependency arrays. - */ -import { useCallback, useLayoutEffect, useRef } from "react"; -/** - * A DIY version of useEffectEvent. - * - * Works like useCallback, except that it doesn't take a dependency array, and - * always returns out the same function on every single render. The returned-out - * function is always able to "see" the most up-to-date version of the callback - * passed in (including its closure values). - * - * This is not a 1:1 replacement for useCallback. 99% of the time, - * useEffectEvent should be called in the same component/custom hook where you - * have a useEffect call. A useEffectEvent function probably shouldn't be a - * prop, unless you're trying to wrangle a weird library. - * - * Example uses of useEffectEvent: - * 1. Stabilizing a function that you don't have direct control over (because it - * comes from a library) without violating useEffect dependency arrays - * 2. Moving the burden of memoization from the parent to the custom hook (e.g., - * making it so that you don't need your components to always use useCallback - * just to get things wired up properly. Similar example: the queryFn - * property on React Query's useQuery) - * - * @see {@link https://react.dev/reference/react/experimental_useEffectEvent} - */ -export function useEffectEvent( - callback: (...args: TArgs) => TReturn, -) { - const callbackRef = useRef(callback); - - // useLayoutEffect should be overkill here 99% of the time, but if this were - // defined as a regular effect, useEffectEvent would not be able to work with - // any layout effects at all; the callback sync here would fire *after* the - // layout effect that needs the useEffectEvent function - useLayoutEffect(() => { - callbackRef.current = callback; - }, [callback]); - - return useCallback((...args: TArgs): TReturn => { - return callbackRef.current(...args); - }, []); -} diff --git a/site/src/hooks/useClickable.ts b/site/src/hooks/useClickable.ts index 6f5e0afff7..26e7d62ff8 100644 --- a/site/src/hooks/useClickable.ts +++ b/site/src/hooks/useClickable.ts @@ -2,9 +2,9 @@ import { type KeyboardEventHandler, type MouseEventHandler, type RefObject, + useEffectEvent, useRef, } from "react"; -import { useEffectEvent } from "./hookPolyfills"; // Literally any object (ideally an HTMLElement) that has a .click method type ClickableElement = { @@ -44,11 +44,11 @@ export const useClickable = < role?: TRole, ): UseClickableResult => { const ref = useRef(null); - const stableOnClick = useEffectEvent(onClick); + const onClickEvent = useEffectEvent(onClick); return { ref, - onClick: stableOnClick, + onClick: onClickEvent, tabIndex: 0, role: (role ?? "button") as TRole, diff --git a/site/src/hooks/useClipboard.ts b/site/src/hooks/useClipboard.ts index fd80d65c9c..a6d7928523 100644 --- a/site/src/hooks/useClipboard.ts +++ b/site/src/hooks/useClipboard.ts @@ -1,6 +1,11 @@ -import { useCallback, useEffect, useRef, useState } from "react"; +import { + useCallback, + useEffect, + useEffectEvent, + useRef, + useState, +} from "react"; import { toast } from "sonner"; -import { useEffectEvent } from "./hookPolyfills"; const CLIPBOARD_TIMEOUT_MS = 1_000; export const COPY_FAILED_MESSAGE = "Failed to copy text to clipboard"; @@ -55,7 +60,7 @@ export const useClipboard = ( return clearTimeoutOnUnmount; }, []); - const stableOnError = useEffectEvent(() => onError(COPY_FAILED_MESSAGE)); + const onErrorEvent = useEffectEvent(() => onError(COPY_FAILED_MESSAGE)); const handleSuccessfulCopy = useEffectEvent(() => { setShowCopiedSuccess(true); if (clearErrorOnSuccess) { @@ -67,30 +72,27 @@ export const useClipboard = ( }, CLIPBOARD_TIMEOUT_MS); }); - const copyToClipboard = useCallback( - async (textToCopy: string) => { - try { - await window.navigator.clipboard.writeText(textToCopy); + const copyToClipboard = useCallback(async (textToCopy: string) => { + try { + await window.navigator.clipboard.writeText(textToCopy); + handleSuccessfulCopy(); + } catch (err) { + const fallbackCopySuccessful = simulateClipboardWrite(textToCopy); + if (fallbackCopySuccessful) { 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); - stableOnError(); + return; } - }, - [stableOnError, handleSuccessfulCopy], - ); + + const wrappedErr = new Error(COPY_FAILED_MESSAGE); + if (err instanceof Error) { + wrappedErr.stack = err.stack; + } + + console.error(wrappedErr); + setError(wrappedErr); + onErrorEvent(); + } + }, []); return { showCopiedSuccess, error, copyToClipboard }; }; diff --git a/site/src/hooks/usePaginatedQuery.ts b/site/src/hooks/usePaginatedQuery.ts index 1ad03272a7..7e60ca52d8 100644 --- a/site/src/hooks/usePaginatedQuery.ts +++ b/site/src/hooks/usePaginatedQuery.ts @@ -1,5 +1,5 @@ import clamp from "lodash/clamp"; -import { useEffect } from "react"; +import { useEffect, useEffectEvent } from "react"; import { keepPreviousData, type QueryFunctionContext, @@ -10,7 +10,6 @@ import { useQueryClient, } from "react-query"; import { type SetURLSearchParams, useSearchParams } from "react-router"; -import { useEffectEvent } from "./hookPolyfills"; const DEFAULT_RECORDS_PER_PAGE = 25; @@ -201,13 +200,13 @@ export function usePaginatedQuery< if (hasNextPage) { void prefetchPage(currentPage + 1); } - }, [prefetchPage, currentPage, hasNextPage]); + }, [currentPage, hasNextPage]); useEffect(() => { if (hasPreviousPage) { void prefetchPage(currentPage - 1); } - }, [prefetchPage, currentPage, hasPreviousPage]); + }, [currentPage, hasPreviousPage]); // Mainly here to catch user if they navigate to a page directly via URL; // totalPages parameterized to insulate function from fetch status changes @@ -259,7 +258,7 @@ export function usePaginatedQuery< ) { void updatePageIfInvalid(totalPages); } - }, [updatePageIfInvalid, query.isFetching, totalPages, currentPage]); + }, [query.isFetching, totalPages, currentPage]); const onPageChange = (newPage: number) => { // Page 1 is the only page that can be safely navigated to without knowing diff --git a/site/src/hooks/useTime.ts b/site/src/hooks/useTime.ts index 7f03c05225..3c431af3d4 100644 --- a/site/src/hooks/useTime.ts +++ b/site/src/hooks/useTime.ts @@ -1,5 +1,4 @@ -import { useEffect, useState } from "react"; -import { useEffectEvent } from "./hookPolyfills"; +import { useEffect, useEffectEvent, useState } from "react"; interface UseTimeOptions { /** @@ -37,7 +36,7 @@ export function useTime(func: () => T, options: UseTimeOptions = {}): T { return () => { clearInterval(handle); }; - }, [thunk, disabled, interval]); + }, [disabled, interval]); return computedValue; } diff --git a/site/src/modules/navigation.ts b/site/src/modules/navigation.ts index e3067681b9..487c0e755d 100644 --- a/site/src/modules/navigation.ts +++ b/site/src/modules/navigation.ts @@ -2,7 +2,7 @@ * @fileoverview TODO: centralize navigation code here! URL constants, URL formatting, all of it */ -import { useEffectEvent } from "#/hooks/hookPolyfills"; +import { useCallback } from "react"; import type { DashboardValue } from "./dashboard/DashboardProvider"; import { useDashboard } from "./dashboard/useDashboard"; @@ -10,9 +10,10 @@ type LinkThunk = (state: DashboardValue) => string; export function useLinks() { const dashboard = useDashboard(); - // Needs to be safe to call `get` from inside of a `useEffect` without causing - // excess triggers from adding it as a dependency. - const get = useEffectEvent((thunk: LinkThunk): string => thunk(dashboard)); + const get = useCallback( + (thunk: LinkThunk): string => thunk(dashboard), + [dashboard], + ); return get; } diff --git a/site/src/modules/notifications/NotificationsInbox/NotificationsInbox.tsx b/site/src/modules/notifications/NotificationsInbox/NotificationsInbox.tsx index a22d7bcb3a..952748a09f 100644 --- a/site/src/modules/notifications/NotificationsInbox/NotificationsInbox.tsx +++ b/site/src/modules/notifications/NotificationsInbox/NotificationsInbox.tsx @@ -1,4 +1,4 @@ -import { type FC, useEffect } from "react"; +import { type FC, useEffect, useEffectEvent } from "react"; import { useMutation, useQuery, useQueryClient } from "react-query"; import { toast } from "sonner"; import { watchInboxNotifications } from "#/api/api"; @@ -7,7 +7,6 @@ import type { ListInboxNotificationsResponse, UpdateInboxNotificationReadStatusResponse, } from "#/api/typesGenerated"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { InboxPopover } from "./InboxPopover"; const NOTIFICATIONS_QUERY_KEY = ["notifications"]; @@ -86,7 +85,7 @@ export const NotificationsInbox: FC = ({ }); return () => socket.close(); - }, [updateNotificationsCache]); + }, []); const { mutate: loadMoreNotifications, diff --git a/site/src/modules/resources/useAgentContainers.ts b/site/src/modules/resources/useAgentContainers.ts index 12a86bb153..97cb079e41 100644 --- a/site/src/modules/resources/useAgentContainers.ts +++ b/site/src/modules/resources/useAgentContainers.ts @@ -1,4 +1,4 @@ -import { useEffect } from "react"; +import { useEffect, useEffectEvent } from "react"; import { useQuery, useQueryClient } from "react-query"; import { toast } from "sonner"; import { watchAgentContainers } from "#/api/api"; @@ -11,7 +11,6 @@ import type { WorkspaceAgentDevcontainer, WorkspaceAgentListContainersResponse, } from "#/api/typesGenerated"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; export function useAgentContainers( agent: WorkspaceAgent, @@ -59,13 +58,7 @@ export function useAgentContainers( }); return () => socket.close(); - }, [ - agent.id, - agent.status, - queryIsLoading, - queryError, - updateDevcontainersCache, - ]); + }, [agent.id, agent.status, queryIsLoading, queryError]); return devcontainers; } diff --git a/site/src/modules/templates/useWatchVersionLogs.ts b/site/src/modules/templates/useWatchVersionLogs.ts index 69fbc2373c..bdfa71f78a 100644 --- a/site/src/modules/templates/useWatchVersionLogs.ts +++ b/site/src/modules/templates/useWatchVersionLogs.ts @@ -1,7 +1,6 @@ -import { useEffect, useState } from "react"; +import { useEffect, useEffectEvent, useState } from "react"; import { watchBuildLogsByTemplateVersionId } from "#/api/api"; import type { ProvisionerJobLog, TemplateVersion } from "#/api/typesGenerated"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; export const useWatchVersionLogs = ( templateVersion: TemplateVersion | undefined, options?: { onDone: () => Promise }, @@ -14,7 +13,7 @@ export const useWatchVersionLogs = ( setLogs([]); } - const stableOnDone = useEffectEvent(() => options?.onDone()); + const onDoneEvent = useEffectEvent(() => options?.onDone()); const status = templateVersion?.job.status; const canWatch = status === "running" || status === "pending"; useEffect(() => { @@ -24,14 +23,14 @@ export const useWatchVersionLogs = ( const socket = watchBuildLogsByTemplateVersionId(templateVersionId, { onError: (error) => console.error(error), - onDone: stableOnDone, + onDone: onDoneEvent, onMessage: (newLog) => { setLogs((current) => [...(current ?? []), newLog]); }, }); return () => socket.close(); - }, [stableOnDone, canWatch, templateVersionId]); + }, [canWatch, templateVersionId]); return logs; }; diff --git a/site/src/modules/terminal/WorkspaceTerminal.tsx b/site/src/modules/terminal/WorkspaceTerminal.tsx index 353b7259fd..b62682b79b 100644 --- a/site/src/modules/terminal/WorkspaceTerminal.tsx +++ b/site/src/modules/terminal/WorkspaceTerminal.tsx @@ -9,6 +9,7 @@ import { type Ref, useCallback, useEffect, + useEffectEvent, useId, useImperativeHandle, useRef, @@ -20,7 +21,6 @@ import { WebsocketBuilder, WebsocketEvent, } from "websocket-ts"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { useClipboard } from "#/hooks/useClipboard"; import { cn } from "#/utils/cn"; import { terminalWebsocketUrl } from "#/utils/terminal"; @@ -121,7 +121,7 @@ export const WorkspaceTerminal = ({ width: terminal.cols, }; }, - [reportTerminalError], + [], ); const refit = useCallback(() => { @@ -261,10 +261,8 @@ export const WorkspaceTerminal = ({ }, [ hasBeenVisible, copyToClipboard, - handleOpenLink, refit, renderer, - reportTerminalError, terminalFontFamily, backgroundColor, ]); @@ -462,13 +460,11 @@ export const WorkspaceTerminal = ({ containerUser, errorMessage, getTerminalDimensions, - handleStatusChange, initialCommand, loading, operatingSystem, reconnectionToken, refit, - reportTerminalError, terminal, ]); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts index a289fc9650..ab660ec4b7 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts @@ -1,9 +1,8 @@ -import { useEffect, useRef, useState } from "react"; +import { 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"; import type * as TypesGen from "#/api/typesGenerated"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import type { OneWayMessageEvent } from "#/utils/OneWayWebSocket"; import { createReconnectingWebSocket } from "#/utils/reconnectingWebSocket"; import type { ChatDetailError } from "../../utils/usageLimitMessage"; @@ -140,12 +139,10 @@ export const useChatStore = ( : undefined; }); - // Keep error-reason callbacks in refs so the WebSocket effect - // can call them without including them in its dependency array. - // This prevents the socket from tearing down when the parent - // re-renders with new callback identities. - const setChatErrorReasonStable = useEffectEvent(setChatErrorReason); - const clearChatErrorReasonStable = useEffectEvent(clearChatErrorReason); + // Wrap error-reason callbacks so the WebSocket effect can call + // them without including them in its dependency array. + const setChatErrorReasonEvent = useEffectEvent(setChatErrorReason); + const clearChatErrorReasonEvent = useEffectEvent(clearChatErrorReason); // True once the initial REST page has resolved for the current // chat. The WebSocket effect gates on this so that @@ -538,7 +535,7 @@ export const useChatStore = ( store.clearRetryState(); } if (nextStatus !== "error") { - clearChatErrorReasonStable(chatID); + clearChatErrorReasonEvent(chatID); } updateSidebarChat((chat) => chat.status === nextStatus @@ -555,7 +552,7 @@ export const useChatStore = ( store.setChatStatus("error"); store.setStreamError(reason); store.clearRetryState(); - setChatErrorReasonStable(chatID, reason); + setChatErrorReasonEvent(chatID, reason); updateSidebarChat((chat) => chat.status === "error" ? chat : { ...chat, status: "error" }, ); @@ -660,15 +657,7 @@ export const useChatStore = ( } activeChatIDRef.current = null; }; - }, [ - chatID, - initialDataLoaded, - queryClient, - store, - setChatErrorReasonStable, - clearChatErrorReasonStable, - upsertCacheMessages, - ]); + }, [chatID, initialDataLoaded, queryClient, store]); return { store, clearStreamError: () => { diff --git a/site/src/pages/AgentsPage/components/ChatScrollContainer.tsx b/site/src/pages/AgentsPage/components/ChatScrollContainer.tsx index 90a81a09f5..4a130b7acb 100644 --- a/site/src/pages/AgentsPage/components/ChatScrollContainer.tsx +++ b/site/src/pages/AgentsPage/components/ChatScrollContainer.tsx @@ -4,12 +4,12 @@ import { type RefCallback, type RefObject, useEffect, + useEffectEvent, useLayoutEffect, useRef, useState, } from "react"; import { Button } from "#/components/Button/Button"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { cn } from "#/utils/cn"; // =========================================================================== @@ -155,18 +155,18 @@ function useStickToBottom(): StickToBottomInstance { } }); - const suppressNextResize = useEffectEvent(() => { + const suppressNextResize = () => { stateRef.current.suppressNextResize = true; - }); + }; - const capturePrependSnapshot = useEffectEvent(() => { + const capturePrependSnapshot = () => { const s = stateRef.current; if (s.scrollElement) { s.pendingPrepend = { scrollHeight: s.scrollElement.scrollHeight, }; } - }); + }; // ----------------------------------------------------------------------- // Event handlers @@ -430,63 +430,68 @@ function useStickToBottom(): StickToBottomInstance { } }); - const scrollRef = useEffectEvent((el: HTMLDivElement | null) => { + // Ref callbacks must have stable identity — React cycles them + // on identity change, which leaks event listeners. Store the + // element in state and let a useEffect manage listeners. + const [scrollElement, setScrollElement] = useState( + null, + ); + + useEffect(() => { const s = stateRef.current; - const prev = s.scrollElement; + s.scrollElement = scrollElement; + if (!scrollElement) return; - if (prev) { - prev.removeEventListener("touchstart", handleTouchStart); - prev.removeEventListener("touchend", handleTouchEnd); - prev.removeEventListener("touchcancel", handleTouchEnd); - prev.removeEventListener("scroll", handleScroll); - prev.removeEventListener("wheel", handleWheel); - prev.removeEventListener("pointerdown", handlePointerDown); - } + s.lastClientHeight = scrollElement.clientHeight; + scrollElement.addEventListener("scroll", handleScroll, { passive: true }); + scrollElement.addEventListener("wheel", handleWheel, { passive: true }); + scrollElement.addEventListener("touchstart", handleTouchStart, { + passive: true, + }); + scrollElement.addEventListener("touchend", handleTouchEnd, { + passive: true, + }); + scrollElement.addEventListener("touchcancel", handleTouchEnd, { + passive: true, + }); + scrollElement.addEventListener("pointerdown", handlePointerDown); - if (s.viewportObserver) { - s.viewportObserver.disconnect(); - s.viewportObserver = null; - } + const vo = new ResizeObserver(handleViewportResize); + vo.observe(scrollElement); + s.viewportObserver = vo; - s.scrollElement = el; + return () => { + scrollElement.removeEventListener("touchstart", handleTouchStart); + scrollElement.removeEventListener("touchend", handleTouchEnd); + scrollElement.removeEventListener("touchcancel", handleTouchEnd); + scrollElement.removeEventListener("scroll", handleScroll); + scrollElement.removeEventListener("wheel", handleWheel); + scrollElement.removeEventListener("pointerdown", handlePointerDown); + if (s.viewportObserver) { + s.viewportObserver.disconnect(); + s.viewportObserver = null; + } + }; + }, [scrollElement]); - if (el) { - s.lastClientHeight = el.clientHeight; - el.addEventListener("scroll", handleScroll, { passive: true }); - el.addEventListener("wheel", handleWheel, { passive: true }); - el.addEventListener("touchstart", handleTouchStart, { - passive: true, - }); - el.addEventListener("touchend", handleTouchEnd, { - passive: true, - }); - el.addEventListener("touchcancel", handleTouchEnd, { - passive: true, - }); - el.addEventListener("pointerdown", handlePointerDown); + const [contentElement, setContentElement] = useState( + null, + ); - const vo = new ResizeObserver(handleViewportResize); - vo.observe(el); - s.viewportObserver = vo; - } - }); - - const contentRef = useEffectEvent((el: HTMLDivElement | null) => { + useEffect(() => { const s = stateRef.current; + s.contentElement = contentElement; + if (!contentElement) return; - if (s.resizeObserver) { - s.resizeObserver.disconnect(); + const ro = new ResizeObserver(handleContentResize); + ro.observe(contentElement); + s.resizeObserver = ro; + + return () => { + ro.disconnect(); s.resizeObserver = null; - } - - s.contentElement = el; - - if (el) { - const ro = new ResizeObserver(handleContentResize); - ro.observe(el); - s.resizeObserver = ro; - } - }); + }; + }, [contentElement]); // ----------------------------------------------------------------------- // Mouse tracking (instance-scoped) @@ -527,33 +532,6 @@ function useStickToBottom(): StickToBottomInstance { }; }, []); - // Cleanup on unmount. - useEffect(() => { - return () => { - const s = stateRef.current; - if (s.scrollElement) { - s.scrollElement.removeEventListener("scroll", handleScroll); - s.scrollElement.removeEventListener("wheel", handleWheel); - s.scrollElement.removeEventListener("touchstart", handleTouchStart); - s.scrollElement.removeEventListener("touchend", handleTouchEnd); - s.scrollElement.removeEventListener("touchcancel", handleTouchEnd); - s.scrollElement.removeEventListener("pointerdown", handlePointerDown); - } - if (s.resizeObserver) { - s.resizeObserver.disconnect(); - } - if (s.viewportObserver) { - s.viewportObserver.disconnect(); - } - }; - }, [ - handleScroll, - handleWheel, - handleTouchStart, - handleTouchEnd, - handlePointerDown, - ]); - // Post-render consistency check. If we believe we're pinned // to the bottom but the physical scroll position disagrees, // correct it before the browser paints. This catches any @@ -572,8 +550,8 @@ function useStickToBottom(): StickToBottomInstance { }); return { - scrollRef, - contentRef, + scrollRef: setScrollElement, + contentRef: setContentElement, scrollToBottom, isAtBottom: isAtBottom || nearBottom, suppressNextResize, @@ -619,11 +597,11 @@ const ChatScrollContainer: FC<{ // Merge our callback ref with the external RefObject so both // point at the same DOM node, and expose scrollToBottom to the // parent via its imperative ref. - const mergedScrollRef = useEffectEvent((el: HTMLDivElement | null) => { + const mergedScrollRef = (el: HTMLDivElement | null) => { scrollRef(el); scrollContainerRef.current = el; scrollToBottomRef.current = el ? () => scrollToBottom("instant") : null; - }); + }; // ------------------------------------------------------------------- // Pagination sentinel (IntersectionObserver) diff --git a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx index 5d0422c757..3377f8e478 100644 --- a/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx +++ b/site/src/pages/AgentsPage/components/Sidebar/AgentsSidebar.tsx @@ -52,6 +52,7 @@ import { type FC, useContext, useEffect, + useEffectEvent, useRef, useState, } from "react"; @@ -84,7 +85,6 @@ import { TooltipContent, TooltipTrigger, } from "#/components/Tooltip/Tooltip"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { useAuthenticated } from "#/hooks/useAuthenticated"; import { UserDropdownContent } from "#/modules/dashboard/Navbar/UserDropdown/UserDropdownContent"; import { useDashboard } from "#/modules/dashboard/useDashboard"; @@ -1419,7 +1419,7 @@ const LoadMoreSentinel: FC<{ isFetchingNextPage?: boolean; }> = ({ onLoadMore, isFetchingNextPage }) => { const sentinelRef = useRef(null); - const onLoadMoreStable = useEffectEvent(() => { + const onLoadMoreEvent = useEffectEvent(() => { onLoadMore?.(); }); @@ -1438,14 +1438,14 @@ const LoadMoreSentinel: FC<{ const observer = new IntersectionObserver( (entries) => { if (entries[0]?.isIntersecting) { - onLoadMoreStable(); + onLoadMoreEvent(); } }, { threshold: 0 }, ); observer.observe(el); return () => observer.disconnect(); - }, [isFetchingNextPage, onLoadMoreStable]); + }, [isFetchingNextPage]); return (
diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 45a60bdd48..55ec46a58a 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -2,6 +2,7 @@ import { type FC, useCallback, useEffect, + useEffectEvent, useMemo, useRef, useState, @@ -26,7 +27,6 @@ import type { Workspace, } from "#/api/typesGenerated"; import { Loader } from "#/components/Loader/Loader"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { useAuthenticated } from "#/hooks/useAuthenticated"; import { getInitialParameterValues } from "#/modules/workspaces/DynamicParameter/DynamicParameter"; import { generateWorkspaceName } from "#/modules/workspaces/generateWorkspaceName"; @@ -187,7 +187,7 @@ const CreateWorkspacePage: FC = () => { return () => { socket.close(); }; - }, [realizedVersionId, onMessage, defaultOwner.id]); + }, [realizedVersionId, defaultOwner.id]); const organizationId = templateQuery.data?.organization_id; @@ -278,7 +278,7 @@ const CreateWorkspacePage: FC = () => { if (autoCreateReady) { void automateWorkspaceCreation(); } - }, [automateWorkspaceCreation, autoCreateReady]); + }, [autoCreateReady]); const sortedParams = useMemo(() => { if (!latestResponse?.parameters) { diff --git a/site/src/pages/TemplatePage/TemplateEmbedPage/TemplateEmbedPageExperimental.tsx b/site/src/pages/TemplatePage/TemplateEmbedPage/TemplateEmbedPageExperimental.tsx index c804611bfc..96828a985d 100644 --- a/site/src/pages/TemplatePage/TemplateEmbedPage/TemplateEmbedPageExperimental.tsx +++ b/site/src/pages/TemplatePage/TemplateEmbedPage/TemplateEmbedPageExperimental.tsx @@ -1,5 +1,12 @@ import { CheckIcon, CopyIcon } from "lucide-react"; -import { type FC, useEffect, useMemo, useRef, useState } from "react"; +import { + type FC, + useEffect, + useEffectEvent, + useMemo, + useRef, + useState, +} from "react"; import { API } from "#/api/api"; import { DetailedError } from "#/api/errors"; import type { @@ -15,7 +22,6 @@ import { Label } from "#/components/Label/Label"; import { RadioGroup, RadioGroupItem } from "#/components/RadioGroup/RadioGroup"; import { Separator } from "#/components/Separator/Separator"; import { Skeleton } from "#/components/Skeleton/Skeleton"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { useAuthenticated } from "#/hooks/useAuthenticated"; import { useClipboard } from "#/hooks/useClipboard"; import { @@ -91,7 +97,7 @@ const TemplateEmbedPageExperimental: FC = () => { return () => { socket.close(); }; - }, [template.active_version_id, onMessage, me]); + }, [template.active_version_id, me]); const sortedParams = useMemo(() => { if (!latestResponse?.parameters) { diff --git a/site/src/pages/WorkspacePage/WorkspacePage.tsx b/site/src/pages/WorkspacePage/WorkspacePage.tsx index dcf8a363bd..91568ba58f 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.tsx @@ -1,4 +1,4 @@ -import { type FC, useEffect } from "react"; +import { type FC, useEffect, useEffectEvent } from "react"; import { useQuery, useQueryClient } from "react-query"; import { useParams } from "react-router"; import { toast } from "sonner"; @@ -13,7 +13,6 @@ import type { Workspace } from "#/api/typesGenerated"; import { ErrorAlert } from "#/components/Alert/ErrorAlert"; import { Loader } from "#/components/Loader/Loader"; import { Margins } from "#/components/Margins/Margins"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { WorkspaceReadyPage } from "./WorkspaceReadyPage"; const WorkspacePage: FC = () => { @@ -99,7 +98,7 @@ const WorkspacePage: FC = () => { }); return () => socket.close(); - }, [updateWorkspaceData, workspaceId, workspaceName]); + }, [workspaceId, workspaceName]); // Page statuses const pageError = diff --git a/site/src/pages/WorkspacePage/useResourcesNav.ts b/site/src/pages/WorkspacePage/useResourcesNav.ts index 7f64cba141..aaf41dc7c4 100644 --- a/site/src/pages/WorkspacePage/useResourcesNav.ts +++ b/site/src/pages/WorkspacePage/useResourcesNav.ts @@ -1,6 +1,5 @@ -import { useCallback, useEffect } from "react"; +import { useCallback, useEffect, useEffectEvent } from "react"; import type { WorkspaceResource } from "#/api/typesGenerated"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { useSearchParamsKey } from "#/hooks/useSearchParamsKey"; export const resourceOptionValue = (resource: WorkspaceResource) => { return `${resource.type}_${resource.name}`; @@ -35,7 +34,7 @@ export const useResourcesNav = (resources: WorkspaceResource[]) => { ); useEffect(() => { onResourceChanges(resources); - }, [onResourceChanges, resources]); + }, [resources]); const select = useCallback( (resource: WorkspaceResource) => { diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx index db765b9221..f5aa89012f 100644 --- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx +++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPageExperimental.tsx @@ -1,6 +1,6 @@ import { CircleHelp } from "lucide-react"; import type { FC } from "react"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useEffectEvent, useMemo, useRef, useState } from "react"; import { useMutation, useQuery } from "react-query"; import { useNavigate, useSearchParams } from "react-router"; import { API } from "#/api/api"; @@ -21,7 +21,6 @@ import { TooltipProvider, TooltipTrigger, } from "#/components/Tooltip/Tooltip"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { docs } from "#/utils/docs"; import { pageTitle } from "#/utils/page"; import type { AutofillBuildParameter } from "#/utils/richParameters"; @@ -146,7 +145,6 @@ const WorkspaceParametersPageExperimental: FC = () => { }, [ templateVersionId, workspace.latest_build.template_version_id, - onMessage, workspace.owner_id, ]); diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 25183288a5..a4bf604154 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -1,4 +1,4 @@ -import { type FC, useMemo, useState } from "react"; +import { type FC, useEffectEvent, useMemo, useState } from "react"; import { useQuery, useQueryClient } from "react-query"; import { useSearchParams } from "react-router"; import { toast } from "sonner"; @@ -9,7 +9,6 @@ import { templates, templateVersionRoot } from "#/api/queries/templates"; import { workspaces } from "#/api/queries/workspaces"; import { useFilter } from "#/components/Filter/Filter"; import { useUserFilterMenu } from "#/components/Filter/UserFilter"; -import { useEffectEvent } from "#/hooks/hookPolyfills"; import { useAuthenticated } from "#/hooks/useAuthenticated"; import { usePagination } from "#/hooks/usePagination"; import { useDashboard } from "#/modules/dashboard/useDashboard"; @@ -28,15 +27,14 @@ const ACTIVE_BUILDS_REFRESH_INTERVAL = 5_000; const NO_ACTIVE_BUILDS_REFRESH_INTERVAL = 30_000; function useSafeSearchParams() { - // Have to wrap setSearchParams because React Router doesn't make sure that - // the function's memory reference stays stable on each render, even though - // its logic never changes, and even though it has function update support + // Have to wrap setSearchParams because React Router doesn't guarantee + // a stable reference for setSearchParams across renders. const [searchParams, setSearchParams] = useSearchParams(); - const stableSetSearchParams = useEffectEvent(setSearchParams); + 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, stableSetSearchParams] as ReturnType< + return [searchParams, setSearchParamsEvent] as ReturnType< typeof useSearchParams >; }