refactor(frontend): extract debounced filter emit into useDebouncedCallback

The previous iteration moved DataTableFilter / InputSearch off
`useDebouncedValue` and onto an inline `setTimeout` + `pendingTimerRef`
+ `cancelPendingEmit` dance. That fixed all four race scenarios but
left imperative scheduling living inside components — a code smell
flagged in review. After surveying the React 19.2 surface area
(`useDeferredValue` is for *render* priority, not side-effect throttling;
`useTransition` doesn't apply to controlled inputs; `useEffectEvent` is
restricted to handlers called from within Effects) and `use-debounce`'s
API, the right shape is a small, dependency-free hook.

`useDebouncedCallback(fn, delayMs)` returns a stable callable with
`.cancel()` / `.isPending()`. Timer state lives in a closure variable
inside a `useState` lazy initialiser, so:
- the returned identity never changes (safe in effect deps and memoised
  children);
- there are no refs to read during render (no React Compiler complaints);
- `fn` and `delayMs` go through `useLatestRef` so the dispatched call
  always sees the freshest closure — inline arrows in parents are free.

DataTableFilter / InputSearch shrink to one useState + one useRef
(`lastEmittedReference` distinguishes our own router round-trip from a
true external change) + one useEffect (external sync). All four
regression tests added in the previous commit still pass; five new unit
tests pin `useDebouncedCallback` semantics (burst coalescing, cancel,
isPending, latest-closure read, unmount cleanup). Browser repro on
/flows for both X-after-flush and X-before-flush scenarios: a single
transition `v="" u=""`, no resurrection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Sergey Kozyrenko
2026-05-23 00:48:10 +07:00
co-authored by Claude Opus 4.7
parent ba354f3701
commit e451d23aab
4 changed files with 279 additions and 120 deletions
+43 -71
View File
@@ -54,6 +54,7 @@ import { Empty, EmptyDescription, EmptyHeader, EmptyMedia, EmptyTitle } from '@/
import { InputGroup, InputGroupAddon, InputGroupButton, InputGroupInput } from '@/components/ui/input-group';
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select';
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/table';
import { useDebouncedCallback } from '@/hooks/use-debounced-callback';
import { useEffectAfterMount } from '@/hooks/use-effect-after-mount';
import { useLatestRef } from '@/hooks/use-latest-ref';
import { usePageStorageKeys } from '@/hooks/use-page-storage-keys';
@@ -934,110 +935,81 @@ function DataTableColumnHeader<TData, TValue = unknown>({ column, title }: DataT
}
/**
* Search input for the table's global filter. Two storage cells, one
* outbound path:
* Search input for the table's global filter.
*
* 1. `localValue` — `useState`, drives the controlled input. Updated
* Controlled debounced input — the kind the React docs warn you cannot
* model with `useDeferredValue` alone, because we want to throttle the
* *side-effect* (URL writes via the parent's `onQueryChange`) rather than
* the render of the filtered rows (`useDeferredValue` already handles the
* latter at the table level — see `deferredGlobalFilter` above). React's
* own recommendation for this case is plain debouncing.
*
* The whole component is two state cells and one effect:
*
* 1. `localValue` (`useState`) — drives the controlled input, updates
* synchronously on every keystroke so the caret never lags.
* 2. `pendingTimerRef` — the *only* pending emit. `handleChange` schedules
* it; everything else (`handleClear`, external `query` change, unmount)
* cancels it synchronously before doing its own thing.
* 2. `lastEmittedReference` (`useRef`) — records the most recent value
* we've handed upstream so the external-sync effect can distinguish
* "this is our own commit coming back through the router" (skip)
* from "something outside changed the source of truth" (accept,
* override any in-flight typing).
* 3. One `useEffect` reconciling external `query` with local state.
*
* The earlier design layered `useDebouncedValue` (its own delayed `useState`)
* plus a `lastEmittedReference` ref and two opposing `useEffect`s on top of
* the same value. That introduced a third storage cell (`debouncedValue`)
* which lagged behind `localValue` by a `setTimeout` and could be read out
* of phase: a synchronous clear ran while `debouncedValue` still held the
* pre-clear text, so the emit effect re-fired and resurrected it. Moving
* the debounce into an imperative timer eliminates that hidden cell —
* there is no "stale debounced value" to read from any effect.
*
* Why we debounce at all: previously every keystroke synchronously walked
* `useTableQueryFilter` → router → route subtree, ~250 ms per keystroke on
* Flows. The debounce drops upstream commits to once per typing pause.
* Everything timer-related lives inside `useDebouncedCallback`, which
* exposes a single `.cancel()` we call from the only two places that need
* to override the schedule — external sync and `handleClear`. There is no
* stale `debouncedValue` cell for an effect to read out of phase, which
* was the entire class of races the previous design carried.
*/
function DataTableFilter({ onQueryChange, placeholder, query }: DataTableFilterProps) {
const [localValue, setLocalValue] = useState(query);
// Tracks the last value we successfully emitted upstream so the
// external-sync effect can tell "the parent confirmed our own commit"
// (skip) from "the parent changed `query` independently — back button,
// shared link, sibling control" (accept and override local edits).
// Mutated only inside `emit`, so the invariant is local to one function.
const lastEmittedReference = useRef(query);
const pendingTimerReference = useRef<null | number>(null);
// The handler from the parent is typically an inline arrow, so its
// identity churns every render. We never want effect deps to depend on
// that — read the latest one through this ref instead.
const onQueryChangeReference = useLatestRef(onQueryChange);
// Generated per-instance so pages with multiple DataTables (e.g.
// /settings/prompts) don't end up with duplicate `id` attributes — that
// breaks `getElementById`, a11y semantics, and any test selector that
// relies on the input id.
const fieldId = useId();
const cancelPendingEmit = useCallback(() => {
if (pendingTimerReference.current !== null) {
window.clearTimeout(pendingTimerReference.current);
pendingTimerReference.current = null;
const debouncedEmit = useDebouncedCallback((next: string) => {
if (next === lastEmittedReference.current) {
return;
}
}, []);
// Single outbound path. Always cancels any pending debounce so the
// caller's value wins; skips no-ops where the parent already holds the
// same string. Records what went out so external-sync can recognise
// the round-trip.
const emit = useCallback(
(next: string) => {
cancelPendingEmit();
if (next === lastEmittedReference.current) {
return;
}
lastEmittedReference.current = next;
onQueryChangeReference.current(next);
},
[cancelPendingEmit, onQueryChangeReference],
);
lastEmittedReference.current = next;
onQueryChange(next);
}, FILTER_DEBOUNCE_MS);
// External → local sync. Runs only when `query` is something we didn't
// emit ourselves — that's the signal an outside source (back button,
// sibling control, programmatic clear) moved the source of truth out
// from under us. Any in-flight debounce of locally-typed text is dropped
// emit ourselves: back button, programmatic clear, sibling control.
// Any in-flight debounce of locally-typed text is cancelled
// synchronously; the external value wins, by design.
useEffect(() => {
if (query === lastEmittedReference.current) {
return;
}
cancelPendingEmit();
debouncedEmit.cancel();
lastEmittedReference.current = query;
setLocalValue(query);
}, [query, cancelPendingEmit]);
// Cancel any pending timer on unmount so it can't fire into a dead
// component (React would warn, and the upstream parent might still
// accept the emit).
useEffect(() => cancelPendingEmit, [cancelPendingEmit]);
}, [query, debouncedEmit]);
const handleChange = useCallback(
(event: ChangeEvent<HTMLInputElement>) => {
const next = event.target.value;
setLocalValue(next);
cancelPendingEmit();
pendingTimerReference.current = window.setTimeout(() => {
pendingTimerReference.current = null;
emit(next);
}, FILTER_DEBOUNCE_MS);
setLocalValue(event.target.value);
debouncedEmit(event.target.value);
},
[cancelPendingEmit, emit],
[debouncedEmit],
);
const handleClear = useCallback(() => {
setLocalValue('');
emit('');
}, [emit]);
debouncedEmit.cancel();
if (lastEmittedReference.current !== '') {
lastEmittedReference.current = '';
onQueryChange('');
}
}, [debouncedEmit, onQueryChange]);
return (
<InputGroup className="max-w-sm">
+30 -49
View File
@@ -5,14 +5,14 @@ import { type ChangeEvent, useCallback, useEffect, useMemo, useRef, useState } f
import { InputGroup, InputGroupAddon, InputGroupButton, InputGroupInput } from '@/components/ui/input-group';
import { Kbd, KbdGroup } from '@/components/ui/kbd';
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip';
import { useLatestRef } from '@/hooks/use-latest-ref';
import { useDebouncedCallback } from '@/hooks/use-debounced-callback';
import { cn } from '@/lib/utils';
import { isMac } from '@/lib/utils/platform';
// Keystrokes hit `localValue` synchronously for an instant UI response; the
// outbound emit goes through a timer ref (see `pendingTimerReference`). Same
// shape as `DataTableFilter`; mirrors `FILTER_DEBOUNCE_MS` so both search
// inputs on a page share one "feels responsive" target.
// upstream emit is debounced via `useDebouncedCallback` (see below). Same
// shape and timing as `DataTableFilter` so both search inputs on a page
// share one "feels responsive" target.
const COMMIT_DEBOUNCE_MS = 150;
interface InputSearchProps {
@@ -67,53 +67,36 @@ export function InputSearch({
const inputRef = useRef<HTMLInputElement>(null);
// Same two-cell shape as `DataTableFilter`: `localValue` drives the input
// (synchronous, instant caret response); `pendingTimerReference` is the
// only outbound emit in flight. `lastEmittedReference` records the most
// recent value we've handed upstream so the external-sync effect can
// distinguish our own round-trip from a true external change.
// (synchronous, instant caret response); `lastEmittedReference` records
// the most recent value we handed upstream so the external-sync effect
// can distinguish our own round-trip from a true external change. All
// timer logic lives inside `useDebouncedCallback` — see its docstring
// for the rationale.
const [localValue, setLocalValue] = useState(searchQuery);
const lastEmittedReference = useRef(searchQuery);
const pendingTimerReference = useRef<null | number>(null);
const onSearchChangeReference = useLatestRef(onSearchChange);
const cancelPendingEmit = useCallback(() => {
if (pendingTimerReference.current !== null) {
window.clearTimeout(pendingTimerReference.current);
pendingTimerReference.current = null;
const debouncedEmit = useDebouncedCallback((next: string) => {
if (next === lastEmittedReference.current) {
return;
}
}, []);
const emit = useCallback(
(next: string) => {
cancelPendingEmit();
if (next === lastEmittedReference.current) {
return;
}
lastEmittedReference.current = next;
onSearchChangeReference.current(next);
},
[cancelPendingEmit, onSearchChangeReference],
);
lastEmittedReference.current = next;
onSearchChange(next);
}, COMMIT_DEBOUNCE_MS);
// External → local sync. Runs only when `searchQuery` is something we
// didn't emit ourselves — programmatic wipe of `?qs=`, back button,
// sibling control. Any in-flight debounce of locally-typed text is
// dropped synchronously; the external value wins, by design.
// cancelled synchronously; the external value wins, by design.
useEffect(() => {
if (searchQuery === lastEmittedReference.current) {
return;
}
cancelPendingEmit();
debouncedEmit.cancel();
lastEmittedReference.current = searchQuery;
setLocalValue(searchQuery);
}, [searchQuery, cancelPendingEmit]);
// Cancel any pending timer on unmount so it can't fire into a dead
// component.
useEffect(() => cancelPendingEmit, [cancelPendingEmit]);
}, [searchQuery, debouncedEmit]);
const expand = useCallback(() => setIsExpanded(true), []);
@@ -210,22 +193,15 @@ export function InputSearch({
const handleChange = useCallback(
(event: ChangeEvent<HTMLInputElement>) => {
const next = event.target.value;
setLocalValue(next);
cancelPendingEmit();
pendingTimerReference.current = window.setTimeout(() => {
pendingTimerReference.current = null;
emit(next);
}, COMMIT_DEBOUNCE_MS);
setLocalValue(event.target.value);
debouncedEmit(event.target.value);
},
[cancelPendingEmit, emit],
[debouncedEmit],
);
// Escape: first press clears the value, second press collapses + blurs.
// Clear goes through `emit('')`, which cancels any pending debounce
// synchronously — no chance for an in-flight typed value to land after
// the Esc.
// The clear path cancels any pending debounce synchronously, then emits
// `''` directly — no chance for an in-flight typed value to land after.
const handleInputKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLInputElement>) => {
if (event.key !== 'Escape') {
@@ -236,7 +212,12 @@ export function InputSearch({
if (localValue.length > 0) {
setLocalValue('');
emit('');
debouncedEmit.cancel();
if (lastEmittedReference.current !== '') {
lastEmittedReference.current = '';
onSearchChange('');
}
return;
}
@@ -244,7 +225,7 @@ export function InputSearch({
inputRef.current?.blur();
setIsExpanded(false);
},
[emit, localValue],
[debouncedEmit, localValue, onSearchChange],
);
return (
@@ -0,0 +1,111 @@
import { act, renderHook } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { useDebouncedCallback } from './use-debounced-callback';
describe('useDebouncedCallback', () => {
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it('defers invocation until delayMs of silence', () => {
const fn = vi.fn();
const { result } = renderHook(() => useDebouncedCallback(fn, 100));
act(() => {
result.current('a');
result.current('b');
result.current('c');
});
expect(fn).not.toHaveBeenCalled();
act(() => {
vi.advanceTimersByTime(100);
});
// Only the latest call lands; intermediate ones are dropped.
expect(fn).toHaveBeenCalledExactlyOnceWith('c');
});
it('cancel() drops the pending dispatch', () => {
const fn = vi.fn();
const { result } = renderHook(() => useDebouncedCallback(fn, 100));
act(() => {
result.current('x');
result.current.cancel();
vi.advanceTimersByTime(500);
});
expect(fn).not.toHaveBeenCalled();
});
it('isPending() flips with the timer', () => {
const fn = vi.fn();
const { result } = renderHook(() => useDebouncedCallback(fn, 100));
expect(result.current.isPending()).toBe(false);
act(() => {
result.current('a');
});
expect(result.current.isPending()).toBe(true);
act(() => {
vi.advanceTimersByTime(100);
});
expect(result.current.isPending()).toBe(false);
});
it('reads the latest `fn` closure at fire time, not at schedule time', () => {
// Mirrors the production use case: a stale fn would close over an
// old prop. The debounced callback identity stays stable across
// renders, but the dispatched fn must be the freshest one.
const calls: string[] = [];
const { rerender, result } = renderHook(({ tag }: { tag: string }) => useDebouncedCallback(() => calls.push(tag), 100), {
initialProps: { tag: 'old' },
});
const initialDebounced = result.current;
act(() => {
result.current();
});
rerender({ tag: 'new' });
// Same callback identity across renders — caller can pass it to
// memoised children or list it in effect deps without churn.
expect(result.current).toBe(initialDebounced);
act(() => {
vi.advanceTimersByTime(100);
});
expect(calls).toEqual(['new']);
});
it('cancels pending dispatch on unmount so dead components never fire', () => {
const fn = vi.fn();
const { result, unmount } = renderHook(() => useDebouncedCallback(fn, 100));
act(() => {
result.current('a');
});
unmount();
act(() => {
vi.advanceTimersByTime(500);
});
expect(fn).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,95 @@
import { useEffect, useState } from 'react';
import { useLatestRef } from './use-latest-ref';
/**
* A function that defers invoking `fn` until `delayMs` have elapsed since the
* last call. The returned function is stable across renders and exposes
* imperative escape hatches for callers that need to override the schedule.
*/
export interface DebouncedCallback<Args extends unknown[]> {
(...args: Args): void;
/**
* Drop any pending call. After this, the next invocation starts a fresh
* timer. Idempotent — safe to call from `handleClear`, route transitions,
* external-sync effects, or anywhere else that needs to override the
* schedule.
*/
cancel: () => void;
/**
* Whether a call is currently scheduled but not yet dispatched. Useful
* for tests; rarely needed in product code.
*/
isPending: () => boolean;
}
/**
* Same idea as `lodash.debounce` / `use-debounce`, but small and
* dependency-free: returns a stable callback that defers invoking `fn` until
* `delayMs` have elapsed since the most recent call. New calls reset the
* timer; the most recent arguments are the ones eventually dispatched.
*
* Why a hook (instead of inlining `setTimeout`):
* - `cancel()` is a single, synchronous call site — usable from `handleClear`,
* external-sync effects, or anywhere else that needs to override the
* pending dispatch. There is no opportunity for a stale timer to fire
* after the cancel, which was the entire class of races in our previous
* filter implementation.
* - The returned function identity is stable across renders, so it can be
* passed to memoised children and listed in effect deps without churn.
* - `fn` is captured in a latest-ref, so the dispatched call always sees the
* most recent closure (props/state at fire time, not at schedule time).
* This is the standard React idiom for stashing an event-like handler.
* - Unmount cleanup is automatic — `useEffect` cancels any pending timer
* so dispatched functions can't land in a torn-down tree.
*
* Tradeoff vs `useDeferredValue`: `useDeferredValue` is React's preferred
* tool for deferring *rendering* work (a memoised list re-renders later
* while the input stays snappy). It does not defer side-effects. Reach for
* `useDebouncedCallback` whenever you need to throttle imperative work that
* leaves React's render tree — URL writes, network requests, persistence —
* and `useDeferredValue` for the render pipeline itself.
*
* Implementation note: the entire API object is built once inside a
* `useState` lazy initialiser, with the timer handle living in a *closure
* variable* (`timerId`) rather than a React `useRef`. This keeps the
* returned `debounced` reference stable forever and side-steps the React
* Compiler rule that flags reading refs during render — there are no refs
* to read, only a private variable closed over by the three methods.
*/
export function useDebouncedCallback<Args extends unknown[]>(
fn: (...args: Args) => void,
delayMs: number,
): DebouncedCallback<Args> {
const fnReference = useLatestRef(fn);
const delayReference = useLatestRef(delayMs);
const [debounced] = useState<DebouncedCallback<Args>>(() => {
let timerId: null | number = null;
const cancel = () => {
if (timerId !== null) {
window.clearTimeout(timerId);
timerId = null;
}
};
const isPending = () => timerId !== null;
const invoker = (...args: Args) => {
cancel();
timerId = window.setTimeout(() => {
timerId = null;
fnReference.current(...args);
}, delayReference.current);
};
return Object.assign(invoker, { cancel, isPending });
});
// Cancel any pending dispatch when the host component unmounts so it
// can't fire into a torn-down tree.
useEffect(() => debounced.cancel, [debounced]);
return debounced;
}