fix(#338): Fix useWebSocket stale closure by using refs for callbacks
- Use useRef to store callbacks, preventing stale closures - Remove callback functions from useEffect dependencies - Only workspaceId and token trigger reconnects now - Callback changes update the ref without causing reconnects - Add 5 new tests verifying no reconnect on callback changes Refs #338 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -215,6 +215,140 @@ describe("useWebSocket", (): void => {
|
||||
expect(io).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
describe("stale closure prevention", (): void => {
|
||||
it("should NOT disconnect when callback functions change", (): void => {
|
||||
const onTaskCreated1 = vi.fn();
|
||||
const onTaskCreated2 = vi.fn();
|
||||
|
||||
const { rerender } = renderHook(
|
||||
({ onTaskCreated }: { onTaskCreated: (task: { id: string }) => void }) =>
|
||||
useWebSocket("workspace-123", "token", { onTaskCreated }),
|
||||
{ initialProps: { onTaskCreated: onTaskCreated1 } }
|
||||
);
|
||||
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
expect(mockSocket.disconnect).not.toHaveBeenCalled();
|
||||
|
||||
// Change the callback - this should NOT cause a reconnect
|
||||
rerender({ onTaskCreated: onTaskCreated2 });
|
||||
|
||||
// Socket should NOT have been disconnected or reconnected
|
||||
expect(mockSocket.disconnect).not.toHaveBeenCalled();
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should use the latest callback after callback change", async (): Promise<void> => {
|
||||
const onTaskCreated1 = vi.fn();
|
||||
const onTaskCreated2 = vi.fn();
|
||||
|
||||
const { rerender } = renderHook(
|
||||
({ onTaskCreated }: { onTaskCreated: (task: { id: string }) => void }) =>
|
||||
useWebSocket("workspace-123", "token", { onTaskCreated }),
|
||||
{ initialProps: { onTaskCreated: onTaskCreated1 } }
|
||||
);
|
||||
|
||||
// Emit event with first callback
|
||||
const task1 = { id: "task-1" };
|
||||
act(() => {
|
||||
eventHandlers["task:created"]?.(task1);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onTaskCreated1).toHaveBeenCalledWith(task1);
|
||||
expect(onTaskCreated2).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Update to new callback
|
||||
rerender({ onTaskCreated: onTaskCreated2 });
|
||||
|
||||
// Emit another event - should use the new callback
|
||||
const task2 = { id: "task-2" };
|
||||
act(() => {
|
||||
eventHandlers["task:created"]?.(task2);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onTaskCreated2).toHaveBeenCalledWith(task2);
|
||||
// First callback should only have been called once (with task1)
|
||||
expect(onTaskCreated1).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("should NOT disconnect when multiple callbacks change simultaneously", (): void => {
|
||||
const callbacks1 = {
|
||||
onTaskCreated: vi.fn(),
|
||||
onTaskUpdated: vi.fn(),
|
||||
onEventCreated: vi.fn(),
|
||||
};
|
||||
const callbacks2 = {
|
||||
onTaskCreated: vi.fn(),
|
||||
onTaskUpdated: vi.fn(),
|
||||
onEventCreated: vi.fn(),
|
||||
};
|
||||
|
||||
interface CallbackProps {
|
||||
onTaskCreated: (task: { id: string }) => void;
|
||||
onTaskUpdated: (task: { id: string }) => void;
|
||||
onEventCreated: (event: { id: string }) => void;
|
||||
}
|
||||
|
||||
const { rerender } = renderHook(
|
||||
(props: CallbackProps) => useWebSocket("workspace-123", "token", props),
|
||||
{ initialProps: callbacks1 }
|
||||
);
|
||||
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Change all callbacks at once - should NOT cause reconnect
|
||||
rerender(callbacks2);
|
||||
|
||||
expect(mockSocket.disconnect).not.toHaveBeenCalled();
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should handle callback being removed without reconnect", (): void => {
|
||||
const onTaskCreated = vi.fn();
|
||||
|
||||
interface CallbackProps {
|
||||
onTaskCreated?: (task: { id: string }) => void;
|
||||
}
|
||||
|
||||
const { rerender } = renderHook(
|
||||
(props: CallbackProps) => useWebSocket("workspace-123", "token", props),
|
||||
{ initialProps: { onTaskCreated } as CallbackProps }
|
||||
);
|
||||
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Remove callback - should NOT cause reconnect
|
||||
rerender({});
|
||||
|
||||
expect(mockSocket.disconnect).not.toHaveBeenCalled();
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should handle callback being added without reconnect", (): void => {
|
||||
const onTaskCreated = vi.fn();
|
||||
|
||||
interface CallbackProps {
|
||||
onTaskCreated?: (task: { id: string }) => void;
|
||||
}
|
||||
|
||||
const { rerender } = renderHook(
|
||||
(props: CallbackProps) => useWebSocket("workspace-123", "token", props),
|
||||
{ initialProps: {} as CallbackProps }
|
||||
);
|
||||
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Add callback - should NOT cause reconnect
|
||||
rerender({ onTaskCreated });
|
||||
|
||||
expect(mockSocket.disconnect).not.toHaveBeenCalled();
|
||||
expect(io).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("should clean up all event listeners on unmount", (): void => {
|
||||
const { unmount } = renderHook(() =>
|
||||
useWebSocket("workspace-123", "token", {
|
||||
|
||||
Reference in New Issue
Block a user