From e11cce3ad28bec3e0656e98da7d00a80810da30d Mon Sep 17 00:00:00 2001 From: Alaister Young <10985857+alaister@users.noreply.github.com> Date: Wed, 30 Sep 2026 07:24:25 -0700 Subject: [PATCH] fix(studio): preserve forward history during unchanged filter sync --- ...TableEditorFiltersSort.navigation.test.tsx | 161 ++++++++++++++++++ .../useTableEditorFiltersSort.test.ts | 42 ++++- .../hooks/misc/useTableEditorFiltersSort.ts | 10 +- 3 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.navigation.test.tsx diff --git a/apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.navigation.test.tsx b/apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.navigation.test.tsx new file mode 100644 index 00000000000..cc9630d40b2 --- /dev/null +++ b/apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.navigation.test.tsx @@ -0,0 +1,161 @@ +import { + createBrowserHistory, + createRootRoute, + createRoute, + createRouter, + Outlet, + RouterProvider, + useParams, + type AnyRouter, +} from '@tanstack/react-router' +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { useMemo } from 'react' +import { describe, expect, it, vi } from 'vitest' + +import Link from '@/compat/next/link' +import { useSyncFiltersToUrl } from '@/components/grid/hooks/useFilterLifeCycle' +import { ENTITY_TYPE } from '@/data/entity-types/entity-type-constants' +import { parseSearch, stringifySearch } from '@/lib/router-search-params' +import { + createTableEditorTableState, + TableEditorTableStateContext, +} from '@/state/table-editor-table' + +vi.mock('next/router', () => import('@/compat/next/router')) + +function FilterSync() { + useSyncFiltersToUrl() + return null +} + +function TablePage() { + const { id } = useParams({ strict: false }) + const state = useMemo( + () => + createTableEditorTableState({ + projectRef: 'default', + table: { + id: Number(id), + schema: 'public', + name: `table_${id}`, + entity_type: ENTITY_TYPE.FOREIGN_TABLE, + comment: null, + columns: [], + foreign_server_name: 'fixture', + foreign_data_wrapper_name: 'fixture', + foreign_data_wrapper_handler: 'fixture', + }, + onAddColumn: vi.fn(), + onExpandJSONEditor: vi.fn(), + onExpandTextEditor: vi.fn(), + }), + [id] + ) + + return ( + + + {id} + Table B + Table A + + + + ) +} + +// Allow the production filter synchronizer's 500ms debounce to finish before traversing. +const settleFilters = () => act(async () => new Promise((resolve) => setTimeout(resolve, 650))) + +for (const basepath of ['/', '/dashboard']) { + describe(`table filter history at ${basepath}`, () => { + it.each([false, true])( + 'preserves repeated Back/Forward with retained application state: %s', + async (hasApplicationState) => { + const prefix = basepath === '/' ? '' : basepath + const tableA = `${prefix}/project/default/editor/17489?schema=public&extra=kept#selection` + const tableB = `${prefix}/project/default/editor/17607?schema=public&extra=kept#selection` + window.history.replaceState(null, '', tableA) + const history = createBrowserHistory() + const root = createRootRoute({ component: Outlet }) + const table = createRoute({ + getParentRoute: () => root, + path: '/project/$ref/editor/$id', + component: TablePage, + }) + const router = createRouter({ + routeTree: root.addChildren([table]), + history, + basepath, + parseSearch, + stringifySearch, + isServer: false, + }) + await router.load() + const view = render() + const push = vi.spyOn(window.history, 'pushState') + + try { + await settleFilters() + if (hasApplicationState) { + const retainedState = { ...history.location.state, legacyState: 'retained' } + await act(async () => history.replace(tableA, retainedState)) + } + fireEvent.click(screen.getByText('Table B')) + await waitFor(() => expect(screen.getByTestId('table')).toHaveTextContent('17607')) + await settleFilters() + + for (let cycle = 0; cycle < 2; cycle++) { + push.mockClear() + act(() => window.history.back()) + await waitFor(() => expect(screen.getByTestId('table')).toHaveTextContent('17489')) + await settleFilters() + expect(window.location.pathname + window.location.search + window.location.hash).toBe( + tableA + ) + expect(push).not.toHaveBeenCalled() + if (hasApplicationState) expect(window.history.state.legacyState).toBe('retained') + + act(() => window.history.forward()) + await waitFor(() => expect(screen.getByTestId('table')).toHaveTextContent('17607')) + await settleFilters() + expect(window.location.pathname + window.location.search + window.location.hash).toBe( + tableB + ) + } + + fireEvent.click(screen.getByText('Filter rows')) + await waitFor(() => expect(window.location.search).toContain('filter=id%3Aeq%3A1')) + expect(window.location.search).toContain('schema=public') + expect(window.location.search).toContain('extra=kept') + expect(window.location.hash).toBe('#selection') + + // Leaving before the next debounce must cancel the old table's pending URL update. + fireEvent.click(screen.getByText('Change filter')) + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))) + fireEvent.click(screen.getByText('Table A')) + await waitFor(() => expect(screen.getByTestId('table')).toHaveTextContent('17489')) + await settleFilters() + expect(window.location.pathname + window.location.search + window.location.hash).toBe( + tableA + ) + } finally { + view.unmount() + push.mockRestore() + history.destroy() + } + }, + 10000 + ) + }) +} diff --git a/apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.test.ts b/apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.test.ts index f32de0395bc..a0ebb9225a1 100644 --- a/apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.test.ts +++ b/apps/studio/hooks/misc/__tests__/useTableEditorFiltersSort.test.ts @@ -1,4 +1,5 @@ -import { renderHook } from '@testing-library/react' +import { act, renderHook } from '@testing-library/react' +import mockRouter from 'next-router-mock' import { useRouter } from 'next/router' import { withNuqsTestingAdapter } from 'nuqs/adapters/testing' import { describe, expect, it, vi } from 'vitest' @@ -61,4 +62,43 @@ describe('useTableEditorFilters', () => { expect(result.current.sorts).toEqual([]) }) + + it('keeps unchanged filters and sorts out of Next navigation history', async () => { + const router = mockRouter + await act(async () => { + await router.push('/test?schema=public&filter=id:eq:1&sort=id:asc#selection') + }) + const { result } = renderHook(() => useTableEditorFiltersSort()) + const onNavigation = vi.fn() + router.events.on('routeChangeStart', onNavigation) + try { + act(() => result.current.setParams((prev) => ({ ...prev }))) + act(() => result.current.setParams(() => ({ filter: ['id:eq:1'] }))) + act(() => result.current.setParams(() => ({ sort: ['id:asc'] }))) + expect(onNavigation).not.toHaveBeenCalled() + + await act(async () => result.current.setParams(() => ({ filter: ['id:eq:2'] }))) + expect(onNavigation).toHaveBeenCalledTimes(1) + expect(router.asPath).toBe('/test?schema=public&filter=id%3Aeq%3A2&sort=id%3Aasc#selection') + + await act(async () => + result.current.setParams(() => ({ sort: ['id:desc', 'created_at:asc'] })) + ) + expect(onNavigation).toHaveBeenCalledTimes(2) + expect(result.current.filters).toEqual(['id:eq:2']) + expect(result.current.sorts).toEqual(['id:desc', 'created_at:asc']) + + await act(async () => + result.current.setParams(() => ({ sort: ['created_at:asc', 'id:desc'] })) + ) + expect(onNavigation).toHaveBeenCalledTimes(3) + expect(result.current.sorts).toEqual(['created_at:asc', 'id:desc']) + + await act(async () => result.current.setParams(() => ({ filter: [], sort: [] }))) + expect(onNavigation).toHaveBeenCalledTimes(4) + expect(router.asPath).toBe('/test?schema=public#selection') + } finally { + router.events.off('routeChangeStart', onNavigation) + } + }) }) diff --git a/apps/studio/hooks/misc/useTableEditorFiltersSort.ts b/apps/studio/hooks/misc/useTableEditorFiltersSort.ts index 5851a8e4d90..728a19c7ab0 100644 --- a/apps/studio/hooks/misc/useTableEditorFiltersSort.ts +++ b/apps/studio/hooks/misc/useTableEditorFiltersSort.ts @@ -1,3 +1,4 @@ +import { isEqual } from 'lodash' import { useRouter } from 'next/router' import { useCallback, useMemo } from 'react' @@ -5,7 +6,7 @@ export const useTableEditorFiltersSort = () => { const router = useRouter() const urlParams = useMemo(() => { - return new URLSearchParams(router.asPath.split('?')[1]) + return new URLSearchParams(router.asPath.split('?')[1]?.split('#')[0]) }, [router.asPath]) const filters = useMemo(() => { @@ -28,9 +29,16 @@ export const useTableEditorFiltersSort = () => { const hasFilter = newParams.filter !== undefined const hasSort = newParams.sort !== undefined + const areFiltersUnchanged = !hasFilter || isEqual(newParams.filter, filters) + const areSortsUnchanged = !hasSort || isEqual(newParams.sort, sorts) + // A no-op URL sync must not create an entry that discards browser Forward history. + if (areFiltersUnchanged && areSortsUnchanged) return + + const hashStart = router.asPath.indexOf('#') router.push( { + hash: hashStart === -1 ? undefined : router.asPath.slice(hashStart), query: { ...router.query, ...(hasFilter ? { filter: newParams.filter } : {}),