Fix four issues from useDraftSocket code review

- Manager listener leak: add socket.io.off() for reconnect_attempt and
  reconnect_failed in cleanup — socket.disconnect() only tears down the
  socket, not the Manager listeners, causing them to accumulate on re-mounts.

- reconnect_failed dead code: add reconnectionAttempts: 10 to io() config
  so the handler is actually reachable after exhausting retries.

- connectionError flicker: remove setConnectionError from connect_error —
  reconnect_attempt fires immediately after and clears it anyway, causing the
  error overlay to flash on every retry cycle. Error now only appears via
  reconnect_failed once all attempts are exhausted. connect_error instead
  ensures setIsReconnecting(true) so the reconnecting overlay shows instead
  of the initial "Connecting to Draft" spinner.

- Add comment to on/off/emit noting they are no-ops if called before the
  effect runs (socketRef.current === null).

https://claude.ai/code/session_016tCZVFjSeHdQsdKktbDHEt
This commit is contained in:
Claude 2026-02-25 17:34:12 +00:00
parent 80ab6a3650
commit 2961c29374
No known key found for this signature in database

View file

@ -27,6 +27,7 @@ export function useDraftSocket(seasonId: string, teamId?: string): UseDraftSocke
const socket = io({
path: "/socket.io",
transports: ["websocket", "polling"],
reconnectionAttempts: 10,
});
socketRef.current = socket;
@ -56,9 +57,11 @@ export function useDraftSocket(seasonId: string, teamId?: string): UseDraftSocke
socket.on("connect_error", (error) => {
console.error("Socket.IO connection error:", error);
setConnectionError(error.message || "Failed to connect to draft server");
// Don't clear isReconnecting here — reconnect_attempt fires immediately
// after and sets it back to true, causing a visible flicker in the overlay.
// Don't set connectionError here — reconnect_attempt fires immediately after
// and would clear it again, causing the error overlay to flicker on every
// retry. Only show a hard error once all attempts are exhausted (reconnect_failed).
// Ensure the "reconnecting" overlay is visible rather than the initial spinner.
setIsReconnecting(true);
});
socket.io.on("reconnect_attempt", () => {
@ -108,6 +111,8 @@ export function useDraftSocket(seasonId: string, teamId?: string): UseDraftSocke
document.addEventListener("visibilitychange", handleVisibilityChange);
return () => {
socket.io.off("reconnect_attempt");
socket.io.off("reconnect_failed");
window.removeEventListener("offline", handleOffline);
window.removeEventListener("online", handleReturn);
document.removeEventListener("visibilitychange", handleVisibilityChange);
@ -117,6 +122,10 @@ export function useDraftSocket(seasonId: string, teamId?: string): UseDraftSocke
};
}, [seasonId, teamId]);
// These read socketRef.current at call time, so calls made before the effect
// has run (socketRef.current === null) are silently no-ops. Consumers should
// only call them inside their own useEffect, not during render or synchronously
// after mount.
const on = useCallback((event: string, callback: (...args: any[]) => void) => {
socketRef.current?.on(event, callback);
}, []);