diff --git a/src/hooks/useSmartPolling.test.ts b/src/hooks/useSmartPolling.test.ts new file mode 100644 index 00000000..e8cd9177 --- /dev/null +++ b/src/hooks/useSmartPolling.test.ts @@ -0,0 +1,135 @@ +import { act, renderHook } from '@testing-library/react'; +import { useSmartPolling } from './useSmartPolling'; + +describe('useSmartPolling (#570)', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('starts polling when enabled=true', async () => { + const fetchFn = jest.fn().mockResolvedValue('data-1'); + const { result, unmount } = renderHook(() => + useSmartPolling({ fetchFn, interval: 1000, enabled: true }) + ); + + await act(async () => { + await Promise.resolve(); + }); + + expect(result.current.isPolling).toBe(true); + expect(fetchFn).toHaveBeenCalled(); + + unmount(); + }); + + it('sets isPolling=false and stops timer when toggled to enabled=false (#570)', async () => { + const fetchFn = jest.fn().mockResolvedValue('data-1'); + const { result, rerender, unmount } = renderHook( + ({ enabled }) => useSmartPolling({ fetchFn, interval: 1000, enabled }), + { initialProps: { enabled: true } } + ); + + await act(async () => { + await Promise.resolve(); + }); + expect(result.current.isPolling).toBe(true); + + // Toggle enabled to false + act(() => { + rerender({ enabled: false }); + }); + + expect(result.current.isPolling).toBe(false); + + const callCountBefore = fetchFn.mock.calls.length; + act(() => { + jest.advanceTimersByTime(5000); + }); + expect(fetchFn.mock.calls.length).toBe(callCountBefore); + + unmount(); + }); + + it('re-starts polling immediately when toggled from false back to true (#570)', async () => { + const fetchFn = jest.fn().mockResolvedValue('data-1'); + const { result, rerender, unmount } = renderHook( + ({ enabled }) => useSmartPolling({ fetchFn, interval: 1000, enabled }), + { initialProps: { enabled: true } } + ); + + await act(async () => { + await Promise.resolve(); + }); + + // Disable + act(() => { + rerender({ enabled: false }); + }); + expect(result.current.isPolling).toBe(false); + + // Re-enable + const callsBeforeReenable = fetchFn.mock.calls.length; + act(() => { + rerender({ enabled: true }); + }); + + await act(async () => { + await Promise.resolve(); + }); + + unmount(); + }); + + it('reschedules running timer when backoff applies and resets on data change (#569)', async () => { + const fetchFn = jest.fn().mockResolvedValue('static-data'); + const { result, unmount } = renderHook(() => + useSmartPolling({ + fetchFn, + interval: 1000, + enabled: true, + unchangedThreshold: 2, + backoffMultiplier: 2, + maxBackoff: 4000, + }) + ); + + await act(async () => { + await Promise.resolve(); + }); + + // Advance 1st interval -> 2nd fetch (unchanged count = 1) + await act(async () => { + jest.advanceTimersByTime(1000); + await Promise.resolve(); + }); + + // Advance 2nd interval -> 3rd fetch (unchanged count = 2 -> enters backoff: interval becomes 2000ms) + await act(async () => { + jest.advanceTimersByTime(1000); + await Promise.resolve(); + }); + + expect(result.current.isBackingOff).toBe(true); + const countAtBackoff = fetchFn.mock.calls.length; + + // Advance by old interval (1000ms) -> should NOT trigger fetch because timer was rescheduled to 2000ms + await act(async () => { + jest.advanceTimersByTime(1000); + await Promise.resolve(); + }); + expect(fetchFn.mock.calls.length).toBe(countAtBackoff); + + // Advance remaining 1000ms (total 2000ms) -> should trigger fetch + await act(async () => { + jest.advanceTimersByTime(1000); + await Promise.resolve(); + }); + expect(fetchFn.mock.calls.length).toBe(countAtBackoff + 1); + + unmount(); + }); +}); diff --git a/src/hooks/useSmartPolling.ts b/src/hooks/useSmartPolling.ts index 81507fc4..5e1824db 100644 --- a/src/hooks/useSmartPolling.ts +++ b/src/hooks/useSmartPolling.ts @@ -39,12 +39,16 @@ const pollingInitialState: PollingState = { function pollingReducer(state: PollingState, action: PollingAction): PollingState { switch (action.type) { case 'START_POLLING': + if (state.isPolling) return state; return { ...state, isPolling: true }; case 'STOP_POLLING': + if (!state.isPolling) return state; return { ...state, isPolling: false }; case 'START_BACKOFF': + if (state.isBackingOff) return state; return { ...state, isBackingOff: true }; case 'STOP_BACKOFF': + if (!state.isBackingOff) return state; return { ...state, isBackingOff: false }; default: return state; @@ -73,26 +77,41 @@ export function useSmartPolling({ const previousDataRef = useRef(null); const isBackgroundedRef = useRef(false); + const fetchFnRef = useRef(fetchFn); + fetchFnRef.current = fetchFn; + + const compareFnRef = useRef(compareFn); + compareFnRef.current = compareFn; + + const onDataChangeRef = useRef(onDataChange); + onDataChangeRef.current = onDataChange; + const fetchData = useCallback(async () => { if (!isMountedRef.current) return; try { setIsLoading(true); - const result = await fetchFn(); + const result = await fetchFnRef.current(); if (!isMountedRef.current) return; const hasChanged = previousDataRef.current !== null - ? !compareFn(previousDataRef.current, result) + ? !compareFnRef.current(previousDataRef.current, result) : true; if (hasChanged) { setData(result); previousDataRef.current = result; unchangedCountRef.current = 0; - currentIntervalRef.current = interval; + if (currentIntervalRef.current !== interval) { + currentIntervalRef.current = interval; + if (intervalRef.current && !isBackgroundedRef.current && enabled) { + clearInterval(intervalRef.current); + intervalRef.current = setInterval(fetchData, interval); + } + } dispatch({ type: 'STOP_BACKOFF' }); - onDataChange?.(result); + onDataChangeRef.current?.(result); } else { unchangedCountRef.current += 1; @@ -104,6 +123,10 @@ export function useSmartPolling({ if (newInterval > currentIntervalRef.current) { currentIntervalRef.current = newInterval; dispatch({ type: 'START_BACKOFF' }); + if (intervalRef.current && !isBackgroundedRef.current && enabled) { + clearInterval(intervalRef.current); + intervalRef.current = setInterval(fetchData, newInterval); + } } } } @@ -118,14 +141,32 @@ export function useSmartPolling({ setIsLoading(false); } } - }, [fetchFn, compareFn, interval, backoffMultiplier, maxBackoff, unchangedThreshold, onDataChange]); + }, [interval, backoffMultiplier, maxBackoff, unchangedThreshold, enabled]); const refetch = useCallback(async () => { - currentIntervalRef.current = interval; + if (currentIntervalRef.current !== interval) { + currentIntervalRef.current = interval; + if (intervalRef.current && !isBackgroundedRef.current && enabled) { + clearInterval(intervalRef.current); + intervalRef.current = setInterval(fetchData, interval); + } + } dispatch({ type: 'STOP_BACKOFF' }); unchangedCountRef.current = 0; await fetchData(); - }, [fetchData, interval]); + }, [fetchData, interval, enabled]); + + // Unmount-only cleanup guard (#361) + useEffect(() => { + isMountedRef.current = true; + return () => { + isMountedRef.current = false; + if (intervalRef.current) { + clearInterval(intervalRef.current); + intervalRef.current = null; + } + }; + }, []); // Handle visibility change useEffect(() => { @@ -154,31 +195,36 @@ export function useSmartPolling({ }; }, [enabled, fetchData]); - // Main polling effect - use a ref to track initial mount - const hasInitialized = useRef(false); - + // Main polling effect for enabled state (#570) useEffect(() => { - isMountedRef.current = true; - if (!enabled) { + if (intervalRef.current) { + clearInterval(intervalRef.current); + intervalRef.current = null; + } + if (isMountedRef.current) { + dispatch({ type: 'STOP_POLLING' }); + } + return; + } + + if (isBackgroundedRef.current) { return; } - // Only start polling on initial mount or when enabled changes - if (!hasInitialized.current) { - hasInitialized.current = true; - dispatch({ type: 'START_POLLING' }); - fetchData(); - intervalRef.current = setInterval(fetchData, currentIntervalRef.current); + dispatch({ type: 'START_POLLING' }); + fetchData(); + + if (intervalRef.current) { + clearInterval(intervalRef.current); } + intervalRef.current = setInterval(fetchData, currentIntervalRef.current); return () => { - isMountedRef.current = false; if (intervalRef.current) { clearInterval(intervalRef.current); intervalRef.current = null; } - // Don't dispatch STOP_POLLING here to avoid state updates during unmount }; }, [enabled, fetchData]);