fix(sidebar): cancel pending debounce emit on external value reset (#1244)

SidebarSearch's value-sync effect adopted external resets but left the
pending setTimeout in place. When a clear or filter-driven reset arrived
inside the 120ms window, the stale timer would fire after the adopt and
emit the previously-typed query back to the parent, silently undoing the
reset. The skip condition also leaned on lastEmittedRef, which kept a
genuine reset from winning if its value happened to equal the last emit.

Switch the skip to compare the parent value against the locally shown
value (tracked through a ref so the effect deps stay on [value]). On any
external transition the effect now clears the pending timer before
adopting, removing the race entirely. lastEmittedRef is dead under this
model and is removed.

Adds a fake-timer test that types mid-window, rerenders with a different
value before the debounce fires, advances past the original deadline, and
asserts the parent never receives the stale emit.
This commit is contained in:
Anso
2026-05-28 15:27:26 -04:00
committed by GitHub
parent 755c3c82cb
commit 0a8e6a79ae
2 changed files with 55 additions and 9 deletions
@@ -1,4 +1,4 @@
import { useEffect, useRef, useState } from 'react';
import { useCallback, useEffect, useRef, useState } from 'react';
import { CommandInput } from '@/components/ui/command';
interface SidebarSearchProps {
@@ -13,17 +13,32 @@ interface SidebarSearchProps {
const DEBOUNCE_MS = 120;
export function SidebarSearch({ value, onValueChange }: SidebarSearchProps) {
const [local, setLocal] = useState(value);
const [local, setLocalState] = useState(value);
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const lastEmittedRef = useRef(value);
// localRef mirrors `local` for the value-sync effect. Reading state via the
// ref keeps the effect deps on [value] without dropping a real read of
// `local`, which would either lie to React or trigger spurious re-runs.
const localRef = useRef(value);
const setLocal = useCallback((next: string) => {
localRef.current = next;
setLocalState(next);
}, []);
useEffect(() => {
// Parent value can move for two reasons:
// 1. Echo of our own debounced emit (lastEmittedRef matches): skip.
// 2. External reset (e.g., clear-on-filter-change): adopt it.
if (value === lastEmittedRef.current) return;
// The parent value moved. Skip only when it already matches what's shown
// locally: that is the post-emit steady state (the debounce echo settled
// back through the parent). Any other movement is an external change
// (clear-on-filter-change, navigation restore, programmatic set, or a
// coincidence) and must win: cancel any in-flight emit so it cannot undo
// the reset, then adopt the value.
if (value === localRef.current) return;
if (timerRef.current) {
clearTimeout(timerRef.current);
timerRef.current = null;
}
setLocal(value);
}, [value]);
}, [value, setLocal]);
useEffect(() => () => {
if (timerRef.current) clearTimeout(timerRef.current);
@@ -33,7 +48,7 @@ export function SidebarSearch({ value, onValueChange }: SidebarSearchProps) {
setLocal(next);
if (timerRef.current) clearTimeout(timerRef.current);
timerRef.current = setTimeout(() => {
lastEmittedRef.current = next;
timerRef.current = null;
onValueChange(next);
}, DEBOUNCE_MS);
};
@@ -88,4 +88,35 @@ describe('SidebarSearch', () => {
expect(input.value).toBe('');
});
it('cancels the pending debounce emit when the parent resets the value mid-window', () => {
const onValueChange = vi.fn();
// Start at a non-empty initial so the later reset to '' is a real prop
// transition; rerendering with the same string would no-op in React.
const { getByPlaceholderText, rerender } = renderInsideCommand({ value: 'initial', onValueChange });
const input = getByPlaceholderText('Search stacks...') as HTMLInputElement;
expect(input.value).toBe('initial');
act(() => {
fireEvent.input(input, { target: { value: 'web' } });
});
act(() => {
vi.advanceTimersByTime(50);
});
expect(onValueChange).not.toHaveBeenCalled();
rerender(
<Command shouldFilter={false}>
<SidebarSearch value="" onValueChange={onValueChange} />
</Command>,
);
act(() => {
vi.advanceTimersByTime(200);
});
// The stale timer must not fire and re-emit 'web', undoing the reset.
expect(onValueChange).not.toHaveBeenCalled();
expect(input.value).toBe('');
});
});