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 <noreply@anthropic.com>
This commit is contained in:
parent
7b491d86d2
commit
ca2fd288ab
8 changed files with 107 additions and 55 deletions
|
|
@ -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({
|
|||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<string, string>;
|
||||
}
|
||||
|
||||
export function DraftGridSection({
|
||||
export const DraftGridSection = memo(function DraftGridSection({
|
||||
draftSlots,
|
||||
draftGrid,
|
||||
currentPick,
|
||||
|
|
@ -377,4 +377,4 @@ export function DraftGridSection({
|
|||
</Sheet>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 (
|
||||
<div className="p-4">
|
||||
{picks.length === 0 ? (
|
||||
|
|
@ -57,4 +58,4 @@ export function SidebarRecentPicks({ picks }: SidebarRecentPicksProps) {
|
|||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
</table>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<string | null>(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,
|
||||
|
|
|
|||
|
|
@ -232,7 +232,7 @@ export default function DraftRoom() {
|
|||
} = useLoaderData<typeof loader>();
|
||||
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,
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue