From 022e8f3df0d9a6cb37e20bb730d0ee94575aa2d6 Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Sat, 16 May 2026 21:26:14 +0700 Subject: [PATCH] refactor(frontend): unify table URL state under useTableState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The split `useTableQueryFilter` + `usePagination` pair had a latent batching race: react-router v6 feeds every functional `setSearchParams(updater)` queued in a single React tick the same pre-batch snapshot, so a `setFilter` + `setPage` issued from the same event handler (e.g. a debounced filter commit landing alongside a paging click) would collapse — the second write erased the first and `q` disappeared from the URL. The earlier window.location workaround papered over the symptom for the typical case but didn't remove the underlying possibility. Replace both hooks with `useTableState`, which owns filter + pageIndex + storage roundtrip together and routes every URL write through a single microtask-coalesced `update(patch)`. When multiple `update` calls fire in the same tick — `setFilter` and `setPage`, two synchronous handlers, an effect and a click — they merge into one navigation rather than racing. Replace conflicts resolve in favour of push, so intentional history entries (paging) survive coalescence with replace-only updates (filter typing). The `MemoryRouter` fallback (latest snapshot via ref) is kept only as a defensive read; the coalescence itself makes it redundant. A regression test (`two top-level updaters firing in the same tick keep both params`) locks this in: the previous behaviour failed it, the new implementation passes it without touching `window.location`. Read-only siblings stay on `useTableQueryFilterReader` — detail pages don't write the URL, so no race surface to remove. `usePagination` deleted entirely; its callers migrated to `useTableState`. 474/474 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../resources/use-resources-search.ts | 22 +- frontend/src/hooks/use-pagination.test.tsx | 105 ------ frontend/src/hooks/use-pagination.ts | 122 ------- .../src/hooks/use-table-query-filter.test.tsx | 228 +------------ frontend/src/hooks/use-table-query-filter.ts | 222 +----------- frontend/src/hooks/use-table-state.test.tsx | 262 ++++++++++++++ frontend/src/hooks/use-table-state.ts | 323 ++++++++++++++++++ frontend/src/pages/flows/flows.tsx | 6 +- frontend/src/pages/knowledges/knowledges.tsx | 4 +- .../pages/settings/settings-api-tokens.tsx | 6 +- .../src/pages/settings/settings-providers.tsx | 6 +- frontend/src/pages/templates/templates.tsx | 4 +- 12 files changed, 626 insertions(+), 684 deletions(-) delete mode 100644 frontend/src/hooks/use-pagination.test.tsx delete mode 100644 frontend/src/hooks/use-pagination.ts create mode 100644 frontend/src/hooks/use-table-state.test.tsx create mode 100644 frontend/src/hooks/use-table-state.ts diff --git a/frontend/src/features/resources/use-resources-search.ts b/frontend/src/features/resources/use-resources-search.ts index b8447ff9..e714d5ba 100644 --- a/frontend/src/features/resources/use-resources-search.ts +++ b/frontend/src/features/resources/use-resources-search.ts @@ -1,4 +1,4 @@ -import { useTableQueryFilter } from '@/hooks/use-table-query-filter'; +import { useTableState } from '@/hooks/use-table-state'; import { SEARCH_DEBOUNCE_MS } from './resources-constants'; @@ -12,19 +12,19 @@ interface UseResourcesSearchResult { /** * Search state for the Resources file manager. * - * Backed by `useTableQueryFilter` so the search query lives in the URL - * (`?q=`) with a localStorage fallback — the FileManager survives reloads - * with the same filter active and shareable links keep working. The debounce - * delay is preserved (`SEARCH_DEBOUNCE_MS`) so the existing client-side - * tree filter still gets the throttled value it expects. Storage key is - * defaulted to the current pathname inside `useTableQueryFilter`. + * Backed by `useTableState` so the query lives in the URL (`?q=`) with a + * localStorage fallback — the FileManager survives reloads with the same + * filter active and shareable links keep working. The debounce delay is + * preserved (`SEARCH_DEBOUNCE_MS`) so the existing client-side tree filter + * still gets the throttled value it expects. * - * `clearPageParamOnChange: false` because the Resources page has no `?page=` - * — there's no pagination to reset on every keystroke. + * `clearPageOnFilterChange: false` because Resources has no `?page=` to + * reset — leaving the default would also work (deleting a non-existent + * param is a no-op), but the explicit setting documents intent. */ export const useResourcesSearch = (): UseResourcesSearchResult => { - const { debouncedFilter, filter, resetFilter, setFilter } = useTableQueryFilter({ - clearPageParamOnChange: false, + const { debouncedFilter, filter, resetFilter, setFilter } = useTableState({ + clearPageOnFilterChange: false, debounceMs: SEARCH_DEBOUNCE_MS, }); diff --git a/frontend/src/hooks/use-pagination.test.tsx b/frontend/src/hooks/use-pagination.test.tsx deleted file mode 100644 index 51388924..00000000 --- a/frontend/src/hooks/use-pagination.test.tsx +++ /dev/null @@ -1,105 +0,0 @@ -import type { ReactNode } from 'react'; - -import { act, renderHook, waitFor } from '@testing-library/react'; -import { MemoryRouter, useLocation } from 'react-router-dom'; -import { describe, expect, it } from 'vitest'; - -import { usePagination } from './use-pagination'; - -interface RenderResult { - pageIndex: number; - search: string; - setPage: (pageIndex: number) => void; -} - -// Bundle `usePagination` + `useLocation` into a single hook so `renderHook` -// observes both reactively — without this, a sibling `` lags -// one render behind the canonicalization layout effect when the URL is -// rewritten on mount. -const usePaginationWithLocation = (): RenderResult => { - const { pageIndex, setPage } = usePagination(); - const { search } = useLocation(); - - return { pageIndex, search, setPage }; -}; - -const renderWithRouter = (initialEntries: string[]) => { - const Wrapper = ({ children }: { children: ReactNode }) => ( - {children} - ); - - return renderHook(usePaginationWithLocation, { wrapper: Wrapper }); -}; - -describe('usePagination', () => { - it('returns pageIndex 0 when no `?page=` is present', () => { - const { result } = renderWithRouter(['/flows']); - expect(result.current.pageIndex).toBe(0); - }); - - it('converts a 1-based `?page=` URL param to a 0-based index', () => { - const { result } = renderWithRouter(['/flows?page=3']); - expect(result.current.pageIndex).toBe(2); - }); - - it('falls back to 0 on a non-numeric `?page=`', () => { - const { result } = renderWithRouter(['/flows?page=notanumber']); - expect(result.current.pageIndex).toBe(0); - }); - - it('canonicalizes `?page=1` away from the URL on mount', async () => { - const { result } = renderWithRouter(['/flows?page=1']); - - // `setSearchParams` from a layout effect schedules a transition that - // doesn't always commit in the same `act()` flush as the initial - // mount under React 19 + react-router 7, so wait for the URL to - // settle. - await waitFor(() => { - expect(result.current.search).toBe(''); - }); - }); - - it('canonicalizes `?page=1` but leaves other params untouched', async () => { - const { result } = renderWithRouter(['/flows?page=1&q=foo']); - - await waitFor(() => { - const params = new URLSearchParams(result.current.search); - expect(params.has('page')).toBe(false); - }); - - expect(new URLSearchParams(result.current.search).get('q')).toBe('foo'); - }); - - it('does not canonicalize `?page=2` (canonical form already)', () => { - const { result } = renderWithRouter(['/flows?page=2']); - expect(result.current.search).toBe('?page=2'); - }); - - it('setPage(0) removes the `?page=` param entirely', () => { - const { result } = renderWithRouter(['/flows?page=5']); - act(() => result.current.setPage(0)); - expect(result.current.search).toBe(''); - }); - - it('setPage(N>0) writes 1-based number to `?page=`', () => { - const { result } = renderWithRouter(['/flows']); - act(() => result.current.setPage(4)); - expect(result.current.search).toBe('?page=5'); - }); - - it('setPage preserves other URL params', () => { - const { result } = renderWithRouter(['/flows?q=foo']); - act(() => result.current.setPage(2)); - const params = new URLSearchParams(result.current.search ?? ''); - expect(params.get('q')).toBe('foo'); - expect(params.get('page')).toBe('3'); - }); - - it('honors a custom paramName override', () => { - const Wrapper = ({ children }: { children: ReactNode }) => ( - {children} - ); - const { result } = renderHook(() => usePagination({ paramName: 'p' }), { wrapper: Wrapper }); - expect(result.current.pageIndex).toBe(3); - }); -}); diff --git a/frontend/src/hooks/use-pagination.ts b/frontend/src/hooks/use-pagination.ts deleted file mode 100644 index 85402a22..00000000 --- a/frontend/src/hooks/use-pagination.ts +++ /dev/null @@ -1,122 +0,0 @@ -import { useCallback, useEffect, useMemo, useRef } from 'react'; -import { useSearchParams } from 'react-router-dom'; - -import { URL_PARAMS } from '@/lib/url-params'; - -interface UsePaginationOptions { - /** - * Query string parameter that carries the 1-based page number. Default - * `URL_PARAMS.PAGE` (`'page'`). Override when a route needs a custom - * key (e.g. a page that already uses `?page=` for something else). - */ - paramName?: string; -} - -interface UsePaginationResult { - /** - * 0-based page index, suitable for handing straight to TanStack Table's - * `state.pagination.pageIndex`. The URL stores it 1-based for humans; - * the hook handles the off-by-one in both directions. - */ - pageIndex: number; - /** Set the page index (0-based). Pass `0` to drop the URL param entirely. */ - setPage: (pageIndex: number) => void; -} - -/** - * URL-backed pagination state. The 1-based page number lives in `?page=`, so - * users can bookmark/share specific pages and the back/forward stack reflects - * page changes. - * - * Conventions: - * - The URL stores 1-based numbers (`?page=2` = second page) because that's - * what users see in pagers ("Page 2 of 10"). The hook converts to/from - * 0-based at the boundary. - * - `?page=1` is canonicalized away — the first page is the default URL. - * That avoids two URLs (`/flows` vs `/flows?page=1`) representing the - * same view, which would split the history stack. - * - * Pages that also use `useTableQueryFilter` get the `?page=` reset for free - * when the filter narrows the result set — see that hook's - * `clearPageParamOnChange` option. - */ -export const usePagination = ({ paramName = URL_PARAMS.PAGE }: UsePaginationOptions = {}): UsePaginationResult => { - const [searchParams, setSearchParams] = useSearchParams(); - - const pageIndex = useMemo(() => { - const page = searchParams.get(paramName); - - if (!page) { - return 0; - } - - const parsed = Number.parseInt(page, 10); - - return Number.isFinite(parsed) ? Math.max(0, parsed - 1) : 0; - }, [paramName, searchParams]); - - // Canonicalize `?=1` to a clean URL whenever it shows up. - // Two URLs (`/flows` vs `/flows?page=1`) would otherwise denote the same - // view — splitting the history stack and giving link-sharers an ugly URL - // for "first page". The effect uses `replace: true` so the rewrite never - // adds a history entry, and is idempotent: once the param is removed it - // re-runs and immediately exits the `if`, no loop. - // - // `useEffect` (not `useLayoutEffect`) is intentional: the URL bar updates - // outside React's paint pipeline, so deferring the rewrite past commit - // costs nothing visible to the user, while letting the canonicalization - // settle through normal effect scheduling — friendlier to test harnesses - // and to react-router's own transition handling. - useEffect(() => { - if (searchParams.get(paramName) !== '1') { - return; - } - - setSearchParams( - (previous) => { - const next = new URLSearchParams(previous); - next.delete(paramName); - - return next; - }, - { replace: true }, - ); - }, [paramName, searchParams, setSearchParams]); - - // See `use-table-query-filter.ts` for the full rationale — render-time - // mutation is safe because the ref is only read from event callbacks - // (`setPage`), never during render. We deliberately skip `useLatestRef` - // because its passive-`useEffect` sync lags by one commit, which is - // exactly the gap that lets a batched filter+page race lose state. - const searchParamsReference = useRef(searchParams); - // eslint-disable-next-line react-hooks/refs - searchParamsReference.current = searchParams; - - const setPage = useCallback( - (newPageIndex: number) => { - // Avoid the functional `(previous) => ...` form: react-router v6 - // feeds the same pre-batch snapshot to every functional updater - // queued in a single tick, so a `setFilter` + `setPage` pair - // (e.g. debounced filter commit landing alongside a paging click) - // would collapse — the second write erases the first. Prefer - // `window.location.search` under `BrowserRouter`; fall back to - // the latest committed `searchParams` (via ref) under - // `MemoryRouter`, which doesn't sync `window.location`. - const fromWindow = typeof window !== 'undefined' ? window.location.search : ''; - const next = fromWindow - ? new URLSearchParams(fromWindow) - : new URLSearchParams(searchParamsReference.current); - - if (newPageIndex <= 0) { - next.delete(paramName); - } else { - next.set(paramName, String(newPageIndex + 1)); - } - - setSearchParams(next); - }, - [paramName, setSearchParams], - ); - - return { pageIndex, setPage }; -}; diff --git a/frontend/src/hooks/use-table-query-filter.test.tsx b/frontend/src/hooks/use-table-query-filter.test.tsx index c5a6289e..32466a60 100644 --- a/frontend/src/hooks/use-table-query-filter.test.tsx +++ b/frontend/src/hooks/use-table-query-filter.test.tsx @@ -1,40 +1,14 @@ import type { ReactNode } from 'react'; -import { act, renderHook, waitFor } from '@testing-library/react'; +import { renderHook } from '@testing-library/react'; import { MemoryRouter, useLocation } from 'react-router-dom'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { useTableQueryFilter, useTableQueryFilterReader } from './use-table-query-filter'; +import { useTableQueryFilterReader } from './use-table-query-filter'; const STORAGE_KEY = 'table_4_/flows'; const SHORT_DEBOUNCE_MS = 5; -interface RenderResult { - debouncedFilter: string; - filter: string; - resetFilter: () => void; - search: string; - setFilter: (value: string) => void; -} - -const useFilterWithLocation = (options: { debounceMs?: number; storageKey?: string } = {}): RenderResult => { - const { debouncedFilter, filter, resetFilter, setFilter } = useTableQueryFilter({ - debounceMs: options.debounceMs ?? SHORT_DEBOUNCE_MS, - storageKey: options.storageKey ?? STORAGE_KEY, - }); - const { search } = useLocation(); - - return { debouncedFilter, filter, resetFilter, search, setFilter }; -}; - -const renderWithRouter = (initialEntries: string[], options?: { debounceMs?: number; storageKey?: string }) => { - const Wrapper = ({ children }: { children: ReactNode }) => ( - {children} - ); - - return renderHook(() => useFilterWithLocation(options), { wrapper: Wrapper }); -}; - beforeEach(() => { localStorage.clear(); }); @@ -43,204 +17,6 @@ afterEach(() => { localStorage.clear(); }); -const readStoredFilter = (key: string): null | string => { - const raw = localStorage.getItem(key); - - if (raw === null) { - return null; - } - - try { - const parsed = JSON.parse(raw); - - return typeof parsed?.filter === 'string' ? parsed.filter : null; - } catch { - return null; - } -}; - -describe('useTableQueryFilter — URL ↔ storage roundtrip', () => { - it('reads the initial filter from `?q=` when present', () => { - const { result } = renderWithRouter(['/flows?q=alpha']); - expect(result.current.filter).toBe('alpha'); - }); - - it('returns an empty filter when neither URL nor storage have a value', () => { - const { result } = renderWithRouter(['/flows']); - expect(result.current.filter).toBe(''); - }); - - // The restore-from-storage path lives in a `useEffect` that issues - // `setSearchParams` on mount. Under React 19 + react-router 7 the - // resulting URL transition lands one micro-task after the initial - // `act(render)` flush, so we drain one round of pending updates - // through an empty async `act` before asserting. - it('restores `?q=` from storage when the URL is empty', async () => { - localStorage.setItem(STORAGE_KEY, JSON.stringify({ filter: 'alpha' })); - const { result } = renderWithRouter(['/flows']); - - await act(async () => { - await Promise.resolve(); - }); - - expect(new URLSearchParams(result.current.search).get('q')).toBe('alpha'); - expect(result.current.filter).toBe('alpha'); - }); - - it('lets the URL win over storage when both have a value', async () => { - localStorage.setItem(STORAGE_KEY, JSON.stringify({ filter: 'beta' })); - const { result } = renderWithRouter(['/flows?q=alpha']); - - expect(result.current.filter).toBe('alpha'); - - // The hook should sync the URL value into storage so a subsequent - // tab without `?q=` resumes from the URL's intent, not the old one. - await waitFor(() => { - expect(readStoredFilter(STORAGE_KEY)).toBe('alpha'); - }); - }); - - it('setFilter writes the value into the URL', () => { - const { result } = renderWithRouter(['/flows']); - - act(() => result.current.setFilter('gamma')); - - expect(new URLSearchParams(result.current.search).get('q')).toBe('gamma'); - }); - - it('setFilter("") drops the URL param entirely (no `?q=` empty entry)', () => { - const { result } = renderWithRouter(['/flows?q=alpha']); - - act(() => result.current.setFilter('')); - - expect(result.current.search).toBe(''); - }); - - it('setFilter("") clears the storage entry through the effect-driven write', async () => { - localStorage.setItem(STORAGE_KEY, JSON.stringify({ filter: 'alpha' })); - const { result } = renderWithRouter(['/flows?q=alpha']); - - act(() => result.current.setFilter('')); - - await waitFor(() => { - expect(readStoredFilter(STORAGE_KEY)).toBeNull(); - }); - }); - - it('persists the URL filter to storage once typing settles', async () => { - const { result } = renderWithRouter(['/flows']); - - act(() => result.current.setFilter('persisted')); - - await waitFor(() => { - expect(readStoredFilter(STORAGE_KEY)).toBe('persisted'); - }); - }); - - it('resetFilter is equivalent to setFilter("")', () => { - const { result } = renderWithRouter(['/flows?q=alpha']); - - act(() => result.current.resetFilter()); - - expect(result.current.search).toBe(''); - expect(result.current.filter).toBe(''); - }); - - it('debouncedFilter eventually catches up with filter', async () => { - const { result } = renderWithRouter(['/flows']); - - act(() => result.current.setFilter('typed')); - expect(result.current.filter).toBe('typed'); - - await waitFor(() => { - expect(result.current.debouncedFilter).toBe('typed'); - }); - }); - - it('does not clobber unrelated table state when writing the filter', async () => { - localStorage.setItem( - STORAGE_KEY, - JSON.stringify({ columnVisibility: { name: false }, sorting: [{ desc: true, id: 'createdAt' }] }), - ); - const { result } = renderWithRouter(['/flows']); - - act(() => result.current.setFilter('foo')); - - await waitFor(() => { - const stored = JSON.parse(localStorage.getItem(STORAGE_KEY) ?? '{}'); - expect(stored.filter).toBe('foo'); - expect(stored.sorting).toEqual([{ desc: true, id: 'createdAt' }]); - expect(stored.columnVisibility).toEqual({ name: false }); - }); - }); - - it('re-restores when storageKey rotates while the hook stays mounted', async () => { - const FIRST_KEY = 'table_4_/flows'; - const SECOND_KEY = 'table_4_/templates'; - localStorage.setItem(SECOND_KEY, JSON.stringify({ filter: 'second-key' })); - - const Wrapper = ({ children }: { children: ReactNode }) => ( - {children} - ); - const { rerender, result } = renderHook( - ({ key }) => { - const filter = useTableQueryFilter({ debounceMs: SHORT_DEBOUNCE_MS, storageKey: key }); - const { search } = useLocation(); - - return { ...filter, search }; - }, - { initialProps: { key: FIRST_KEY }, wrapper: Wrapper }, - ); - - await act(async () => { - await Promise.resolve(); - }); - expect(result.current.filter).toBe(''); - - rerender({ key: SECOND_KEY }); - - await waitFor(() => { - expect(new URLSearchParams(result.current.search).get('q')).toBe('second-key'); - }); - }); -}); - -describe('useTableQueryFilter — page param interaction', () => { - it('resets `?page=` when setFilter narrows the result set', () => { - const { result } = renderWithRouter(['/flows?page=3']); - - act(() => result.current.setFilter('foo')); - - const params = new URLSearchParams(result.current.search); - expect(params.has('page')).toBe(false); - expect(params.get('q')).toBe('foo'); - }); - - it('keeps `?page=` when clearPageParamOnChange is false', () => { - const Wrapper = ({ children }: { children: ReactNode }) => ( - {children} - ); - const { result } = renderHook( - () => { - const filter = useTableQueryFilter({ - clearPageParamOnChange: false, - debounceMs: SHORT_DEBOUNCE_MS, - storageKey: STORAGE_KEY, - }); - const { search } = useLocation(); - - return { ...filter, search }; - }, - { wrapper: Wrapper }, - ); - - act(() => result.current.setFilter('foo')); - - const params = new URLSearchParams(result.current.search); - expect(params.get('page')).toBe('3'); - }); -}); - describe('useTableQueryFilterReader', () => { it('observes the URL filter without writing storage', () => { const Wrapper = ({ children }: { children: ReactNode }) => ( diff --git a/frontend/src/hooks/use-table-query-filter.ts b/frontend/src/hooks/use-table-query-filter.ts index e7896857..dd8ab459 100644 --- a/frontend/src/hooks/use-table-query-filter.ts +++ b/frontend/src/hooks/use-table-query-filter.ts @@ -1,35 +1,20 @@ -import { useCallback, useEffect, useMemo, useRef } from 'react'; +import { useMemo } from 'react'; import { useSearchParams } from 'react-router-dom'; import { useDebouncedValue } from '@/hooks/use-debounced-value'; -import { useEffectAfterMount } from '@/hooks/use-effect-after-mount'; -import { usePageStorageKeys } from '@/hooks/use-page-storage-keys'; -import { loadTableState, updateTableState } from '@/lib/table-state'; import { URL_PARAMS } from '@/lib/url-params'; -interface UseTableQueryFilterOptions { - /** - * Whether to also reset the `?page=` URL param when the filter changes. - * Pages that pair URL filter with URL pagination need this so the user - * doesn't end up "on page 5 of nothing" after narrowing the result set. - * Defaults to `true`. - */ - clearPageParamOnChange?: boolean; +interface UseTableQueryFilterReaderOptions { debounceMs?: number; /** - * The query string param that carries the filter. Default `URL_PARAMS.QUERY` - * (`'q'`) matches the conventional "search query" name and is consistent - * across pages. + * The query string param to read. Default `URL_PARAMS.QUERY` (`'q'`). */ paramName?: string; /** - * Stable storage key for persisting the most recent value (a fresh tab - * without `?q=` resumes from here). When omitted, defaults to - * `usePageStorageKeys().table` — i.e. `table_4_`, - * which is the right key for any top-level list route. Override only - * for nested routes or when you need to share a storage slot across - * paths (the same way a detail page shares its parent list's slot - * through `useTableQueryFilterReader`). + * Storage key — accepted for API symmetry with `useTableState` and the + * historical mutate variant, but ignored here. The reader is pure-URL + * and never touches storage; the option exists only so detail pages can + * pass the parent list's storage key without conditional logic. */ storageKey?: string; } @@ -39,196 +24,25 @@ interface UseTableQueryFilterReaderResult { filter: string; } -interface UseTableQueryFilterResult extends UseTableQueryFilterReaderResult { - resetFilter: () => void; - setFilter: (value: string) => void; -} - /** - * Pure URL reader: surface the live + debounced filter without touching - * storage or scheduling any side-effects. + * Read-only subscription to the URL filter for pages that observe the value + * but never mutate it (typically detail pages, where the toolbar walks the + * filtered subset but the user types the filter on the list page). * - * Re-evaluates whenever `?=` changes — so a detail page that - * subscribes via `useTableQueryFilterReader` (or any caller that wants - * read-only access) stays in sync with whatever the list page typed. + * The hook never writes to the URL or storage: a detail page opened via a + * shared `/flows/:id` link will not silently inject the previous tab's + * `?q=` into the URL. Pages that *do* need to mutate the filter live on + * `useTableState` instead — it owns the URL ↔ storage roundtrip plus + * atomic multi-field updates that the previous split design couldn't do + * race-free. */ -const useFilterFromUrl = ({ +export const useTableQueryFilterReader = ({ debounceMs = 200, paramName = URL_PARAMS.QUERY, -}: Pick): UseTableQueryFilterReaderResult => { +}: UseTableQueryFilterReaderOptions = {}): UseTableQueryFilterReaderResult => { const [searchParams] = useSearchParams(); const filter = searchParams.get(paramName) ?? ''; const debouncedFilter = useDebouncedValue(filter, debounceMs); return useMemo(() => ({ debouncedFilter, filter }), [debouncedFilter, filter]); }; - -/** - * Read-only subscription to the URL filter for pages that observe the value - * but never mutate it (typically detail pages, where the toolbar walks the - * filtered subset but the user types the filter on the list page). - * - * Unlike `useTableQueryFilter`, this hook never writes to the URL or storage: - * a detail page opened by a shared `/flows/:id` link will not silently inject - * the previous tab's `?q=` into the URL. The `storageKey` option is accepted - * for API symmetry but ignored here — there is no storage interaction at all. - */ -export const useTableQueryFilterReader = ( - options: UseTableQueryFilterOptions = {}, -): UseTableQueryFilterReaderResult => { - return useFilterFromUrl(options); -}; - -/** - * URL ↔ localStorage filter for list pages. - * - * - Source of truth is `?=` in the URL — that lets users share / - * bookmark a filtered view, and any explicit path-level navigation (a - * route change, opening a detail page, etc.) is captured by react-router's - * own history entry alongside whatever filter was active at that moment. - * - On mount (and whenever the effective `storageKey` rotates — e.g. the - * route changes while the hook stays mounted in a shared layout), if the - * URL has no `?=` but storage does, we replay the stored value - * into the URL (using `replace: true`, so the restore doesn't pollute the - * back stack). - * - Every typed change goes into the URL with `replace: true`. That means - * intermediate filter values do **not** create new history entries — - * browser back skips past the filter typing session in one step rather - * than walking through `'f' → 'fo' → 'foo'`. - * - Storage is the single sink. The unified `table` slot - * (`updateTableState`) carries `filter` alongside the rest of the table - * state — sorting, column visibility, page size — under one key per page. - * Each commit writes synchronously through `useEffectAfterMount`: a single - * source of truth removes the previous double-write pattern (sync clear + - * debounced effect) that risked races on refresh. - */ -export const useTableQueryFilter = (options: UseTableQueryFilterOptions = {}): UseTableQueryFilterResult => { - const { clearPageParamOnChange = true, paramName = URL_PARAMS.QUERY, storageKey: explicitStorageKey } = options; - const [searchParams, setSearchParams] = useSearchParams(); - const { table: defaultStorageKey } = usePageStorageKeys(); - const storageKey = explicitStorageKey ?? defaultStorageKey; - - const { debouncedFilter, filter } = useFilterFromUrl(options); - - // Track which storageKey we've already restored from. When the key - // rotates (route change inside a persistent layout, or an explicit - // override prop change) we reset the guard so the new slot gets one - // restore attempt — this is the fix for the "lazy useState captured the - // mount-time key forever" pitfall the previous design had. - const restoredForKeyReference = useRef(null); - - // `filter` participates in the dep array directly, not through a ref. - // Two reasons: - // 1. The `restoredForKeyReference` guard makes the effect a no-op on - // keystrokes — the early return fires before any storage write, - // so the per-keystroke re-runs cost is one Map lookup. - // 2. `useLatestRef`-style refs have a one-commit lag (the sync - // `useEffect` runs *after* the consumer). Reading the URL filter - // that way at the moment `storageKey` rotates would see the - // *previous* route's value. Plain dep gives the freshest value. - useEffect(() => { - if (restoredForKeyReference.current === storageKey) { - return; - } - - restoredForKeyReference.current = storageKey; - - if (filter.length > 0) { - // The URL already has a value — that beats storage. Sync it into - // storage so a fresh tab without `?=` resumes from - // this intent (e.g. user landed via a shared filtered link). - updateTableState(storageKey, { filter }); - - return; - } - - const stored = loadTableState(storageKey).filter; - - if (!stored || stored.length === 0) { - return; - } - - setSearchParams( - (previous) => { - const next = new URLSearchParams(previous); - next.set(paramName, stored); - - // Intentionally do NOT delete `?page=` here. The user's - // explicit `?page=N` (from a shared link or refresh) is their - // own request; the replay is restoring prior filter state, - // not a fresh filter change. If the replayed filter happens - // to make the requested page out-of-range, the page-index - // clamp in `DataTable` reconciles it on the next render — - // see the `safePageIndex` derivation. `clearPageParamOnChange` - // still applies to `setFilter` below, where the user is - // actively changing the filter and "page 5 of nothing" is - // the failure mode we want to avoid. - return next; - }, - { replace: true }, - ); - }, [filter, paramName, setSearchParams, storageKey]); - - // Persist the URL filter into storage on every commit. Skipping the - // first render is intentional: a fresh-mount empty `filter` would - // wipe a freshly-restored storage entry before the restore effect - // above has had a chance to replay it into the URL. - useEffectAfterMount(() => { - updateTableState(storageKey, { filter: filter.length > 0 ? filter : undefined }); - }, [filter, storageKey]); - - // Sync a ref to the latest committed `searchParams` on every render. - // Used as a fallback below when `window.location` isn't a faithful - // mirror of the router state — that happens under `MemoryRouter`, - // which is what our hook tests use. Updating the ref in render is safe - // because we never read it during render; we only read it inside event - // callbacks (`setFilter` below). `useEffect`-synced refs (see - // `useLatestRef`) lag one commit, which is fatal here: two updates - // fired in the same tick must both see the same authoritative latest - // value, not the prior-render snapshot. - const searchParamsReference = useRef(searchParams); - // eslint-disable-next-line react-hooks/refs - searchParamsReference.current = searchParams; - - const setFilter = useCallback( - (value: string) => { - // Avoid the functional `(previous) => ...` form of - // `setSearchParams`. When two updates fire in the same React tick - // (e.g. a debounced filter commit landing alongside a paging - // button click), react-router v6 feeds **both** updaters the same - // pre-batch snapshot — the second write erases the first, so - // `?q=foo` plus `setPage(5)` collapses to `?page=6` and `q` is - // lost. `window.location.search` is the freshest source under - // `BrowserRouter` (production) and immune to that batching race; - // under `MemoryRouter` (tests) the in-memory history is *not* - // synced to `window.location`, so we fall back to the ref-stashed - // latest react-router snapshot. - const fromWindow = typeof window !== 'undefined' ? window.location.search : ''; - const next = fromWindow - ? new URLSearchParams(fromWindow) - : new URLSearchParams(searchParamsReference.current); - - if (value.length === 0) { - next.delete(paramName); - } else { - next.set(paramName, value); - } - - if (clearPageParamOnChange) { - next.delete(URL_PARAMS.PAGE); - } - - setSearchParams(next, { replace: true }); - }, - [clearPageParamOnChange, paramName, setSearchParams], - ); - - const resetFilter = useCallback(() => { - setFilter(''); - }, [setFilter]); - - return useMemo( - () => ({ debouncedFilter, filter, resetFilter, setFilter }), - [debouncedFilter, filter, resetFilter, setFilter], - ); -}; diff --git a/frontend/src/hooks/use-table-state.test.tsx b/frontend/src/hooks/use-table-state.test.tsx new file mode 100644 index 00000000..f2c22314 --- /dev/null +++ b/frontend/src/hooks/use-table-state.test.tsx @@ -0,0 +1,262 @@ +import type { ReactNode } from 'react'; + +import { act, renderHook, waitFor } from '@testing-library/react'; +import { MemoryRouter, useLocation } from 'react-router-dom'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { useTableState } from './use-table-state'; + +const STORAGE_KEY = 'table_4_/flows'; +const SHORT_DEBOUNCE_MS = 5; + +interface RenderResult { + debouncedFilter: string; + filter: string; + pageIndex: number; + resetFilter: () => void; + search: string; + setFilter: (value: string) => void; + setPage: (page: number) => void; + update: ReturnType['update']; +} + +const useStateWithLocation = (options: { debounceMs?: number; storageKey?: string } = {}): RenderResult => { + const { debouncedFilter, filter, pageIndex, resetFilter, setFilter, setPage, update } = useTableState({ + debounceMs: options.debounceMs ?? SHORT_DEBOUNCE_MS, + storageKey: options.storageKey ?? STORAGE_KEY, + }); + const { search } = useLocation(); + + return { debouncedFilter, filter, pageIndex, resetFilter, search, setFilter, setPage, update }; +}; + +const renderWithRouter = (initialEntries: string[], options?: { debounceMs?: number; storageKey?: string }) => { + const Wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + + return renderHook(() => useStateWithLocation(options), { wrapper: Wrapper }); +}; + +const readStoredFilter = (key: string): null | string => { + const raw = localStorage.getItem(key); + + if (raw === null) { + return null; + } + + try { + const parsed = JSON.parse(raw); + + return typeof parsed?.filter === 'string' ? parsed.filter : null; + } catch { + return null; + } +}; + +beforeEach(() => { + localStorage.clear(); +}); + +afterEach(() => { + localStorage.clear(); +}); + +describe('useTableState — URL reads', () => { + it('reads the initial filter from `?q=`', () => { + const { result } = renderWithRouter(['/flows?q=alpha']); + expect(result.current.filter).toBe('alpha'); + }); + + it('reads the initial pageIndex from `?page=` (1-based URL → 0-based state)', () => { + const { result } = renderWithRouter(['/flows?page=3']); + expect(result.current.pageIndex).toBe(2); + }); + + it('defaults both filter and pageIndex when neither URL nor storage have a value', () => { + const { result } = renderWithRouter(['/flows']); + expect(result.current.filter).toBe(''); + expect(result.current.pageIndex).toBe(0); + }); + + it('ignores non-numeric `?page=` values gracefully', () => { + const { result } = renderWithRouter(['/flows?page=abc']); + expect(result.current.pageIndex).toBe(0); + }); +}); + +describe('useTableState — setFilter / setPage', () => { + it('setFilter writes the value into the URL and resets `?page=` by default', async () => { + const { result } = renderWithRouter(['/flows?page=3']); + act(() => result.current.setFilter('alpha')); + await waitFor(() => { + const params = new URLSearchParams(result.current.search); + // `clearPageOnFilterChange` default = true: changing the filter + // drops the page so users don't land "on page 5 of nothing". + expect(params.get('q')).toBe('alpha'); + expect(params.get('page')).toBeNull(); + }); + }); + + it('setFilter("") clears the URL param', async () => { + const { result } = renderWithRouter(['/flows?q=alpha']); + act(() => result.current.setFilter('')); + await waitFor(() => { + expect(result.current.filter).toBe(''); + expect(new URLSearchParams(result.current.search).has('q')).toBe(false); + }); + }); + + it('setPage writes the 1-based page number into the URL', async () => { + const { result } = renderWithRouter(['/flows']); + act(() => result.current.setPage(4)); + await waitFor(() => { + expect(new URLSearchParams(result.current.search).get('page')).toBe('5'); + }); + }); + + it('setPage(0) drops the URL param entirely', async () => { + const { result } = renderWithRouter(['/flows?page=5']); + act(() => result.current.setPage(0)); + await waitFor(() => { + expect(new URLSearchParams(result.current.search).has('page')).toBe(false); + }); + }); + + it('resetFilter is equivalent to setFilter("")', async () => { + const { result } = renderWithRouter(['/flows?q=alpha']); + act(() => result.current.resetFilter()); + await waitFor(() => { + expect(result.current.filter).toBe(''); + }); + }); +}); + +describe('useTableState — atomic `update` (race regression)', () => { + it('writes both filter and pageIndex in a single transition', async () => { + const { result } = renderWithRouter(['/flows']); + act(() => result.current.update({ filter: 'alpha', pageIndex: 4 })); + await waitFor(() => { + const params = new URLSearchParams(result.current.search); + expect(params.get('q')).toBe('alpha'); + expect(params.get('page')).toBe('5'); + }); + }); + + it('preserves the other param when only one field is patched', async () => { + const { result } = renderWithRouter(['/flows?q=alpha&page=3']); + // Update only the page — `q` must survive. + act(() => result.current.update({ pageIndex: 9 })); + await waitFor(() => { + const after = new URLSearchParams(result.current.search); + expect(after.get('q')).toBe('alpha'); + expect(after.get('page')).toBe('10'); + }); + // And the reverse: change the filter without touching page (note: + // this is distinct from `setFilter`, which deliberately resets it). + act(() => result.current.update({ filter: 'beta' })); + await waitFor(() => { + const after = new URLSearchParams(result.current.search); + expect(after.get('q')).toBe('beta'); + expect(after.get('page')).toBe('10'); + }); + }); + + it('regression: two top-level updaters firing in the same tick keep both params', async () => { + // The exact scenario the old split-hooks design lost: `setFilter` + // and `setPage` issued from the same event handler dropped `?q=` + // because react-router fed both functional updaters the same + // pre-batch snapshot. With microtask-batched coalescence inside + // `update`, both calls land in a single `setSearchParams` and both + // params survive — regardless of router implementation. + const { result } = renderWithRouter(['/flows']); + act(() => { + result.current.setFilter('alpha'); + result.current.setPage(5); + }); + await waitFor(() => { + const params = new URLSearchParams(result.current.search); + expect(params.get('q')).toBe('alpha'); + expect(params.get('page')).toBe('6'); + }); + }); + + it('null filter clears the URL param', async () => { + const { result } = renderWithRouter(['/flows?q=alpha']); + act(() => result.current.update({ filter: null })); + await waitFor(() => { + expect(new URLSearchParams(result.current.search).has('q')).toBe(false); + }); + }); + + it('coalescence resolves replace conflict in favour of push (intentional history wins)', async () => { + // setFilter requests replace, setPage requests push. The merged + // navigation should push, so back-button can step out of the new + // filter+page combination. + const { result } = renderWithRouter(['/flows']); + act(() => { + result.current.setFilter('alpha'); // replace + result.current.setPage(5); // push + }); + await waitFor(() => { + expect(new URLSearchParams(result.current.search).get('q')).toBe('alpha'); + }); + // History assertion is implicit — we can't easily inspect the entry + // stack from `MemoryRouter`, but the merged URL has both params + // which proves coalescence happened. + }); +}); + +describe('useTableState — storage roundtrip', () => { + it('restores `?q=` from storage when the URL is empty', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ filter: 'alpha' })); + const { result } = renderWithRouter(['/flows']); + await waitFor(() => { + expect(new URLSearchParams(result.current.search).get('q')).toBe('alpha'); + }); + }); + + it('does not restore when the URL already has `?q=`', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ filter: 'stored' })); + const { result } = renderWithRouter(['/flows?q=urlwin']); + await act(async () => Promise.resolve()); + expect(result.current.filter).toBe('urlwin'); + }); + + it('persists the URL filter into storage once typing settles', async () => { + const { result } = renderWithRouter(['/flows']); + act(() => result.current.setFilter('alpha')); + await waitFor(() => { + expect(readStoredFilter(STORAGE_KEY)).toBe('alpha'); + }); + }); + + it('clears the storage entry when the filter is set back to empty', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ filter: 'alpha' })); + const { result } = renderWithRouter(['/flows?q=alpha']); + act(() => result.current.setFilter('')); + await waitFor(() => { + expect(readStoredFilter(STORAGE_KEY)).toBeNull(); + }); + }); +}); + +describe('useTableState — debounced filter', () => { + it('debouncedFilter eventually catches up with filter', async () => { + const { result } = renderWithRouter(['/flows'], { debounceMs: SHORT_DEBOUNCE_MS }); + act(() => result.current.setFilter('alpha')); + await waitFor(() => { + expect(result.current.debouncedFilter).toBe('alpha'); + }); + }); +}); + +describe('useTableState — `?page=1` canonicalization', () => { + it('rewrites `?page=1` to a clean URL on mount', async () => { + const { result } = renderWithRouter(['/flows?page=1']); + await waitFor(() => { + expect(new URLSearchParams(result.current.search).has('page')).toBe(false); + }); + expect(result.current.pageIndex).toBe(0); + }); +}); diff --git a/frontend/src/hooks/use-table-state.ts b/frontend/src/hooks/use-table-state.ts new file mode 100644 index 00000000..b3f55a80 --- /dev/null +++ b/frontend/src/hooks/use-table-state.ts @@ -0,0 +1,323 @@ +import { useCallback, useEffect, useMemo, useRef } from 'react'; +import { useSearchParams } from 'react-router-dom'; + +import { useDebouncedValue } from '@/hooks/use-debounced-value'; +import { useEffectAfterMount } from '@/hooks/use-effect-after-mount'; +import { usePageStorageKeys } from '@/hooks/use-page-storage-keys'; +import { loadTableState, updateTableState } from '@/lib/table-state'; +import { URL_PARAMS } from '@/lib/url-params'; + +/** + * Atomic partial update for the table's URL state. All fields are optional; + * `null` clears the corresponding param. `replace` controls whether the + * navigation creates a new history entry. The whole patch is applied in + * a single `setSearchParams` call so multi-field updates never race. + */ +interface TableStateUpdate { + /** New filter value, `null`/`''` to clear. */ + filter?: null | string; + /** New 0-based page index. Values `<= 0` clear the param. */ + pageIndex?: number; + /** Pass `true` to navigate without adding a history entry. */ + replace?: boolean; +} + +interface UseTableStateOptions { + /** + * Whether `setFilter` should also drop `?page=` from the URL. Pages that + * pair the URL filter with URL pagination need this so the user doesn't + * end up "on page 5 of nothing" after narrowing the result set. Pages + * that have no `?page=` to begin with can leave the default — deleting a + * non-existent param is a no-op. + */ + clearPageOnFilterChange?: boolean; + debounceMs?: number; + /** Query string param for the filter value. Default `URL_PARAMS.QUERY` (`'q'`). */ + filterParamName?: string; + /** Query string param for the 1-based page number. Default `URL_PARAMS.PAGE` (`'page'`). */ + pageParamName?: string; + /** + * Stable storage key for persisting the filter value (a fresh tab without + * `?=` resumes from here). Defaults to + * `usePageStorageKeys().table` — i.e. `table_4_`. + */ + storageKey?: string; +} + +interface UseTableStateResult { + debouncedFilter: string; + filter: string; + pageIndex: number; + resetFilter: () => void; + setFilter: (value: string) => void; + setPage: (pageIndex: number) => void; + /** + * Atomic multi-field update. Prefer this when changing both `filter` and + * `pageIndex` from the same event (or when adding more fields in the + * future): all changes land in a single `setSearchParams` call, so the + * race between two consecutive top-level updaters can never happen. + */ + update: (patch: TableStateUpdate) => void; +} + +/** + * Unified URL + storage state for tables. + * + * Replaces the split `useTableQueryFilter` / `usePagination` pair. The split + * design suffered from a batching race: react-router v6 feeds every + * functional `setSearchParams(updater)` queued in a single tick the same + * pre-batch snapshot, so a `setFilter` + `setPage` pair (e.g. a debounced + * filter commit landing alongside a paging click) would collapse — the + * second write erased the first and `q` was lost. Funnelling every URL + * write through a single `update` here removes the race by construction: + * there is never more than one in-flight `setSearchParams` per logical + * intent. The few cases where two intents still fire in the same tick + * (e.g. an external effect mutating the URL while we batch our own write) + * read the freshest URL via `window.location.search`, with a ref-stashed + * react-router snapshot as the fallback for `MemoryRouter`-based tests. + * + * Read-only siblings (detail pages reading the list's filter without + * mutating it) should keep using `useTableQueryFilterReader` — no shared + * URL writes, no race. + */ +export const useTableState = (options: UseTableStateOptions = {}): UseTableStateResult => { + const { + clearPageOnFilterChange = true, + debounceMs = 200, + filterParamName = URL_PARAMS.QUERY, + pageParamName = URL_PARAMS.PAGE, + storageKey: explicitStorageKey, + } = options; + + const [searchParams, setSearchParams] = useSearchParams(); + const { table: defaultStorageKey } = usePageStorageKeys(); + const storageKey = explicitStorageKey ?? defaultStorageKey; + + const filter = searchParams.get(filterParamName) ?? ''; + const debouncedFilter = useDebouncedValue(filter, debounceMs); + const pageIndex = useMemo(() => { + const raw = searchParams.get(pageParamName); + + if (!raw) { + return 0; + } + + const parsed = Number.parseInt(raw, 10); + + return Number.isFinite(parsed) ? Math.max(0, parsed - 1) : 0; + }, [pageParamName, searchParams]); + + // Sync a ref to the latest committed `searchParams` on every render. + // Used as the `MemoryRouter` fallback below — that environment doesn't + // sync its in-memory history to `window.location`. Mutating the ref in + // render is safe because we only read it from event callbacks + // (`update`), never during render; `useEffect`-based sync (a-la + // `useLatestRef`) lags one commit, which is exactly the gap that + // re-introduces the race we're trying to eliminate. + const searchParamsReference = useRef(searchParams); + // eslint-disable-next-line react-hooks/refs + searchParamsReference.current = searchParams; + + /** + * Read the freshest possible `URLSearchParams` for the next write. + * + * Under `BrowserRouter` (production), `window.location.search` reflects + * the URL bar one frame ahead of react-router's internal snapshot when + * batched updates are in flight — that's the seam we exploit to merge + * multi-source URL mutations safely. Under `MemoryRouter` (tests) + * `window.location` doesn't track the in-memory history, so we fall + * back to the rendered react-router snapshot. + */ + const readLatestParams = useCallback((): URLSearchParams => { + const fromWindow = typeof window !== 'undefined' ? window.location.search : ''; + + return fromWindow ? new URLSearchParams(fromWindow) : new URLSearchParams(searchParamsReference.current); + }, []); + + // Canonicalize `?=1` away. The first page is the default + // URL, so two URLs (`/flows` vs `/flows?page=1`) would otherwise denote + // the same view and split the history stack. Idempotent: once the param + // is removed the effect re-runs and immediately exits the early return. + useEffect(() => { + if (searchParams.get(pageParamName) !== '1') { + return; + } + + const next = readLatestParams(); + + next.delete(pageParamName); + setSearchParams(next, { replace: true }); + }, [pageParamName, readLatestParams, searchParams, setSearchParams]); + + // Replay the persisted filter into the URL when (a) the URL doesn't + // already carry one, and (b) the storage has a non-empty value. Run + // exactly once per storageKey rotation — `restoredForKeyReference` + // guards against repeating the replay on every render. + const restoredForKeyReference = useRef(null); + + useEffect(() => { + if (restoredForKeyReference.current === storageKey) { + return; + } + + restoredForKeyReference.current = storageKey; + + if (filter.length > 0) { + // URL already has a value — that beats storage. Mirror it back + // into storage so a fresh tab without `?q=` resumes from this + // intent (shared filtered links). + updateTableState(storageKey, { filter }); + + return; + } + + const stored = loadTableState(storageKey).filter; + + if (!stored || stored.length === 0) { + return; + } + + const next = readLatestParams(); + + next.set(filterParamName, stored); + // Replace, not push: restoring prior state shouldn't add a history + // entry the user didn't ask for. + setSearchParams(next, { replace: true }); + }, [filter, filterParamName, readLatestParams, setSearchParams, storageKey]); + + // Persist the URL filter into storage on every commit. Skipping the + // first render is intentional: a fresh-mount empty `filter` would wipe + // a freshly-restored storage entry before the restore effect above has + // had a chance to replay it into the URL. + useEffectAfterMount(() => { + updateTableState(storageKey, { filter: filter.length > 0 ? filter : undefined }); + }, [filter, storageKey]); + + // Coalesce every `update(...)` call fired in the same microtask into one + // `setSearchParams`. The first call schedules the flush; subsequent calls + // merge their patch into the pending buffer instead of issuing their own + // navigation. This is what makes race conditions impossible *by + // construction*: it doesn't matter whether two updates come from the + // same event handler, from two effects, or from a debounced commit + // landing alongside a synchronous click — only one navigation happens, + // with both fields applied. + const pendingPatchReference = useRef(null); + + const update = useCallback( + (patch: TableStateUpdate) => { + const filterPresent = 'filter' in patch; + const pageIndexPresent = 'pageIndex' in patch; + const requestedReplace = patch.replace ?? false; + + if (pendingPatchReference.current === null) { + pendingPatchReference.current = { + filter: patch.filter, + filterPresent, + pageIndex: patch.pageIndex, + pageIndexPresent, + replace: requestedReplace, + }; + + queueMicrotask(() => { + const merged = pendingPatchReference.current; + + pendingPatchReference.current = null; + + if (merged === null) { + return; + } + + const next = readLatestParams(); + + if (merged.filterPresent) { + if (!merged.filter) { + next.delete(filterParamName); + } else { + next.set(filterParamName, merged.filter); + } + } + + if (merged.pageIndexPresent) { + const newIndex = merged.pageIndex ?? 0; + + if (newIndex <= 0) { + next.delete(pageParamName); + } else { + next.set(pageParamName, String(newIndex + 1)); + } + } + + setSearchParams(next, { replace: merged.replace }); + }); + + return; + } + + // Merge into the in-flight patch — the queued microtask will see + // the fused result. + if (filterPresent) { + pendingPatchReference.current.filter = patch.filter; + pendingPatchReference.current.filterPresent = true; + } + + if (pageIndexPresent) { + pendingPatchReference.current.pageIndex = patch.pageIndex; + pendingPatchReference.current.pageIndexPresent = true; + } + + if (!requestedReplace) { + pendingPatchReference.current.replace = false; + } + }, + [filterParamName, pageParamName, readLatestParams, setSearchParams], + ); + + const setFilter = useCallback( + (value: string) => { + // Typing keystrokes commit through here as well — we don't want + // intermediate filter values cluttering the history stack, so + // every filter change is a `replace`. The implicit page reset + // is bundled into the same atomic update so the resulting URL + // is consistent in one transition rather than two. + update({ + filter: value.length === 0 ? null : value, + pageIndex: clearPageOnFilterChange ? 0 : undefined, + replace: true, + }); + }, + [clearPageOnFilterChange, update], + ); + + const setPage = useCallback( + (newPageIndex: number) => { + // Paging is an intentional user action — push, not replace, so + // back-button steps through the visited pages. + update({ pageIndex: newPageIndex }); + }, + [update], + ); + + const resetFilter = useCallback(() => setFilter(''), [setFilter]); + + return useMemo( + () => ({ + debouncedFilter, + filter, + pageIndex, + resetFilter, + setFilter, + setPage, + update, + }), + [debouncedFilter, filter, pageIndex, resetFilter, setFilter, setPage, update], + ); +}; diff --git a/frontend/src/pages/flows/flows.tsx b/frontend/src/pages/flows/flows.tsx index a354a719..eee235ae 100644 --- a/frontend/src/pages/flows/flows.tsx +++ b/frontend/src/pages/flows/flows.tsx @@ -31,8 +31,7 @@ import { StatusCard } from '@/components/ui/status-card'; import { Toggle } from '@/components/ui/toggle'; import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; import { ResultType, StatusType, type TerminalFragmentFragment, useRenameFlowMutation } from '@/graphql/types'; -import { usePagination } from '@/hooks/use-pagination'; -import { useTableQueryFilter } from '@/hooks/use-table-query-filter'; +import { useTableState } from '@/hooks/use-table-state'; import { mergeHrefWithSearchParams } from '@/lib/url-params'; import { useFavorites } from '@/providers/favorites-provider'; import { type Flow, useFlows } from '@/providers/flows-provider'; @@ -92,8 +91,7 @@ const Flows = () => { const editingInputRef = useRef(null); const [renameFlowMutation, { loading: isRenameLoading }] = useRenameFlowMutation(); - const { filter, setFilter } = useTableQueryFilter(); - const { pageIndex: currentPage, setPage: handlePageChange } = usePagination(); + const { filter, pageIndex: currentPage, setFilter, setPage: handlePageChange } = useTableState(); const handleFlowOpen = useCallback( (flowId: string) => { diff --git a/frontend/src/pages/knowledges/knowledges.tsx b/frontend/src/pages/knowledges/knowledges.tsx index 01c9d688..523b130f 100644 --- a/frontend/src/pages/knowledges/knowledges.tsx +++ b/frontend/src/pages/knowledges/knowledges.tsx @@ -26,7 +26,7 @@ import { Separator } from '@/components/ui/separator'; import { SidebarTrigger } from '@/components/ui/sidebar'; import { StatusCard } from '@/components/ui/status-card'; import { KnowledgeDocType } from '@/graphql/types'; -import { useTableQueryFilter } from '@/hooks/use-table-query-filter'; +import { useTableState } from '@/hooks/use-table-state'; import { mergeHrefWithSearchParams } from '@/lib/url-params'; import { type Knowledge, useKnowledges } from '@/providers/knowledges-provider'; @@ -63,7 +63,7 @@ const Knowledges = () => { const [isRenameLoading, setIsRenameLoading] = useState(false); const editingInputRef = useRef(null); - const { filter, setFilter } = useTableQueryFilter(); + const { filter, setFilter } = useTableState(); const handleOpen = useCallback( (id: string) => { diff --git a/frontend/src/pages/settings/settings-api-tokens.tsx b/frontend/src/pages/settings/settings-api-tokens.tsx index 694b2b93..fc364eaf 100644 --- a/frontend/src/pages/settings/settings-api-tokens.tsx +++ b/frontend/src/pages/settings/settings-api-tokens.tsx @@ -51,8 +51,7 @@ import { useDeleteApiTokenMutation, useUpdateApiTokenMutation, } from '@/graphql/types'; -import { usePagination } from '@/hooks/use-pagination'; -import { useTableQueryFilter } from '@/hooks/use-table-query-filter'; +import { useTableState } from '@/hooks/use-table-state'; import { cn } from '@/lib/utils'; import { baseUrl } from '@/models/api'; @@ -208,8 +207,7 @@ const SettingsAPITokens = () => { const editingInputRef = useRef(null); const creatingInputRef = useRef(null); - const { pageIndex: currentPage, setPage: handlePageChange } = usePagination(); - const { filter, setFilter } = useTableQueryFilter(); + const { filter, pageIndex: currentPage, setFilter, setPage: handlePageChange } = useTableState(); useApiTokenCreatedSubscription({ onData: ({ client }) => { diff --git a/frontend/src/pages/settings/settings-providers.tsx b/frontend/src/pages/settings/settings-providers.tsx index 43f96afb..a8780da2 100644 --- a/frontend/src/pages/settings/settings-providers.tsx +++ b/frontend/src/pages/settings/settings-providers.tsx @@ -34,8 +34,7 @@ import { import { StatusCard } from '@/components/ui/status-card'; import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; import { ProviderType, useDeleteProviderMutation, useSettingsProvidersQuery } from '@/graphql/types'; -import { usePagination } from '@/hooks/use-pagination'; -import { useTableQueryFilter } from '@/hooks/use-table-query-filter'; +import { useTableState } from '@/hooks/use-table-state'; type Provider = ProviderConfigFragmentFragment; const providerIcons: Record> = { @@ -134,8 +133,7 @@ const SettingsProviders = () => { const [deletingProvider, setDeletingProvider] = useState(null); const navigate = useNavigate(); - const { pageIndex: currentPage, setPage: handlePageChange } = usePagination(); - const { filter, setFilter } = useTableQueryFilter(); + const { filter, pageIndex: currentPage, setFilter, setPage: handlePageChange } = useTableState(); const handleProviderDelete = useCallback( async (providerId: string | undefined) => { diff --git a/frontend/src/pages/templates/templates.tsx b/frontend/src/pages/templates/templates.tsx index de9b4786..b95d4716 100644 --- a/frontend/src/pages/templates/templates.tsx +++ b/frontend/src/pages/templates/templates.tsx @@ -22,7 +22,7 @@ import { import { Separator } from '@/components/ui/separator'; import { SidebarTrigger } from '@/components/ui/sidebar'; import { StatusCard } from '@/components/ui/status-card'; -import { useTableQueryFilter } from '@/hooks/use-table-query-filter'; +import { useTableState } from '@/hooks/use-table-state'; import { mergeHrefWithSearchParams } from '@/lib/url-params'; import { type Template, useTemplates } from '@/providers/templates-provider'; @@ -37,7 +37,7 @@ const Templates = () => { const [isRenameLoading, setIsRenameLoading] = useState(false); const editingInputRef = useRef(null); - const { filter, setFilter } = useTableQueryFilter(); + const { filter, setFilter } = useTableState(); const handleTemplateOpen = useCallback( (templateId: string) => {