From 17451e75dd410ba89f7c8758f1bb5297b97fb45e Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Sat, 23 May 2026 08:28:27 +0700 Subject: [PATCH] refactor(frontend): migrate filter/search debounce to use-debounce npm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After landing the imperative-timer fix and extracting `useDebouncedCallback` into our own hook, the use-site count climbed past three and the case for a single, battle-tested contract beat the case for in-tree code. Drops both our `useDebouncedValue` and `useDebouncedCallback` and routes every caller through `use-debounce@10.1.1` (1M+ weekly, ~3KB gzip). API mapping at each use-site: - value debounce: `useDebouncedValue(v, ms)` → `const [v] = useDebounce(v, ms)` - callback debounce: `useDebouncedCallback(fn, ms)` → identical signature, same `.cancel()`/`.isPending()` semantics we relied on, plus `.flush()` and leading/trailing/maxWait options we now get for free. Files touched (7 use-sites): data-table.tsx, input-search.tsx, use-table-state.ts, use-table-query-filter.ts, knowledges-provider.tsx, use-flow-files-search.ts, use-detail-navigation.ts. pnpm + Vite gotcha: pnpm hoists into `.pnpm/@/`, so any transitive `import React from 'react'` from a third-party package can resolve to its own physical copy. With one path through our code and another through use-debounce, the runtime crashed at `useRef` with "Cannot read properties of null (reading 'useRef')". Added `resolve.dedupe: ['react', 'react-dom']` in vite.config.ts so the bundle ships exactly one React. Tests: - new input-search.test.tsx (10 tests) covers collapsed/expanded presentation, expand-on-click + rAF focus, burst-typing coalesce, Esc-clear mid-debounce, two-stage Esc semantics, external-source-wins race, Ctrl+F / Cmd+F global hotkey expansion. - data-table.test.tsx +5 tests: deep-link initial filterValue, 6-keystroke burst coalesce, unmount-mid-debounce cleanup, two independent DataTables on one page (settings/prompts shape), back-button-style external clear, three-round type/clear stress. - full suite: 541/541 passing, lint 0 errors. Manual smoke via Chrome DevTools MCP on every page that mounts a filter input — /flows, /templates, /settings/api-tokens, /settings/prompts (both tables independent), /knowledges (filter + semantic search hitting the server through ?qs=). All scenarios collapse to a single `[{v="", u=""}]` transition; no URL re-emit after X, no leftover timer after Esc, no cross-table leakage. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/package.json | 1 + frontend/pnpm-lock.yaml | 13 ++ .../use-detail-navigation.ts | 4 +- .../src/components/ui/data-table.test.tsx | 188 +++++++++++++++ frontend/src/components/ui/data-table.tsx | 2 +- .../src/components/ui/input-search.test.tsx | 215 ++++++++++++++++++ frontend/src/components/ui/input-search.tsx | 2 +- .../flows/files/use-flow-files-search.ts | 5 +- .../src/hooks/use-debounced-callback.test.tsx | 111 --------- frontend/src/hooks/use-debounced-callback.ts | 95 -------- frontend/src/hooks/use-debounced-value.ts | 24 -- frontend/src/hooks/use-table-query-filter.ts | 4 +- frontend/src/hooks/use-table-state.ts | 4 +- .../src/providers/knowledges-provider.tsx | 5 +- frontend/vite.config.ts | 8 + 15 files changed, 438 insertions(+), 243 deletions(-) create mode 100644 frontend/src/components/ui/input-search.test.tsx delete mode 100644 frontend/src/hooks/use-debounced-callback.test.tsx delete mode 100644 frontend/src/hooks/use-debounced-callback.ts delete mode 100644 frontend/src/hooks/use-debounced-value.ts diff --git a/frontend/package.json b/frontend/package.json index 862af285..48d94e4d 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -90,6 +90,7 @@ "tailwind-merge": "^3.6.0", "tiptap-markdown": "^0.9.0", "tw-animate-css": "^1.4.0", + "use-debounce": "^10.1.1", "vaul": "^1.1.2", "zod": "^4.4.3" }, diff --git a/frontend/pnpm-lock.yaml b/frontend/pnpm-lock.yaml index ca0c64af..237fe8ea 100644 --- a/frontend/pnpm-lock.yaml +++ b/frontend/pnpm-lock.yaml @@ -224,6 +224,9 @@ importers: tw-animate-css: specifier: ^1.4.0 version: 1.4.0 + use-debounce: + specifier: ^10.1.1 + version: 10.1.1(react@19.2.6) vaul: specifier: ^1.1.2 version: 1.1.2(@types/react-dom@19.2.3(@types/react@19.2.14))(@types/react@19.2.14)(react-dom@19.2.6(react@19.2.6))(react@19.2.6) @@ -5708,6 +5711,12 @@ packages: '@types/react': optional: true + use-debounce@10.1.1: + resolution: {integrity: sha512-kvds8BHR2k28cFsxW8k3nc/tRga2rs1RHYCqmmGqb90MEeE++oALwzh2COiuBLO1/QXiOuShXoSN2ZpWnMmvuQ==} + engines: {node: '>= 16.0.0'} + peerDependencies: + react: '*' + use-isomorphic-layout-effect@1.2.1: resolution: {integrity: sha512-tpZZ+EX0gaghDAiFR37hj5MgY6ZN55kLiPkJsKxBMZ6GZdOSPJXiOzPM984oPYZ5AnehYx5WQp1+ME8I/P/pRA==} peerDependencies: @@ -12025,6 +12034,10 @@ snapshots: optionalDependencies: '@types/react': 19.2.14 + use-debounce@10.1.1(react@19.2.6): + dependencies: + react: 19.2.6 + use-isomorphic-layout-effect@1.2.1(@types/react@19.2.14)(react@19.2.6): dependencies: react: 19.2.6 diff --git a/frontend/src/components/shared/detail-navigation/use-detail-navigation.ts b/frontend/src/components/shared/detail-navigation/use-detail-navigation.ts index 64898e85..dfeef411 100644 --- a/frontend/src/components/shared/detail-navigation/use-detail-navigation.ts +++ b/frontend/src/components/shared/detail-navigation/use-detail-navigation.ts @@ -1,7 +1,7 @@ import { useCallback, useMemo, useState } from 'react'; import { useNavigate, useSearchParams } from 'react-router-dom'; +import { useDebounce } from 'use-debounce'; -import { useDebouncedValue } from '@/hooks/use-debounced-value'; import { useLatestRef } from '@/hooks/use-latest-ref'; import { useTableQueryFilterReader } from '@/hooks/use-table-query-filter'; import { mergeHrefWithSearchParams } from '@/lib/url-params'; @@ -222,7 +222,7 @@ export function useDetailNavigation({ const [internalSearchQuery, setInternalSearchQuery] = useState(defaultSearchQuery ?? ''); const isSearchControlled = searchQuery !== undefined; const activeSearchQuery = isSearchControlled ? searchQuery : internalSearchQuery; - const debouncedSearchQuery = useDebouncedValue(activeSearchQuery, searchDebounceMs ?? DEFAULT_SEARCH_DEBOUNCE_MS); + const [debouncedSearchQuery] = useDebounce(activeSearchQuery, searchDebounceMs ?? DEFAULT_SEARCH_DEBOUNCE_MS); const setSearchQuery = useCallback( (next: string) => { diff --git a/frontend/src/components/ui/data-table.test.tsx b/frontend/src/components/ui/data-table.test.tsx index f1be2b37..8392e1c0 100644 --- a/frontend/src/components/ui/data-table.test.tsx +++ b/frontend/src/components/ui/data-table.test.tsx @@ -293,6 +293,194 @@ describe('DataTable — controlled filter projection', () => { await waitFor(() => expect(emitted.at(-1)).toBe('seed')); expect((input as HTMLInputElement).value).toBe('seed'); }); + + it('mounts with a deep-linked filterValue and shows the X clear button immediately', () => { + // Deep links land on the page with `?q=alpha` already in the URL — + // the parent passes that value down at first render, the input should + // be populated, and the trailing X should be available right away. + const emitted: string[] = []; + + render(, { wrapper: Wrapper }); + + const input = screen.getByPlaceholderText('Filter...') as HTMLInputElement; + expect(input.value).toBe('alpha'); + // No emission happened — we accepted the prop, we didn't echo it back. + expect(emitted).toEqual([]); + + // X button is rendered because the input is non-empty. + const inputGroup = input.closest('[data-slot="input-group"]'); + expect(within(inputGroup as HTMLElement).getByRole('button')).toBeInTheDocument(); + }); + + it('coalesces burst typing into a single emit with the last-typed value', async () => { + // We type six characters back-to-back. The 150ms debounce should + // collapse the burst to a single outbound call — sequential prefix + // commits would defeat the debounce and hammer the URL writer. + const emitted: string[] = []; + const user = userEvent.setup(); + + render(, { wrapper: Wrapper }); + + const input = screen.getByPlaceholderText('Filter...'); + await user.type(input, 'foobar'); + + await waitFor(() => expect(emitted.at(-1)).toBe('foobar')); + + // No intermediate prefixes — only the final value lands. + expect(emitted).toEqual(['foobar']); + }); + + it('does not write to the upstream on unmount with a pending debounce', async () => { + // If the user types and immediately navigates away, the in-flight + // timer should be cancelled by `useDebouncedCallback` so the dead + // component can't push a stale value into the parent (which by then + // may belong to a different page). + const emitted: string[] = []; + const user = userEvent.setup(); + + const { unmount } = render(, { wrapper: Wrapper }); + + await user.type(screen.getByPlaceholderText('Filter...'), 'gone', { delay: 5 }); + + // Unmount before the debounce settles. + unmount(); + + // Give any leftover timer time to misfire. + await new Promise((resolve) => setTimeout(resolve, 400)); + + // Nothing made it upstream — the timer was cancelled by the hook's + // unmount cleanup. + expect(emitted).toEqual([]); + }); + + it('keeps two DataTables on one page independent (settings/prompts shape)', async () => { + // `/settings/prompts` mounts an Agent table and a Tool table on the + // same route. Typing in one input must not leak into the other — + // both DataTableFilter instances own private state, with separate + // `lastEmittedReference`s and timers. + const emittedAgent: string[] = []; + const emittedTool: string[] = []; + const user = userEvent.setup(); + + const TwoTablesHost = () => { + const [agent, setAgent] = useState(''); + const [tool, setTool] = useState(''); + + return ( + <> + + columns={COLUMNS} + data={ROWS} + filterColumn="name" + filterPlaceholder="Agent filter..." + filterValue={agent} + onFilterChange={(next) => { + emittedAgent.push(next); + setAgent(next); + }} + /> + + columns={COLUMNS} + data={ROWS} + filterColumn="name" + filterPlaceholder="Tool filter..." + filterValue={tool} + onFilterChange={(next) => { + emittedTool.push(next); + setTool(next); + }} + /> + + ); + }; + + render(, { wrapper: Wrapper }); + + const agentInput = screen.getByPlaceholderText('Agent filter...'); + const toolInput = screen.getByPlaceholderText('Tool filter...'); + + await user.type(agentInput, 'agX'); + await waitFor(() => expect(emittedAgent.at(-1)).toBe('agX')); + + // Tool table never saw anything. + expect(emittedTool).toEqual([]); + expect((toolInput as HTMLInputElement).value).toBe(''); + + // Now type in the tool input; agent must stay frozen. + await user.type(toolInput, 'toY'); + await waitFor(() => expect(emittedTool.at(-1)).toBe('toY')); + + expect((agentInput as HTMLInputElement).value).toBe('agX'); + expect(emittedAgent).toEqual(['agX']); + }); + + it('accepts a back-button-style external clear without re-emitting the old value', async () => { + // History pop: user typed `'foo'`, then hit back. The router resets + // `filterValue` to `''`. The input should snap to `''` without + // re-emitting `'foo'` from a stale timer or sync effect. + const emitted: string[] = []; + const user = userEvent.setup(); + let setExternal: ((next: string) => void) | undefined; + + render( + { + setExternal = setter; + }} + />, + { wrapper: Wrapper }, + ); + + const input = screen.getByPlaceholderText('Filter...'); + await user.type(input, 'foo'); + await waitFor(() => expect(emitted.at(-1)).toBe('foo')); + + // Simulate the browser back button clearing `?q=`. The external + // setter bypasses `onFilterChange` (which is the realistic shape — + // a real router pop doesn't echo through our handler), so we measure + // the absence of further emits rather than a `''` arrival. + const indexOfFoo = emitted.lastIndexOf('foo'); + expect(setExternal).toBeDefined(); + act(() => setExternal!('')); + + await new Promise((resolve) => setTimeout(resolve, 400)); + + // The input cleared; no leftover timer pushed `'foo'` back upstream. + expect((input as HTMLInputElement).value).toBe(''); + expect(emitted.slice(indexOfFoo + 1)).not.toContain('foo'); + }); + + it('survives rapid alternation between typing and X clicks', async () => { + // Stress: type, clear, type, clear — three rounds back to back. + // Each round must end with a clean `''` upstream and an empty input. + const emitted: string[] = []; + const user = userEvent.setup(); + + render(, { wrapper: Wrapper }); + + const input = screen.getByPlaceholderText('Filter...'); + + for (const round of ['one', 'two', 'three']) { + await user.type(input, round); + await waitFor(() => expect(emitted.at(-1)).toBe(round)); + + const inputGroup = input.closest('[data-slot="input-group"]'); + const clearButton = within(inputGroup as HTMLElement).getByRole('button'); + await user.click(clearButton); + await waitFor(() => expect((input as HTMLInputElement).value).toBe('')); + } + + await new Promise((resolve) => setTimeout(resolve, 400)); + + // Final state: input empty, last emit is the clear from round three. + expect((input as HTMLInputElement).value).toBe(''); + expect(emitted.at(-1)).toBe(''); + // Every round's value made it through cleanly. + expect(emitted).toContain('one'); + expect(emitted).toContain('two'); + expect(emitted).toContain('three'); + }); }); describe('DataTable — uncontrolled filter is still routed through the same input', () => { diff --git a/frontend/src/components/ui/data-table.tsx b/frontend/src/components/ui/data-table.tsx index a09170c4..e94a2964 100644 --- a/frontend/src/components/ui/data-table.tsx +++ b/frontend/src/components/ui/data-table.tsx @@ -41,6 +41,7 @@ import { useState, } from 'react'; import { useLocation } from 'react-router-dom'; +import { useDebouncedCallback } from 'use-debounce'; import { Button } from '@/components/ui/button'; import { ContextMenu, ContextMenuContent, ContextMenuTrigger } from '@/components/ui/context-menu'; @@ -54,7 +55,6 @@ 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'; diff --git a/frontend/src/components/ui/input-search.test.tsx b/frontend/src/components/ui/input-search.test.tsx new file mode 100644 index 00000000..5d4dc87e --- /dev/null +++ b/frontend/src/components/ui/input-search.test.tsx @@ -0,0 +1,215 @@ +import type { ReactNode } from 'react'; + +import { act, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useState } from 'react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { TooltipProvider } from '@/components/ui/tooltip'; + +import { InputSearch } from './input-search'; + +// Collapsed state shows a single `