From d364b0988594b95fd571da67b9269daf3b809c33 Mon Sep 17 00:00:00 2001 From: Matt Ciaccio Date: Sat, 2 May 2026 23:11:52 +0200 Subject: [PATCH] fix(realtime): keep socket through reconnects, stop re-subscribe storm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two correctness bugs in the real-time stack — both silent failures, both session-wide once they trigger. (1) `SocketProvider` was setting the React context to null on every `disconnect` event. socket.io's built-in reconnection re-establishes the underlying transport and replays handlers, but the React tree had already lost its reference to the socket — so every `useSocket()` consumer saw null until a session/port change forced a remount. Effect: after the first transient drop (laptop sleep, wifi blip, server restart), realtime invalidation and toasts went dead session-wide with no user-visible signal. Fix: keep the socket reference stable for the lifetime of the session+port, and surface a separate `isConnected` boolean for any UI that wants to render an offline indicator. Exposed as a new `useIsSocketConnected()` hook; `useSocket()` signature is unchanged. (2) `useRealtimeInvalidation` captured `eventMap` as a useEffect dependency. Every caller passes a fresh `{ ... }` object literal on each render, so the effect re-ran every render → `socket.off`/`socket.on` storm on pages with many subscribed events. Fix: extract the subscription logic into a pure helper (`realtime-invalidation-core.ts`, JSX-free for vitest). The hook now keeps the latest map in a ref and only re-subscribes when the SET of event names changes (joined-keys signature, not object identity). The handler reads `ref.current` at fire time, so callers still see fresh queryKey lists without re-binding. Helper is unit-tested with a stub socket: registration count, fire-time map lookup, cleanup deregistration, missing-event safety. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/hooks/realtime-invalidation-core.ts | 52 ++++++ src/hooks/use-realtime-invalidation.ts | 54 +++--- src/providers/socket-provider.tsx | 56 ++++++- .../unit/hooks/realtime-invalidation.test.ts | 158 ++++++++++++++++++ 4 files changed, 287 insertions(+), 33 deletions(-) create mode 100644 src/hooks/realtime-invalidation-core.ts create mode 100644 tests/unit/hooks/realtime-invalidation.test.ts diff --git a/src/hooks/realtime-invalidation-core.ts b/src/hooks/realtime-invalidation-core.ts new file mode 100644 index 0000000..876611c --- /dev/null +++ b/src/hooks/realtime-invalidation-core.ts @@ -0,0 +1,52 @@ +import type { QueryClient, QueryKey } from '@tanstack/react-query'; + +/** Minimum surface of socket.io's client we use here. Kept loose so the + * helper can be unit-tested with a stub object without dragging the full + * socket.io dependency into the test runtime. */ +export interface SocketLike { + on(event: string, handler: (...args: unknown[]) => void): unknown; + off(event: string, handler: (...args: unknown[]) => void): unknown; +} + +export type EventMap = Record; + +/** + * Pure subscription logic for `useRealtimeInvalidation`. Registers one + * handler per event key. Each handler reads the latest eventMap from the + * supplied getter so callers can pass a fresh object literal on every render + * without re-subscribing. + * + * Returns a cleanup function that removes the registered handlers. + * + * Lives in its own JSX-free file so it can be unit-tested under vitest's + * node environment without dragging the React provider into the bundle. + */ +export function subscribeRealtimeInvalidations( + socket: SocketLike, + eventKeys: string[], + queryClient: Pick, + getEventMap: () => EventMap, +): () => void { + const handlers: Array<{ event: string; handler: (...args: unknown[]) => void }> = []; + + for (const event of eventKeys) { + const handler = () => { + // Read the LATEST map at fire-time — not at subscription time — so + // callers passing inline `{ 'client:created': [...] }` literals don't + // bind a stale snapshot if they re-render. + const queryKeys = getEventMap()[event]; + if (!queryKeys) return; + for (const key of queryKeys) { + queryClient.invalidateQueries({ queryKey: key }); + } + }; + socket.on(event, handler); + handlers.push({ event, handler }); + } + + return () => { + for (const { event, handler } of handlers) { + socket.off(event, handler); + } + }; +} diff --git a/src/hooks/use-realtime-invalidation.ts b/src/hooks/use-realtime-invalidation.ts index be16608..93cc2f8 100644 --- a/src/hooks/use-realtime-invalidation.ts +++ b/src/hooks/use-realtime-invalidation.ts @@ -1,14 +1,20 @@ 'use client'; -import { useEffect } from 'react'; -import { useQueryClient, type QueryKey } from '@tanstack/react-query'; +import { useEffect, useRef } from 'react'; +import { useQueryClient } from '@tanstack/react-query'; import { useSocket } from '@/providers/socket-provider'; +import { subscribeRealtimeInvalidations, type EventMap } from '@/hooks/realtime-invalidation-core'; + +// Re-export for convenience so callers don't need to know about the split. +export type { EventMap, SocketLike } from '@/hooks/realtime-invalidation-core'; /** * Subscribes to socket events and invalidates React Query caches. * - * @param eventMap - Maps socket event names to arrays of query keys to invalidate. + * Safe to call with an inline-literal `eventMap` — the hook only re-subscribes + * when the SET of event keys actually changes (not when the object identity + * changes). The latest query-key list is read at event fire-time via a ref. * * @example * useRealtimeInvalidation({ @@ -17,31 +23,29 @@ import { useSocket } from '@/providers/socket-provider'; * 'client:archived': [['clients']], * }); */ -export function useRealtimeInvalidation( - eventMap: Record, -) { +export function useRealtimeInvalidation(eventMap: EventMap) { const socket = useSocket(); const queryClient = useQueryClient(); + // Stash the latest map in a ref so handlers always see fresh queryKeys + // without re-subscribing. + const eventMapRef = useRef(eventMap); + eventMapRef.current = eventMap; + + // Re-subscribe ONLY when the set of event names changes. Object identity + // of `eventMap` flips on every caller render; the joined key signature + // doesn't. + const eventKeysSig = Object.keys(eventMap).sort().join('|'); + useEffect(() => { if (!socket) return; - - const handlers: Array<{ event: string; handler: (...args: unknown[]) => void }> = []; - - for (const [event, queryKeys] of Object.entries(eventMap)) { - const handler = () => { - for (const key of queryKeys) { - queryClient.invalidateQueries({ queryKey: key }); - } - }; - socket.on(event, handler); - handlers.push({ event, handler }); - } - - return () => { - for (const { event, handler } of handlers) { - socket.off(event, handler); - } - }; - }, [socket, queryClient, eventMap]); + // eventMapRef is intentionally not in deps — it's a ref; we only want to + // re-run when the socket, queryClient, or the event-key SET changes. + return subscribeRealtimeInvalidations( + socket, + eventKeysSig.length > 0 ? eventKeysSig.split('|') : [], + queryClient, + () => eventMapRef.current, + ); + }, [socket, queryClient, eventKeysSig]); } diff --git a/src/providers/socket-provider.tsx b/src/providers/socket-provider.tsx index 75ab810..b55f00d 100644 --- a/src/providers/socket-provider.tsx +++ b/src/providers/socket-provider.tsx @@ -13,7 +13,20 @@ import { io, type Socket } from 'socket.io-client'; import { useSession } from '@/lib/auth/client'; import { usePortStore } from '@/stores/ui-store'; -const SocketContext = createContext(null); +interface SocketContextValue { + /** Stable socket instance reference. Persists across reconnects — socket.io's + * built-in reconnection re-establishes the underlying transport without + * changing the JS object, so this stays valid as long as the session and + * port are unchanged. Consumers should NOT null-check this for "is online"; + * use `isConnected` instead. */ + socket: Socket | null; + /** Live transport state. Flips false on disconnect and back to true on + * reconnect. Use this if you need to surface offline UX; the socket itself + * stays subscribed to the same event handlers. */ + isConnected: boolean; +} + +const SocketContext = createContext({ socket: null, isConnected: false }); /** Returns true once the component has mounted on the client. Avoids calling * better-auth's `useSession()` (which dispatches React hooks via nanostores) @@ -32,7 +45,9 @@ export function SocketProvider({ children }: { children: ReactNode }) { return hasMounted ? ( {children} ) : ( - {children} + + {children} + ); } @@ -40,9 +55,14 @@ function SocketProviderClient({ children }: { children: ReactNode }) { const { data: session } = useSession(); const currentPortId = usePortStore((s) => s.currentPortId); const [socket, setSocket] = useState(null); + const [isConnected, setIsConnected] = useState(false); useEffect(() => { - if (!session?.user || !currentPortId) return; + if (!session?.user || !currentPortId) { + setSocket(null); + setIsConnected(false); + return; + } const s = io(process.env.NEXT_PUBLIC_APP_URL!, { path: '/socket.io/', @@ -51,18 +71,38 @@ function SocketProviderClient({ children }: { children: ReactNode }) { transports: ['websocket', 'polling'], }); - s.on('connect', () => setSocket(s)); - s.on('disconnect', () => setSocket(null)); + // Set the socket reference immediately and keep it stable across the + // session+port lifetime. socket.io reconnects internally; the same + // instance survives transient drops, and any handlers registered via + // `socket.on(...)` stay attached. Previously we set/unset `socket` on + // connect/disconnect, which made the React context flip to null on every + // network blip and silently killed every `useRealtimeInvalidation` + // subscription session-wide. + setSocket(s); + + s.on('connect', () => setIsConnected(true)); + s.on('disconnect', () => setIsConnected(false)); return () => { s.disconnect(); setSocket(null); + setIsConnected(false); }; }, [session?.user, currentPortId]); - return {children}; + return ( + {children} + ); } -export function useSocket() { - return useContext(SocketContext); +/** Returns the Socket.IO client instance. The reference is stable for the + * duration of a session+port, even across transient disconnects. */ +export function useSocket(): Socket | null { + return useContext(SocketContext).socket; +} + +/** True while the socket transport is connected. Flips false on disconnect, + * back to true on reconnect. Useful for surfacing an "offline" indicator. */ +export function useIsSocketConnected(): boolean { + return useContext(SocketContext).isConnected; } diff --git a/tests/unit/hooks/realtime-invalidation.test.ts b/tests/unit/hooks/realtime-invalidation.test.ts new file mode 100644 index 0000000..3bc2df4 --- /dev/null +++ b/tests/unit/hooks/realtime-invalidation.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { QueryClient, QueryKey } from '@tanstack/react-query'; + +import { + subscribeRealtimeInvalidations, + type EventMap, + type SocketLike, +} from '@/hooks/realtime-invalidation-core'; + +/** + * Pure-logic tests for the realtime-invalidation subscription helper. The + * React hook (`useRealtimeInvalidation`) is just a thin wrapper around this + * function — verifying the handler-registration / fire-time-lookup behavior + * here is sufficient to lock in the bug fixes: + * 1. Re-subscribe storm (caller passing inline literals) + * 2. Fresh queryKeys read at fire-time + * + * The `useSocket` provider fix (don't null-context on disconnect) is verified + * separately by manual smoke + the existing socket integration coverage. + */ + +function makeStubSocket() { + const listeners = new Map void>>(); + const onCalls: Array<{ event: string }> = []; + const offCalls: Array<{ event: string }> = []; + + const socket: SocketLike = { + on(event, handler) { + onCalls.push({ event }); + const arr = listeners.get(event) ?? []; + arr.push(handler); + listeners.set(event, arr); + }, + off(event, handler) { + offCalls.push({ event }); + const arr = listeners.get(event) ?? []; + listeners.set( + event, + arr.filter((h) => h !== handler), + ); + }, + }; + + function emit(event: string, ...args: unknown[]) { + for (const h of listeners.get(event) ?? []) h(...args); + } + + return { socket, emit, onCalls, offCalls, listeners }; +} + +function makeStubQueryClient() { + const calls: QueryKey[] = []; + const queryClient = { + invalidateQueries: vi.fn(({ queryKey }: { queryKey: QueryKey }) => { + calls.push(queryKey); + return Promise.resolve(); + }), + } as unknown as QueryClient; + return { queryClient, calls }; +} + +describe('subscribeRealtimeInvalidations', () => { + it('registers one .on() per event key', () => { + const { socket, onCalls } = makeStubSocket(); + const { queryClient } = makeStubQueryClient(); + const map: EventMap = { + 'client:created': [['clients']], + 'client:updated': [['clients'], ['clients', 'abc']], + }; + + subscribeRealtimeInvalidations(socket, Object.keys(map), queryClient, () => map); + + expect(onCalls.map((c) => c.event).sort()).toEqual(['client:created', 'client:updated']); + }); + + it('invalidates each queryKey for the matching event', () => { + const { socket, emit } = makeStubSocket(); + const { queryClient, calls } = makeStubQueryClient(); + const map: EventMap = { + 'client:updated': [['clients'], ['clients', 'abc']], + }; + + subscribeRealtimeInvalidations(socket, Object.keys(map), queryClient, () => map); + emit('client:updated'); + + expect(calls).toEqual([['clients'], ['clients', 'abc']]); + }); + + it('reads the LATEST eventMap at fire time, not at subscription time', () => { + // This is the core of the re-subscribe-storm fix: callers can swap in a + // new eventMap object without re-subscribing, and the handler still sees + // the fresh queryKey list. + const { socket, emit } = makeStubSocket(); + const { queryClient, calls } = makeStubQueryClient(); + + let currentMap: EventMap = { + 'client:updated': [['clients']], + }; + subscribeRealtimeInvalidations(socket, ['client:updated'], queryClient, () => currentMap); + + // First fire: see the original map + emit('client:updated'); + expect(calls).toEqual([['clients']]); + + // Caller re-renders with a fresh literal that includes more queryKeys + currentMap = { + 'client:updated': [['clients'], ['clients', 'abc']], + }; + emit('client:updated'); + + expect(calls).toEqual([['clients'], ['clients'], ['clients', 'abc']]); + }); + + it('cleanup deregisters every handler it registered', () => { + const { socket, emit, offCalls, listeners } = makeStubSocket(); + const { queryClient, calls } = makeStubQueryClient(); + const map: EventMap = { + 'a:event': [['a']], + 'b:event': [['b']], + }; + + const cleanup = subscribeRealtimeInvalidations( + socket, + Object.keys(map), + queryClient, + () => map, + ); + + cleanup(); + + expect(offCalls.map((c) => c.event).sort()).toEqual(['a:event', 'b:event']); + // All listeners removed — emitting after cleanup invalidates nothing. + emit('a:event'); + emit('b:event'); + expect(calls).toEqual([]); + // Defensive: the listener list should be empty after cleanup. + expect(listeners.get('a:event')?.length ?? 0).toBe(0); + expect(listeners.get('b:event')?.length ?? 0).toBe(0); + }); + + it('silently ignores events that have no entry in the current map', () => { + // If the caller swaps an event OUT mid-session, the registered handler + // still fires (we don't re-subscribe) but should be a no-op rather than + // throw. + const { socket, emit } = makeStubSocket(); + const { queryClient, calls } = makeStubQueryClient(); + + let currentMap: EventMap = { + 'client:updated': [['clients']], + }; + subscribeRealtimeInvalidations(socket, ['client:updated'], queryClient, () => currentMap); + + // Wipe the entry — handler will fire but find nothing to invalidate. + currentMap = {}; + expect(() => emit('client:updated')).not.toThrow(); + expect(calls).toEqual([]); + }); +});