Skip to content

Commit 9043739

Browse files
committed
SEP-1779: Lift the duplicated search debounce into @sep/framework
The Snippet Manager list and the ATW Collect pane's snippet picker each carried the same debounce: a `useEffect` opening a `setTimeout`, clearing it on cleanup, against its own locally-declared 300ms constant. Two copies means a change to the window has to be found and applied twice, and the second copy is the one that gets missed. `useDebouncedValue(value, delayMs)` and the shared `SEARCH_DEBOUNCE_MS` now live in `@sep/framework`, and both consumers call it. The primitive seeds its state from the incoming value rather than from a blank, so a list mounted with a term already in state queries for that term instead of fetching the unfiltered page first; both consumers mount with an empty box today, so nothing user-visible moves. Callers trim at the call site, which is where the old effects trimmed too, only inside the timer. Behaviour is otherwise unchanged: same window, same one-publish-per-pause coalescing, same clear on unmount. `SnippetsListPage.test.tsx` moves from a full `vi.mock('@sep/framework')` to an `importOriginal` partial mock so the page exercises the real primitive; only the download hook stays stubbed. The ATW suite needed no change. Scope note: the ticket also asked for a shared snippets-search query hook. SEP-1821 (#1349) has since moved ATW's search onto ATW's own router, dropping the `approval` param and the row projection, and `useSnippets` has gained a `sort` param, so the two hooks no longer share an endpoint, a param surface, or a mapper. The only remaining overlap is `apiClient.get` -> `normalizeAppListResponse` -> `keepPreviousData`, which `useTasksList` shares equally: a generic app-list wrapper, not a snippets-search one. Left for a follow-up rather than built here.
1 parent ff2e543 commit 9043739

7 files changed

Lines changed: 188 additions & 24 deletions

File tree

frontend/packages/apps/atw/src/CollectPane.tsx

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@
1717

1818
import { useCallback, useEffect, useMemo, useState } from 'react';
1919
import { Alert, Autocomplete, Box, CircularProgress, TextField, Typography } from '@mui/material';
20-
import { SchemaFormRenderer, SNIPPET_FORM_RESERVED_FIELD_NAMES } from '@sep/framework';
20+
import {
21+
SchemaFormRenderer,
22+
SNIPPET_FORM_RESERVED_FIELD_NAMES,
23+
useDebouncedValue,
24+
} from '@sep/framework';
2125
import type { FormSection, SectionField } from '@sep/api';
2226
import { CategoryBrowser } from './CategoryBrowser';
2327
import { useAtwBatchExecute, useAtwMergedSchema, useAtwSnippetSearch } from './hooks';
@@ -28,9 +32,6 @@ export interface CollectPaneProps {
2832
isClosed?: boolean;
2933
}
3034

31-
/** Pause after the last keystroke before the snippet search fires (ms). */
32-
const SNIPPET_SEARCH_DEBOUNCE_MS = 300;
33-
3435
/** Stable empty list so an idle search does not churn the options memo. */
3536
const NO_SNIPPETS: AtwSnippetSummary[] = [];
3637

@@ -194,21 +195,13 @@ export function CollectPane({ incidentId, isClosed = false }: CollectPaneProps)
194195
const [selected, setSelected] = useState<AtwSnippetSummary[]>([]);
195196
const [itemErrors, setItemErrors] = useState<string[]>([]);
196197
const [searchInput, setSearchInput] = useState('');
197-
const [debouncedSearch, setDebouncedSearch] = useState('');
198+
// Shares the Snippet Manager list's search-debounce window.
199+
const debouncedSearch = useDebouncedValue(searchInput.trim());
198200

199201
const handleSnippetsChange = useCallback((snippets: AtwSnippetSummary[]) => {
200202
setAvailable(snippets);
201203
}, []);
202204

