From 2961c29374108939dfe4b244e453d1d6984e9a77 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Wed, 25 Feb 2026 17:34:12 +0000
Subject: [PATCH] Fix four issues from useDraftSocket code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- 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
---
 app/hooks/useDraftSocket.ts | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/hooks/useDraftSocket.ts b/app/hooks/useDraftSocket.ts
index d630d07..e559fe6 100644
--- a/app/hooks/useDraftSocket.ts
+++ b/app/hooks/useDraftSocket.ts
@@ -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);
   }, []);