From 307757e734d1adbd5db244c6ae09a807461ce8b4 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Sat, 1 Aug 2026 17:06:30 +0200 Subject: [PATCH] fix(emoji-board): cancel in-flight gif searches and debounce timers --- src/app/components/emoji-board/EmojiBoard.tsx | 17 ++- .../emoji-board/useGifSearch.test.tsx | 134 ++++++++++++++++++ .../components/emoji-board/useGifSearch.ts | 58 ++++++-- src/app/hooks/useDebounce.test.tsx | 29 ++++ src/app/hooks/useDebounce.ts | 25 +++- 5 files changed, 243 insertions(+), 20 deletions(-) create mode 100644 src/app/components/emoji-board/useGifSearch.test.tsx diff --git a/src/app/components/emoji-board/EmojiBoard.tsx b/src/app/components/emoji-board/EmojiBoard.tsx index 6c8d78a044..95fcb23763 100644 --- a/src/app/components/emoji-board/EmojiBoard.tsx +++ b/src/app/components/emoji-board/EmojiBoard.tsx @@ -1,5 +1,5 @@ import type { - ChangeEventHandler, + ChangeEvent, FocusEventHandler, MouseEventHandler, ReactNode, @@ -525,6 +525,7 @@ export function EmojiBoard({ loading: gifsLoading, error: gifsError, searchGifs, + cancelSearch: cancelGifSearch, } = useGifSearch(favoriteGifs, showGifPicker, gifSearch); const [emojiGroupItems, stickerGroupItems, gifGroupItems] = useGroups(tab, imagePacks, gifs); const [showFavoritesOnly, setShowFavoritesOnly] = useState(true); @@ -553,9 +554,9 @@ export function EmojiBoard({ const groups = groupsByTab[tab]; const renderItem = useItemRenderer(tab, saveStickerEmojiBandwidth); - const handleOnChange: ChangeEventHandler = useDebounce( + const handleOnChange = useDebounce( useCallback( - (evt) => { + (evt: ChangeEvent) => { const term = evt.target.value; if (tab === EmojiBoardTab.Gif) { if (term) { @@ -563,6 +564,7 @@ export function EmojiBoard({ searchGifs(term); } else { setShowFavoritesOnly(true); + cancelGifSearch(); resetGifSearch(); } } else if (term) { @@ -571,11 +573,18 @@ export function EmojiBoard({ resetEmojiSearch(); } }, - [emojiSearch, resetEmojiSearch, searchGifs, resetGifSearch, tab] + [cancelGifSearch, emojiSearch, resetEmojiSearch, searchGifs, resetGifSearch, tab] ), { wait: 200 } ); + useEffect(() => { + if (!showGifPicker) { + handleOnChange.cancel(); + cancelGifSearch(); + } + }, [cancelGifSearch, handleOnChange, showGifPicker]); + const contentScrollRef = useRef(null); const virtualBaseRef = useRef(null); const virtualizer = useVirtualizer({ diff --git a/src/app/components/emoji-board/useGifSearch.test.tsx b/src/app/components/emoji-board/useGifSearch.test.tsx new file mode 100644 index 0000000000..26d37a29c0 --- /dev/null +++ b/src/app/components/emoji-board/useGifSearch.test.tsx @@ -0,0 +1,134 @@ +import { act, renderHook } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { useGifSearch } from './useGifSearch'; + +const { fetchMock } = vi.hoisted(() => ({ + fetchMock: vi.fn<(input: string, init?: RequestInit) => Promise>(), +})); + +vi.mock('$utils/fetch', () => ({ fetch: fetchMock })); +vi.mock('$hooks/useClientConfig', () => ({ + useClientConfig: () => ({ gifs: { klipyApiKey: 'test-key' } }), +})); + +type Deferred = { + promise: Promise; + resolve: (value: T) => void; + reject: (reason?: unknown) => void; +}; + +const deferred = (): Deferred => { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + return { promise, resolve, reject }; +}; + +const responseFor = (id: string): Response => + new Response( + JSON.stringify({ + data: { + data: [ + { + id, + title: id, + file: { xs: { gif: { url: `https://${id}.preview` } } }, + }, + ], + }, + }), + { status: 200 } + ); + +const flushPromises = async () => { + await Promise.resolve(); + await Promise.resolve(); +}; + +afterEach(() => { + fetchMock.mockReset(); +}); + +describe('useGifSearch', () => { + it('keeps stale success, error, and finally handlers from changing the latest request', async () => { + const first = deferred(); + const second = deferred(); + fetchMock.mockImplementation((url: string) => + url.includes('q=first') ? first.promise : second.promise + ); + + const { result } = renderHook(() => useGifSearch([], true, vi.fn<() => void>())); + + act(() => { + void result.current.searchGifs('first'); + void result.current.searchGifs('second'); + }); + + const firstSignal = fetchMock.mock.calls[0]?.[1]?.signal as AbortSignal; + expect(firstSignal.aborted).toBe(true); + + first.reject(new Error('stale failure')); + await act(async () => { + await flushPromises(); + }); + expect(result.current.loading).toBe(true); + expect(result.current.error).toBeNull(); + + second.resolve(responseFor('second')); + await act(async () => { + await flushPromises(); + }); + expect(result.current.gifs.gifs[0]?.id).toBe('second'); + expect(result.current.loading).toBe(false); + + first.resolve(responseFor('first')); + await act(async () => { + await flushPromises(); + }); + expect(result.current.gifs.gifs[0]?.id).toBe('second'); + }); + + it('aborts and invalidates a request when cancelled or closed', async () => { + const request = deferred(); + fetchMock.mockReturnValue(request.promise); + + const { result, rerender } = renderHook( + ({ open }) => useGifSearch([], open, vi.fn<() => void>()), + { initialProps: { open: true } } + ); + + act(() => { + void result.current.searchGifs('query'); + }); + const signal = fetchMock.mock.calls[0]?.[1]?.signal as AbortSignal; + + rerender({ open: false }); + + expect(signal.aborted).toBe(true); + expect(result.current.loading).toBe(false); + + request.resolve(responseFor('stale')); + await act(async () => { + await flushPromises(); + }); + expect(result.current.gifs.gifs).toEqual([]); + }); + + it('aborts the active request on unmount', () => { + const request = deferred(); + fetchMock.mockReturnValue(request.promise); + const { result, unmount } = renderHook(() => useGifSearch([], true, vi.fn<() => void>())); + + act(() => { + void result.current.searchGifs('query'); + }); + const signal = fetchMock.mock.calls[0]?.[1]?.signal as AbortSignal; + + unmount(); + + expect(signal.aborted).toBe(true); + }); +}); diff --git a/src/app/components/emoji-board/useGifSearch.ts b/src/app/components/emoji-board/useGifSearch.ts index dc7402a666..0e2cf14213 100644 --- a/src/app/components/emoji-board/useGifSearch.ts +++ b/src/app/components/emoji-board/useGifSearch.ts @@ -1,4 +1,4 @@ -import { useCallback, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import type { AsyncSearchHandler } from '$utils/AsyncSearch'; import { fetch } from '$utils/fetch'; import { useClientConfig } from '$hooks/useClientConfig'; @@ -57,14 +57,34 @@ export function useGifSearch( const [error, setError] = useState(null); const clientConfig = useClientConfig(); const klipyApiKey = clientConfig.gifs?.klipyApiKey ?? ''; + const requestGenerationRef = useRef(0); + const abortControllerRef = useRef(); + const mountedRef = useRef(true); + const showGifPickerRef = useRef(showGifPicker); + showGifPickerRef.current = showGifPicker; + + const cancelRequest = useCallback(() => { + requestGenerationRef.current += 1; + abortControllerRef.current?.abort(); + abortControllerRef.current = undefined; + }, []); + + const cancelSearch = useCallback(() => { + cancelRequest(); + if (mountedRef.current) setLoading(false); + }, [cancelRequest]); const searchGifs = useCallback( async (query: string) => { - if (!showGifPicker) { + if (!mountedRef.current || !showGifPickerRef.current) { return; } const trimmedQuery = query.trim(); + cancelRequest(); + const generation = requestGenerationRef.current; + const controller = new AbortController(); + abortControllerRef.current = controller; setLoading(true); setError(null); @@ -77,30 +97,50 @@ export function useGifSearch( url.searchParams.set('q', trimmedQuery); url.searchParams.set('per_page', '50'); // TODO: infinite scroll? - const response = await fetch(url.toString()); + const response = await fetch(url.toString(), { signal: controller.signal }); if (response.status === 200) { const data = (await response.json()) as KlipySearchResponse; const results = data.data?.data; - setSearchResults(results ? results.map(parseKlipyResult) : []); + if (generation === requestGenerationRef.current && mountedRef.current) { + setSearchResults(results ? results.map(parseKlipyResult) : []); + } } else { throw new Error(`HTTP ${response.status}`); } } catch { - setError('Failed to search GIFs'); - setSearchResults([]); + if (generation === requestGenerationRef.current && mountedRef.current) { + setError('Failed to search GIFs'); + setSearchResults([]); + } } finally { - setLoading(false); + if (generation === requestGenerationRef.current && mountedRef.current) { + abortControllerRef.current = undefined; + setLoading(false); + } } }, - [klipyApiKey, showGifPicker, gifSearch] + [cancelRequest, klipyApiKey, gifSearch] ); + useEffect(() => { + mountedRef.current = true; + + return () => { + mountedRef.current = false; + cancelRequest(); + }; + }, [cancelRequest]); + + useEffect(() => { + if (!showGifPicker) cancelSearch(); + }, [cancelSearch, showGifPicker]); + const gifs = useMemo( () => ({ gifs: searchResults, favorites: favoriteGifs }), [searchResults, favoriteGifs] ); - return { gifs, loading, error, searchGifs }; + return { gifs, loading, error, searchGifs, cancelSearch }; } diff --git a/src/app/hooks/useDebounce.test.tsx b/src/app/hooks/useDebounce.test.tsx index 0c28edea5f..1de88db558 100644 --- a/src/app/hooks/useDebounce.test.tsx +++ b/src/app/hooks/useDebounce.test.tsx @@ -97,4 +97,33 @@ describe('useDebounce', () => { expect(fn).toHaveBeenCalledOnce(); expect(fn).toHaveBeenCalledWith('go'); }); + + it('cancels a pending callback explicitly', () => { + const fn = vi.fn<(arg: string) => void>(); + const { result } = renderHook(() => useDebounce(fn, { wait: 200 })); + + act(() => { + result.current('cancelled'); + result.current.cancel(); + vi.advanceTimersByTime(200); + }); + + expect(fn).not.toHaveBeenCalled(); + }); + + it('cancels a pending callback when unmounted', () => { + const fn = vi.fn<(arg: string) => void>(); + const { result, unmount } = renderHook(() => useDebounce(fn, { wait: 200 })); + + act(() => { + result.current('cancelled'); + }); + unmount(); + + act(() => { + vi.advanceTimersByTime(200); + }); + + expect(fn).not.toHaveBeenCalled(); + }); }); diff --git a/src/app/hooks/useDebounce.ts b/src/app/hooks/useDebounce.ts index 5f33976a3e..018defe79d 100644 --- a/src/app/hooks/useDebounce.ts +++ b/src/app/hooks/useDebounce.ts @@ -1,23 +1,32 @@ -import { useCallback, useRef } from 'react'; +import { useCallback, useEffect, useMemo, useRef } from 'react'; export interface DebounceOptions { wait?: number; immediate?: boolean; } export type DebounceCallback = (...args: T) => void; +export type DebouncedCallback = DebounceCallback & { + cancel: () => void; +}; export function useDebounce( callback: DebounceCallback, options?: DebounceOptions -): DebounceCallback { +): DebouncedCallback { const timeoutIdRef = useRef(); const { wait, immediate } = options ?? {}; + const cancel = useCallback(() => { + if (timeoutIdRef.current !== undefined) { + clearTimeout(timeoutIdRef.current); + timeoutIdRef.current = undefined; + } + }, []); + const debounceCallback = useCallback( (...cbArgs: T) => { - if (timeoutIdRef.current) { - clearTimeout(timeoutIdRef.current); - timeoutIdRef.current = undefined; + if (timeoutIdRef.current !== undefined) { + cancel(); } else if (immediate) { callback(...cbArgs); } @@ -27,8 +36,10 @@ export function useDebounce( timeoutIdRef.current = undefined; }, wait); }, - [callback, wait, immediate] + [callback, cancel, wait, immediate] ); - return debounceCallback; + useEffect(() => cancel, [cancel]); + + return useMemo(() => Object.assign(debounceCallback, { cancel }), [debounceCallback, cancel]); }