From 0a8e6a79ae01261fa15c09f99da7db539cca7559 Mon Sep 17 00:00:00 2001 From: Anso Date: Thu, 28 May 2026 15:27:26 -0400 Subject: [PATCH] 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. --- .../src/components/sidebar/SidebarSearch.tsx | 33 ++++++++++++++----- .../sidebar/__tests__/SidebarSearch.test.tsx | 31 +++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/frontend/src/components/sidebar/SidebarSearch.tsx b/frontend/src/components/sidebar/SidebarSearch.tsx index 6cf7f4ce..ee2c76e1 100644 --- a/frontend/src/components/sidebar/SidebarSearch.tsx +++ b/frontend/src/components/sidebar/SidebarSearch.tsx @@ -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 | 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); }; diff --git a/frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx b/frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx index 1ff19592..f586c5f7 100644 --- a/frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx +++ b/frontend/src/components/sidebar/__tests__/SidebarSearch.test.tsx @@ -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( + + + , + ); + + 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(''); + }); });