From ca2fd288ab345f2f479f3af3d1913a556f4240f8 Mon Sep 17 00:00:00 2001 From: Chris Parsons <438676+chrisparsons83@users.noreply.github.com> Date: Mon, 2 Mar 2026 13:18:47 -0800 Subject: [PATCH] perf: fix draft room lag from excessive re-renders and listener leaks (#54) Primary fix: setTeamTimers now bails out with `return prev` when the value hasn't changed, preventing a full DraftRoom re-render on every 1-second timer tick (was 33% of profiler samples). Memoization: wrap AvailableParticipantsSection, TeamsDraftedGrid, QueueSection, SidebarRecentPicks, and DraftGridSection in React.memo so timer ticks don't cascade into heavy components that don't use timer state. Stable refs: wrap nine action handlers in useCallback and extract two inline arrow functions from props objects so memo() comparisons actually bail out. Memoize the { numFlexPicks } object passed to TeamsDraftedGrid. socketVersion: expose an incrementing counter from useDraftSocket so the socket handler effect re-registers on socket recreation. Async cleanup: add abort flag + in-flight guard to the visibilitychange JWT refresh handler to prevent concurrent executions and stale state updates after unmount. Add abort flag to useDraftNotifications permissions.query() to prevent dangling onchange if unmounted mid-promise. Co-authored-by: Claude Sonnet 4.6 --- .../draft/AvailableParticipantsSection.tsx | 6 +- app/components/draft/DraftGridSection.tsx | 6 +- app/components/draft/QueueSection.tsx | 5 +- app/components/draft/SidebarRecentPicks.tsx | 5 +- app/components/draft/TeamsDraftedGrid.tsx | 6 +- app/hooks/useDraftNotifications.ts | 8 +- app/hooks/useDraftSocket.ts | 6 + .../leagues/$leagueId.draft.$seasonId.tsx | 120 ++++++++++++------ 8 files changed, 107 insertions(+), 55 deletions(-) diff --git a/app/components/draft/AvailableParticipantsSection.tsx b/app/components/draft/AvailableParticipantsSection.tsx index 81061c0..4733bce 100644 --- a/app/components/draft/AvailableParticipantsSection.tsx +++ b/app/components/draft/AvailableParticipantsSection.tsx @@ -1,4 +1,4 @@ -import { useCallback, useMemo } from "react"; +import { memo, useCallback, useMemo } from "react"; import { Button } from "~/components/ui/button"; import { Badge } from "~/components/ui/badge"; import { Checkbox } from "~/components/ui/checkbox"; @@ -155,7 +155,7 @@ interface AvailableParticipantsSectionProps { onRemoveFromQueue: (queueId: string) => void; } -export function AvailableParticipantsSection({ +export const AvailableParticipantsSection = memo(function AvailableParticipantsSection({ participants, searchQuery, sportFilters, @@ -604,4 +604,4 @@ export function AvailableParticipantsSection({ ); -} +}); diff --git a/app/components/draft/DraftGridSection.tsx b/app/components/draft/DraftGridSection.tsx index 321ee54..f87ac53 100644 --- a/app/components/draft/DraftGridSection.tsx +++ b/app/components/draft/DraftGridSection.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from "react"; +import { memo, useMemo, useState } from "react"; import { ContextMenu, ContextMenuContent, @@ -58,7 +58,7 @@ interface DraftGridSectionProps { ownerMap?: Record; } -export function DraftGridSection({ +export const DraftGridSection = memo(function DraftGridSection({ draftSlots, draftGrid, currentPick, @@ -377,4 +377,4 @@ export function DraftGridSection({ ); -} +}); diff --git a/app/components/draft/QueueSection.tsx b/app/components/draft/QueueSection.tsx index 6aef661..b68193e 100644 --- a/app/components/draft/QueueSection.tsx +++ b/app/components/draft/QueueSection.tsx @@ -1,3 +1,4 @@ +import { memo } from "react"; import { Button } from "~/components/ui/button"; import { Badge } from "~/components/ui/badge"; import { AutodraftSettings } from "~/components/AutodraftSettings"; @@ -134,7 +135,7 @@ function SortableQueueItem({ ); } -export function QueueSection({ +export const QueueSection = memo(function QueueSection({ queue, availableParticipants, seasonId, @@ -219,4 +220,4 @@ export function QueueSection({ /> ); -} +}); diff --git a/app/components/draft/SidebarRecentPicks.tsx b/app/components/draft/SidebarRecentPicks.tsx index fa4cf04..7214d95 100644 --- a/app/components/draft/SidebarRecentPicks.tsx +++ b/app/components/draft/SidebarRecentPicks.tsx @@ -1,3 +1,4 @@ +import { memo } from "react"; import { Badge } from "~/components/ui/badge"; interface SidebarRecentPicksProps { @@ -17,7 +18,7 @@ interface SidebarRecentPicksProps { }>; } -export function SidebarRecentPicks({ picks }: SidebarRecentPicksProps) { +export const SidebarRecentPicks = memo(function SidebarRecentPicks({ picks }: SidebarRecentPicksProps) { return (
{picks.length === 0 ? ( @@ -57,4 +58,4 @@ export function SidebarRecentPicks({ picks }: SidebarRecentPicksProps) { )}
); -} +}); diff --git a/app/components/draft/TeamsDraftedGrid.tsx b/app/components/draft/TeamsDraftedGrid.tsx index 5f1ef82..881ba86 100644 --- a/app/components/draft/TeamsDraftedGrid.tsx +++ b/app/components/draft/TeamsDraftedGrid.tsx @@ -1,5 +1,5 @@ +import { memo, useMemo } from "react"; import { Badge } from "~/components/ui/badge"; -import { useMemo } from "react"; interface TeamsDraftedGridProps { draftSlots: Array<{ @@ -34,7 +34,7 @@ interface TeamsDraftedGridProps { }; } -export function TeamsDraftedGrid({ +export const TeamsDraftedGrid = memo(function TeamsDraftedGrid({ draftSlots, picks, sports, @@ -165,4 +165,4 @@ export function TeamsDraftedGrid({ ); -} +}); diff --git a/app/hooks/useDraftNotifications.ts b/app/hooks/useDraftNotifications.ts index cdac59c..7952c81 100644 --- a/app/hooks/useDraftNotifications.ts +++ b/app/hooks/useDraftNotifications.ts @@ -38,11 +38,16 @@ export function useDraftNotifications(seasonId: string, userId: string) { setModeState(storedMode); } - // Watch for the user revoking/granting permission in browser settings + // Watch for the user revoking/granting permission in browser settings. + // Use an abort flag so that if the component unmounts before the promise + // resolves, the cleanup doesn't fail to clear onchange (permissionStatus + // would still be null at that point without the flag). + let aborted = false; let permissionStatus: PermissionStatus | null = null; navigator.permissions .query({ name: "notifications" }) .then((status) => { + if (aborted) return; permissionStatus = status; status.onchange = () => { setPermissionState(status.state as NotificationPermission); @@ -57,6 +62,7 @@ export function useDraftNotifications(seasonId: string, userId: string) { }); return () => { + aborted = true; if (permissionStatus) { permissionStatus.onchange = null; } diff --git a/app/hooks/useDraftSocket.ts b/app/hooks/useDraftSocket.ts index e559fe6..37f2bee 100644 --- a/app/hooks/useDraftSocket.ts +++ b/app/hooks/useDraftSocket.ts @@ -6,6 +6,9 @@ interface UseDraftSocketReturn { connectionError: string | null; isReconnecting: boolean; reconnectCount: number; + /** Increments each time a new socket instance is created. Include in + * useEffect deps so handler effects re-register on socket recreation. */ + socketVersion: number; on: (event: string, callback: (...args: any[]) => void) => void; off: (event: string, callback?: (...args: any[]) => void) => void; emit: (event: string, ...args: any[]) => void; @@ -18,6 +21,7 @@ export function useDraftSocket(seasonId: string, teamId?: string): UseDraftSocke const [connectionError, setConnectionError] = useState(null); const [isReconnecting, setIsReconnecting] = useState(false); const [reconnectCount, setReconnectCount] = useState(0); + const [socketVersion, setSocketVersion] = useState(0); useEffect(() => { // Reset per-socket state so a new seasonId/teamId doesn't inherit the previous @@ -31,6 +35,7 @@ export function useDraftSocket(seasonId: string, teamId?: string): UseDraftSocke }); socketRef.current = socket; + setSocketVersion((v) => v + 1); socket.on("connect", () => { console.log("Connected to Socket.IO:", socket.id); @@ -143,6 +148,7 @@ export function useDraftSocket(seasonId: string, teamId?: string): UseDraftSocke connectionError, isReconnecting, reconnectCount, + socketVersion, on, off, emit, diff --git a/app/routes/leagues/$leagueId.draft.$seasonId.tsx b/app/routes/leagues/$leagueId.draft.$seasonId.tsx index 23cb384..68beb71 100644 --- a/app/routes/leagues/$leagueId.draft.$seasonId.tsx +++ b/app/routes/leagues/$leagueId.draft.$seasonId.tsx @@ -232,7 +232,7 @@ export default function DraftRoom() { } = useLoaderData(); const { revalidate, state: revalidatorState } = useRevalidator(); const { userId: clerkUserId, getToken } = useAuth(); - const { isConnected, connectionError, isReconnecting, reconnectCount, on, off } = useDraftSocket(season.id, userTeam?.id); + const { isConnected, connectionError, isReconnecting, reconnectCount, socketVersion, on, off } = useDraftSocket(season.id, userTeam?.id); const { permissionState: notificationsPermission, enabled: notificationsEnabled, @@ -321,29 +321,48 @@ export default function DraftRoom() { // 60-second refresh may not have run. Getting a fresh token here prevents // the next user action from hitting a 401. useEffect(() => { + // Nothing to do once auth is fully degraded — avoid re-registering a + // listener that would immediately exit on every future visibility change. + if (authDegraded) return; + + let aborted = false; + let inFlight = false; + const handleVisibilityChange = async () => { - if (document.visibilityState !== "visible" || !clerkUserId || authDegraded) return; + // Ignore hide events, and skip if another invocation is already running + // (rapid tab switching) or this effect has been cleaned up. + if (document.visibilityState !== "visible" || !clerkUserId || inFlight) return; + inFlight = true; try { const token = await getToken({ skipCache: true }); + if (aborted) return; if (token === null) { // Retry once after a short delay — the network may still be waking up. await new Promise((r) => setTimeout(r, 1000)); + if (aborted) return; const retryToken = await getToken({ skipCache: true }); + if (aborted) return; if (retryToken === null) setAuthDegraded(true); } } catch { try { await new Promise((r) => setTimeout(r, 1000)); + if (aborted) return; const retryToken = await getToken({ skipCache: true }); + if (aborted) return; if (retryToken === null) setAuthDegraded(true); } catch { - setAuthDegraded(true); + if (!aborted) setAuthDegraded(true); } + } finally { + inFlight = false; } }; document.addEventListener("visibilitychange", handleVisibilityChange); - return () => + return () => { + aborted = true; document.removeEventListener("visibilitychange", handleVisibilityChange); + }; }, [clerkUserId, getToken, authDegraded]); // Track revalidation lifecycle to sync local state from fresh loader data. @@ -639,15 +658,19 @@ export default function DraftRoom() { }; const handleTimerUpdate = (data: any) => { - // Update the specific team's timer - setTeamTimers((prev) => ({ - ...prev, - [data.teamId]: data.timeRemaining, - })); + // Bail out without creating a new object reference if nothing changed. + // Without this guard the spread `{ ...prev }` always returns a new object, + // causing a full re-render of the DraftRoom tree on every 1-second tick. + setTeamTimers((prev) => { + if (prev[data.teamId] === data.timeRemaining) return prev; + return { ...prev, [data.teamId]: data.timeRemaining }; + }); // Also sync the current pick number from the server. This is critical // for mobile reconnection: timer-update events arrive every second, so // this ensures currentPick is correct within 1s of reconnecting — even // if the HTTP revalidation hasn't completed yet. + // React's useState already bails out when the new value equals the old + // one (Object.is comparison on primitives), so no manual guard needed. if (data.currentPickNumber != null) { setCurrentPick(data.currentPickNumber); } @@ -780,11 +803,13 @@ export default function DraftRoom() { off("draft-rolled-back", handleDraftRolledBack); off("draft-state-sync", handleDraftStateSync); }; - // `on`/`off` are stable useCallback refs — this effect runs once per socket instance. - // `draftSlots`, `userTeam`, and `season.league.name` are intentionally omitted: they - // come from useLoaderData and are immutable for the lifetime of the page load. + // `on`/`off` are stable useCallback refs. `socketVersion` increments whenever + // useDraftSocket creates a new socket instance, ensuring this effect re-runs + // and re-registers all handlers on the fresh socket. + // `draftSlots`, `userTeam`, and `season.league.name` are intentionally omitted: + // they come from useLoaderData and are immutable for the lifetime of the page load. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [on, off]); + }, [on, off, socketVersion]); // Persist sidebar collapsed state useEffect(() => { @@ -794,7 +819,7 @@ export default function DraftRoom() { }, [sidebarCollapsed]); // Queue handlers - const handleAddToQueue = async (participantId: string) => { + const handleAddToQueue = useCallback(async (participantId: string) => { if (!userTeam) return; // Check if participant is already in queue (client-side) @@ -850,9 +875,12 @@ export default function DraftRoom() { setQueue((prev) => prev.filter((item) => item.id !== tempQueueItem.id)); toast.error("Network error - failed to add to queue"); } - }; + // queue is read directly (not via functional updater) for the duplicate check and + // optimistic position, so it must be a dep. This is fine: AvailableParticipantsSection + // already re-renders when queue changes (it receives queue as a prop). + }, [userTeam, queue, season.id, authFetch]); - const handleRemoveFromQueue = async (queueId: string) => { + const handleRemoveFromQueue = useCallback(async (queueId: string) => { if (!userTeam) return; // Optimistically remove the item immediately @@ -885,9 +913,9 @@ export default function DraftRoom() { setQueue(previousQueue); toast.error("Network error - failed to remove from queue"); } - }; + }, [userTeam, authFetch]); - const handleReorderQueue = async (participantIds: string[]) => { + const handleReorderQueue = useCallback(async (participantIds: string[]) => { if (!userTeam) return; // Optimistically reorder the queue immediately @@ -923,7 +951,7 @@ export default function DraftRoom() { setQueue(previousQueue); toast.error("Network error - failed to reorder queue"); } - }; + }, [userTeam, season.id, authFetch]); const handleStartDraft = async () => { try { @@ -991,7 +1019,7 @@ export default function DraftRoom() { } }; - const handleMakePick = async (participantId: string) => { + const handleMakePick = useCallback(async (participantId: string) => { try { const formData = new FormData(); formData.append("seasonId", season.id); @@ -1010,9 +1038,9 @@ export default function DraftRoom() { } catch { toast.error("Network error - failed to make pick"); } - }; + }, [season.id, authFetch]); - const handleForceAutopick = async (pickNumber: number, teamId: string) => { + const handleForceAutopick = useCallback(async (pickNumber: number, teamId: string) => { try { const formData = new FormData(); formData.append("seasonId", season.id); @@ -1032,7 +1060,7 @@ export default function DraftRoom() { } catch { toast.error("Network error - failed to force autopick"); } - }; + }, [season.id, authFetch]); const handleForceManualPick = async (participantId: string) => { if (!selectedPickSlot) return; @@ -1062,7 +1090,7 @@ export default function DraftRoom() { } }; - const handleReplacePickOpen = (pickNumber: number, teamId: string) => { + const handleReplacePickOpen = useCallback((pickNumber: number, teamId: string) => { const pick = picks.find((p: any) => p.pickNumber === pickNumber); if (!pick) { toast.error("Pick not found — try refreshing the page"); @@ -1070,7 +1098,8 @@ export default function DraftRoom() { } setReplacePickSlot({ pickNumber, teamId, oldParticipantId: pick.participant.id }); setReplacePickDialogOpen(true); - }; + // picks is read directly to find the pick being replaced. + }, [picks]); const handleReplacePick = async (participantId: string) => { if (!replacePickSlot) return; @@ -1100,10 +1129,10 @@ export default function DraftRoom() { } }; - const handleRollbackToPick = (pickNumber: number) => { + const handleRollbackToPick = useCallback((pickNumber: number) => { setRollbackPickNumber(pickNumber); setRollbackConfirmOpen(true); - }; + }, []); const handleConfirmRollback = async () => { if (!rollbackPickNumber || isRollingBack) return; @@ -1135,13 +1164,13 @@ export default function DraftRoom() { } }; - const handleAdjustTimeBankOpen = (teamId: string) => { + const handleAdjustTimeBankOpen = useCallback((teamId: string) => { setTimeBankTeamId(teamId); setTimeBankAmount("1"); setTimeBankUnit("minutes"); setTimeBankDirection("add"); setTimeBankDialogOpen(true); - }; + }, []); const handleConfirmAdjustTimeBank = async () => { if (!timeBankTeamId || isAdjustingTimeBank) return; @@ -1193,10 +1222,10 @@ export default function DraftRoom() { } }; - const handleForceManualPickOpen = (pickNumber: number, teamId: string) => { + const handleForceManualPickOpen = useCallback((pickNumber: number, teamId: string) => { setSelectedPickSlot({ pickNumber, teamId }); setForcePickDialogOpen(true); - }; + }, []); // Calculate current round const currentRound = draftSlots.length > 0 ? Math.ceil(currentPick / draftSlots.length) : 1; @@ -1319,6 +1348,13 @@ export default function DraftRoom() { }, [availableParticipants, hideDrafted, hideIneligible, hideCompletedSports, eligibility, draftedParticipantIds, userDraftedSportIds, searchQuery, sportFilters]); // Shared component props — defined once to avoid duplication between desktop and mobile layouts + const handleHideCompletedSportsChange = useCallback((hide: boolean) => { + setHideCompletedSports(hide); + if (hide) { + setSportFilters((prev) => prev.filter((s) => !userDraftedSportNames.has(s))); + } + }, [userDraftedSportNames]); + const availableParticipantsSectionProps = { participants: filteredParticipants, searchQuery, @@ -1337,12 +1373,7 @@ export default function DraftRoom() { onHideIneligibleChange: setHideIneligible, hideCompletedSports, userDraftedSportNames, - onHideCompletedSportsChange: (hide: boolean) => { - setHideCompletedSports(hide); - if (hide) { - setSportFilters((prev) => prev.filter((s) => !userDraftedSportNames.has(s))); - } - }, + onHideCompletedSportsChange: handleHideCompletedSportsChange, onMakePick: handleMakePick, onAddToQueue: handleAddToQueue, onRemoveFromQueue: handleRemoveFromQueue, @@ -1364,14 +1395,23 @@ export default function DraftRoom() { onRollbackToPick: isCommissioner && !isDraftComplete ? handleRollbackToPick : undefined, }; + const teamsDraftedGridSeason = useMemo(() => ({ numFlexPicks }), [numFlexPicks]); + const teamsDraftedGridProps = { draftSlots, picks, sports: seasonSportsData, - season: { numFlexPicks }, + season: teamsDraftedGridSeason, ownerMap, }; + const handleAutodraftUpdate = useCallback( + (isEnabled: boolean, mode: "next_pick" | "while_on", queueOnly: boolean) => { + setUserAutodraft({ isEnabled, mode, queueOnly }); + }, + [] + ); + const queueSectionProps = userTeam ? { queue, @@ -1382,9 +1422,7 @@ export default function DraftRoom() { canPick, userAutodraft, onRemoveFromQueue: handleRemoveFromQueue, - onAutodraftUpdate: (isEnabled: boolean, mode: "next_pick" | "while_on", queueOnly: boolean) => { - setUserAutodraft({ isEnabled, mode, queueOnly }); - }, + onAutodraftUpdate: handleAutodraftUpdate, onReorder: handleReorderQueue, onMakePick: handleMakePick, }