203-
// Matches the Snippet Manager list's 300ms window.
204-
useEffect(() => {
205-
const handle = setTimeout(
206-
() => setDebouncedSearch(searchInput.trim()),
207-
SNIPPET_SEARCH_DEBOUNCE_MS,
208-
);
209-
return () => clearTimeout(handle);
210-
}, [searchInput]);
211-
212205
useEffect(() => {
213206
if (!isClosed) {
214207
return;

frontend/packages/apps/snippets/src/SnippetsListPage.test.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ vi.mock('react-router', () => ({
3636
// The download button owns a mutation now (the SPA fetches the file through the
3737
// Bearer-authenticated JSON endpoint), so the download-before-approve guard
3838
// flips on the mutation's success callback rather than on a plain anchor click.
39-
vi.mock('@sep/framework', () => ({
39+
// Partial mock: only the download hook is stubbed. `useDebouncedValue` is a
40+
// pure timer primitive with no transport of its own, so the page exercises the
41+
// real one — the same 300ms window it debounced inline before.
42+
vi.mock('@sep/framework', async (importOriginal) => ({
43+
...(await importOriginal<typeof import('@sep/framework')>()),
4044
useSnippetDownload: vi.fn(() => ({
4145
mutate: (_params: unknown, callbacks?: { onSuccess?: () => void }) => callbacks?.onSuccess?.(),
4246
isPending: false,

frontend/packages/apps/snippets/src/SnippetsListPage.tsx

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import DownloadIcon from '@mui/icons-material/Download';
4949
import RemoveCircleOutlineIcon from '@mui/icons-material/RemoveCircleOutline';
5050
import SearchIcon from '@mui/icons-material/Search';
5151
import { ApiError, DEFAULT_APP_LIST_LIMIT, DEFAULT_APP_LIST_OFFSET } from '@sep/api';
52-
import { useSnippetDownload } from '@sep/framework';
52+
import { useDebouncedValue, useSnippetDownload } from '@sep/framework';
5353
import {
5454
useSnippets,
5555
useApproveSnippet,
@@ -99,9 +99,6 @@ function decodeServiceType(value: string): string {
9999
return value.slice(SERVICE_TYPE_PREFIX.length);
100100
}
101101

102-
/** Debounce window (ms) before a search keystroke drives a server refetch. */
103-
const SEARCH_DEBOUNCE_MS = 300;
104-
105102
/** Label shown for snippets that declare no `service_type`. */
106103
const UNCATEGORIZED_LABEL = 'Uncategorized';
107104

@@ -241,11 +238,7 @@ export function SnippetsListPage({ isAdmin = false }: SnippetsListPageProps) {
241238
const [serviceTypeFilter, setServiceTypeFilter] = useState<string>(ALL_SERVICES);
242239

243240
// Debounce the search box so typing drives one refetch per pause, not per key.
244-
const [debouncedSearch, setDebouncedSearch] = useState('');
245-
useEffect(() => {
246-
const handle = setTimeout(() => setDebouncedSearch(search.trim()), SEARCH_DEBOUNCE_MS);
247-
return () => clearTimeout(handle);
248-
}, [search]);
241+
const debouncedSearch = useDebouncedValue(search.trim());
249242

250243
// Map the service-type UI selection onto the server equality param: no filter,
251244
// "uncategorized" (carried by a separate flag below), or the decoded free-form

frontend/packages/framework/src/hooks/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,6 @@ export type { UseSnippetAppExecutionOptions } from './useSnippetAppExecution';
8686
export { useTaskStats } from './useTaskStats';
8787
export type { TaskStatsView } from './useTaskStats';
8888

89+
export { useDebouncedValue, SEARCH_DEBOUNCE_MS } from './useDebouncedValue';
90+
8991
export { sepRetry } from './sepRetry';
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* Copyright (C) 2026 Percona LLC
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the GNU Affero General Public License as published by
6+
* the Free Software Foundation, either version 3 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU Affero General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU Affero General Public License
15+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
16+
*/
17+
18+
import { act, renderHook } from '@testing-library/react';
19+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
20+
21+
import { SEARCH_DEBOUNCE_MS, useDebouncedValue } from './useDebouncedValue';
22+
23+
beforeEach(() => {
24+
vi.useFakeTimers();
25+
});
26+
27+
afterEach(() => {
28+
vi.useRealTimers();
29+
});
30+
31+
/** Advance timers inside `act` so the resulting state update is flushed. */
32+
function advance(ms: number) {
33+
act(() => {
34+
vi.advanceTimersByTime(ms);
35+
});
36+
}
37+
38+
describe('useDebouncedValue', () => {
39+
it('publishes the initial value immediately', () => {
40+
const { result } = renderHook(() => useDebouncedValue('seed'));
41+
42+
expect(result.current).toBe('seed');
43+
});
44+
45+
it('withholds a new value until the delay elapses', () => {
46+
const { result, rerender } = renderHook(({ value }) => useDebouncedValue(value), {
47+
initialProps: { value: '' },
48+
});
49+
50+
rerender({ value: 'pg' });
51+
expect(result.current).toBe('');
52+
53+
advance(SEARCH_DEBOUNCE_MS - 1);
54+
expect(result.current).toBe('');
55+
56+
advance(1);
57+
expect(result.current).toBe('pg');
58+
});
59+
60+
it('publishes once per pause, not once per change', () => {
61+
const { result, rerender } = renderHook(({ value }) => useDebouncedValue(value), {
62+
initialProps: { value: '' },
63+
});
64+
65+
for (const value of ['p', 'pg', 'pgs', 'pgst']) {
66+
rerender({ value });
67+
advance(SEARCH_DEBOUNCE_MS - 50);
68+
}
69+
// Every keystroke landed inside the window, so nothing has been published.
70+
expect(result.current).toBe('');
71+
72+
advance(50);
73+
expect(result.current).toBe('pgst');
74+
});
75+
76+
it('honours a caller-supplied delay', () => {
77+
const { result, rerender } = renderHook(({ value }) => useDebouncedValue(value, 1000), {
78+
initialProps: { value: '' },
79+
});
80+
81+
rerender({ value: 'slow' });
82+
advance(SEARCH_DEBOUNCE_MS);
83+
expect(result.current).toBe('');
84+
85+
advance(1000 - SEARCH_DEBOUNCE_MS);
86+
expect(result.current).toBe('slow');
87+
});
88+
89+
it('restarts the window when the delay itself changes', () => {
90+
const { result, rerender } = renderHook(({ value, delay }) => useDebouncedValue(value, delay), {
91+
initialProps: { value: 'a', delay: SEARCH_DEBOUNCE_MS },
92+
});
93+
94+
rerender({ value: 'b', delay: SEARCH_DEBOUNCE_MS });
95+
advance(SEARCH_DEBOUNCE_MS - 100);
96+
97+
rerender({ value: 'b', delay: 1000 });
98+
advance(100);
99+
expect(result.current).toBe('a');
100+
101+
advance(900);
102+
expect(result.current).toBe('b');
103+
});
104+
105+
it('clears the pending timer on unmount', () => {
106+
const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout');
107+
const { rerender, unmount } = renderHook(({ value }) => useDebouncedValue(value), {
108+
initialProps: { value: '' },
109+
});
110+
111+
rerender({ value: 'gone' });
112+
clearTimeoutSpy.mockClear();
113+
unmount();
114+
115+
expect(clearTimeoutSpy).toHaveBeenCalledTimes(1);
116+
117+
// The timer never fires, so no state update is attempted after unmount.
118+
expect(() => advance(SEARCH_DEBOUNCE_MS)).not.toThrow();
119+
clearTimeoutSpy.mockRestore();
120+
});
121+
});
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* Copyright (C) 2026 Percona LLC
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the GNU Affero General Public License as published by
6+
* the Free Software Foundation, either version 3 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU Affero General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU Affero General Public License
15+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
16+
*/
17+
18+
import { useEffect, useState } from 'react';
19+
20+
/**
21+
* Default settle window for a typed input before it drives work downstream (ms).
22+
*
23+
* Named for its origin — the search boxes across the apps, where the work is a
24+
* server refetch — and shared so they all settle on one window rather than each
25+
* redeclaring it. It is the hook's default rather than its only value: a
26+
* consumer wanting a different pause states that delay at the call site.
27+
*/
28+
export const SEARCH_DEBOUNCE_MS = 300;
29+
30+
/**
31+
* Track `value`, but only publish it once it has held still for `delayMs`.
32+
*
33+
* The debounced value starts at the initial `value` rather than at a blank —
34+
* a list mounted with a search term already in its state queries for that term
35+
* immediately instead of fetching the unfiltered page first.
36+
*
37+
* The pending timer is cleared on every change and on unmount, so a fast typist
38+
* causes one publish per pause, and an unmounted component never publishes.
39+
*/
40+
export function useDebouncedValue<T>(value: T, delayMs: number = SEARCH_DEBOUNCE_MS): T {
41+
const [debounced, setDebounced] = useState<T>(value);
42+
43+
useEffect(() => {
44+
const handle = setTimeout(() => setDebounced(value), delayMs);
45+
return () => clearTimeout(handle);
46+
}, [value, delayMs]);
47+
48+
return debounced;
49+
}

frontend/packages/framework/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ export {
152152
useTaskHistoryFiles,
153153
useTaskFileDownload,
154154
useSnippetDownload,
155+
useDebouncedValue,
156+
SEARCH_DEBOUNCE_MS,
155157
} from './hooks';
156158
export type {
157159
TaskLogsState,

0 commit comments

Comments
 (0)