diff --git a/src/hooks/useSmartPolling.test.ts b/src/hooks/useSmartPolling.test.ts new file mode 100644 index 0000000..44c4735 --- /dev/null +++ b/src/hooks/useSmartPolling.test.ts @@ -0,0 +1,89 @@ +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(); + }); + + expect(result.current.isPolling).toBe(true); + expect(fetchFn.mock.calls.length).toBeGreaterThan(callsBeforeReenable); + + unmount(); + }); +}); diff --git a/src/hooks/useSmartPolling.ts b/src/hooks/useSmartPolling.ts index 81507fc..779c670 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,17 +77,26 @@ 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) { @@ -92,7 +105,7 @@ export function useSmartPolling({ unchangedCountRef.current = 0; currentIntervalRef.current = interval; dispatch({ type: 'STOP_BACKOFF' }); - onDataChange?.(result); + onDataChangeRef.current?.(result); } else { unchangedCountRef.current += 1; @@ -118,7 +131,7 @@ export function useSmartPolling({ setIsLoading(false); } } - }, [fetchFn, compareFn, interval, backoffMultiplier, maxBackoff, unchangedThreshold, onDataChange]); + }, [interval, backoffMultiplier, maxBackoff, unchangedThreshold]); const refetch = useCallback(async () => { currentIntervalRef.current = interval; @@ -127,6 +140,18 @@ export function useSmartPolling({ await fetchData(); }, [fetchData, interval]); + // 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(() => { const handleVisibilityChange = () => { @@ -154,31 +179,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; } - // 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); + if (isBackgroundedRef.current) { + return; } + 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]);