letsbe/pn-new-crm
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(audit): H15 (saved-view sort) + H14 (back/forward URL resync) in usePaginatedQuery

Browse Source 
H15: new applyView({filters,sort}) atomic mutator (one URL write) restores a
saved view's sort, threaded through all six list components instead of being
discarded. H14: a guarded effect resyncs page/sort/filters FROM the URL on
Back/Forward; the resync setStates carry a scoped, justified
set-state-in-effect disable (loop-guarded external-URL sync).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Matt
2026-06-02 12:26:10 +02:00
parent 808e80744b
commit 29fb882478
7 changed files with 152 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/components/berths/berth-list.tsx
View File

@@ -85,7 +85,7 @@ export function BerthList() {
setSort,
filters,
setFilter,
setAllFilters,
applyView,
clearFilters,
setPage,
setPageSize,
@@ -183,8 +183,8 @@ export function BerthList() {
<div className="flex items-center gap-2 ml-auto">
<SavedViewsDropdown
entityType="berths"
onApplyView={(savedFilters, _savedSort) => {
setAllFilters(savedFilters);
onApplyView={(savedFilters, savedSort) => {
applyView({ filters: savedFilters, sort: savedSort });
}}
/>
<Button

14
src/components/clients/client-list.tsx
View File

@@ -115,7 +115,7 @@ export function ClientList() {
setPageSize,
filters,
setFilter,
setAllFilters,
applyView,
clearFilters,
} = usePaginatedQuery<ClientRow>({
queryKey: ['clients'],
@@ -189,12 +189,12 @@ export function ClientList() {
/>
<SavedViewsDropdown
entityType="clients"
onApplyView={(savedFilters, _savedSort) => {
// Atomic replace - sequential setFilter() calls dropped all
// but the last value (each one read stale `filters` from
// closure and overwrote). setAllFilters writes the whole
// saved view in one setState.
setAllFilters(savedFilters);
onApplyView={(savedFilters, savedSort) => {
// Atomic replace of filters AND sort in one URL write. Passing
// both args fixes H15 (the saved sort was previously dropped);
// applyView also avoids the two-write race that a setAllFilters
// + setSort pair would hit.
applyView({ filters: savedFilters, sort: savedSort });
}}
/>
<ColumnPicker

6
src/components/companies/company-list.tsx
View File

@@ -97,7 +97,7 @@ export function CompanyList() {
setPageSize,
filters,
setFilter,
setAllFilters,
applyView,
clearFilters,
} = usePaginatedQuery<CompanyRow>({
queryKey: ['companies'],
@@ -155,8 +155,8 @@ export function CompanyList() {
<div className="ml-auto flex items-center gap-2">
<SavedViewsDropdown
entityType="companies"
onApplyView={(savedFilters, _savedSort) => {
setAllFilters(savedFilters);
onApplyView={(savedFilters, savedSort) => {
applyView({ filters: savedFilters, sort: savedSort });
}}
/>
<ColumnPicker columns={COMPANY_COLUMN_OPTIONS} hidden={hidden} onChange={setHidden} />

6
src/components/interests/interest-list.tsx
View File

@@ -110,7 +110,7 @@ export function InterestList() {
setPageSize,
filters,
setFilter,
setAllFilters,
applyView,
clearFilters,
} = usePaginatedQuery<InterestRow>({
queryKey: ['interests'],
@@ -266,8 +266,8 @@ export function InterestList() {
<>
<SavedViewsDropdown
entityType="interests"
onApplyView={(savedFilters) => {
setAllFilters(savedFilters);
onApplyView={(savedFilters, savedSort) => {
applyView({ filters: savedFilters, sort: savedSort });
}}
/>
<ColumnPicker

6
src/components/residential/residential-interests-list.tsx
View File

@@ -83,7 +83,7 @@ export function ResidentialInterestsList() {
setPageSize,
filters,
setFilter,
setAllFilters,
applyView,
clearFilters,
} = usePaginatedQuery<ResidentialInterestRow>({
queryKey: ['residential-interests'],
@@ -193,7 +193,9 @@ export function ResidentialInterestsList() {
<div className="ml-auto flex flex-wrap items-center gap-2">
<SavedViewsDropdown
entityType="residential_interests"
onApplyView={(savedFilters) => setAllFilters(savedFilters)}
onApplyView={(savedFilters, savedSort) =>
applyView({ filters: savedFilters, sort: savedSort })
}
/>
<ColumnPicker
columns={RESIDENTIAL_INTEREST_COLUMN_OPTIONS}

6
src/components/yachts/yacht-list.tsx
View File

@@ -99,7 +99,7 @@ export function YachtList() {
setPageSize,
filters,
setFilter,
setAllFilters,
applyView,
clearFilters,
} = usePaginatedQuery<YachtRow>({
queryKey: ['yachts'],
@@ -153,8 +153,8 @@ export function YachtList() {
/>
<SavedViewsDropdown
entityType="yachts"
onApplyView={(savedFilters, _savedSort) => {
setAllFilters(savedFilters);
onApplyView={(savedFilters, savedSort) => {
applyView({ filters: savedFilters, sort: savedSort });
}}
/>
</div>

135
src/hooks/use-paginated-query.ts
View File

@@ -1,6 +1,6 @@
'use client';
import { useState, useCallback, useMemo } from 'react';
import { useState, useCallback, useMemo, useEffect, useRef } from 'react';
import { useQuery, useQueryClient, type QueryKey } from '@tanstack/react-query';
import { useSearchParams, useRouter, usePathname } from 'next/navigation';
@@ -55,9 +55,13 @@ export function usePaginatedQuery<T>({
deserializeFiltersFromParams(searchParams, filterDefinitions),
);
// Sync state to URL
const syncUrl = useCallback(
(p: number, ps: number, s?: typeof sort, f?: FilterValues) => {
// Canonical query string for a given state. Sorted keys so two URLs
// that carry the same params in a different order compare equal - this
// is what lets the back/forward resync effect (H14) tell "the URL the
// browser just restored" apart from "the URL we last wrote ourselves"
// without thrashing on key ordering.
const buildCanonicalQs = useCallback(
(p: number, ps: number, s?: typeof sort, f?: FilterValues): string => {
const params = new URLSearchParams();
if (p !== 1) params.set('page', String(p));
if (ps !== initialPageSize) params.set('limit', String(ps));
@@ -72,12 +76,26 @@ export function usePaginatedQuery<T>({
// Keep existing tab param
const tab = searchParams.get('tab');
if (tab) params.set('tab', tab);
params.sort();
return params.toString();
},
[searchParams, initialPageSize],
);
const qs = params.toString();
// Records the query string we last pushed via `router.replace`, so the
// resync effect (H14) can skip params it itself authored and only react
// to genuinely-external URL changes (Back/Forward).
const lastWrittenQsRef = useRef<string | null>(null);
// Sync state to URL
const syncUrl = useCallback(
(p: number, ps: number, s?: typeof sort, f?: FilterValues) => {
const qs = buildCanonicalQs(p, ps, s, f);
lastWrittenQsRef.current = qs;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
router.replace(`${pathname}${qs ? `?${qs}` : ''}` as any, { scroll: false });
},
[pathname, router, searchParams, initialPageSize],
[pathname, router, buildCanonicalQs],
);
function setPage(p: number) {
@@ -130,6 +148,110 @@ export function usePaginatedQuery<T>({
syncUrl(1, pageSize, sort, next);
}
/**
* Atomically apply a saved view's filters AND sort in a single URL
* write. The saved-views apply path previously called `setAllFilters`
* only, silently dropping the view's stored sort (H15). Splitting the
* write into `setAllFilters` + `setSort` would also race: each reads a
* stale closure and the second `syncUrl` clobbers the first. `applyView`
* sets both pieces of state and emits ONE `syncUrl`, so the view lands
* intact with its sort.
*
* `sort` is optional - a view saved without an explicit sort clears the
* active sort back to the list's `initialSort` (handled by the consumer
* passing `undefined`). The incoming `direction` is the loose `string`
* shape from `SavedViewsDropdown`; we narrow non-'asc' to 'desc'.
*/
function applyView({
filters: nextFilters,
sort: nextSort,
}: {
filters: FilterValues;
sort?: { field: string; direction: string } | undefined;
}) {
const normalizedSort: { field: string; direction: 'asc' | 'desc' } | undefined = nextSort
? {
field: nextSort.field,
direction: nextSort.direction === 'asc' ? ('asc' as const) : ('desc' as const),
}
: initialSort;
setFiltersState(nextFilters);
setSortState(normalizedSort);
setPageState(1);
syncUrl(1, pageSize, normalizedSort, nextFilters);
}
/**
* H14 - resync state FROM the URL on external navigation (Back/Forward).
*
* The slices above seed from the URL once (useState initializers) then
* drive it one-way via `syncUrl` -> `router.replace`. Nothing pulled the
* URL back into state, so Back/Forward moved the address bar but left the
* list showing the previous page/sort/filters.
*
* Loop safety: every write we make records its canonical query string in
* `lastWrittenQsRef`. Here we re-derive the canonical form of the params
* the router is currently handing us and compare it against that ref. If
* they match, this render was caused by our OWN write - bail before
* touching state. We also compare against the canonical form of CURRENT
* state and only call a setter for a slice that actually differs, so even
* an external change that happens to equal current state is a no-op. Both
* guards mean this effect cannot feed itself: it never calls `syncUrl`,
* and it never setState's a value equal to what state already holds.
*/
useEffect(() => {
const incomingQs = buildCanonicalQs(
Number(searchParams.get('page')) || initialPage,
Number(searchParams.get('limit')) || initialPageSize,
searchParams.get('sort')
? {
field: searchParams.get('sort') as string,
direction: (searchParams.get('order') as 'asc' | 'desc') ?? 'desc',
}
: initialSort,
deserializeFiltersFromParams(searchParams, filterDefinitions),
);
// This params object is the one we just wrote - not an external nav.
if (incomingQs === lastWrittenQsRef.current) return;
// The state already matches the URL - nothing to do (also prevents a
// redundant setState that could schedule an extra render).
const currentQs = buildCanonicalQs(page, pageSize, sort, filters);
if (incomingQs === currentQs) return;
// Genuine external change (Back/Forward): pull each slice from the URL,
// setting only the ones that drifted.
const nextPage = Number(searchParams.get('page')) || initialPage;
const nextPageSize = Number(searchParams.get('limit')) || initialPageSize;
const nextSortField = searchParams.get('sort');
const nextSort = nextSortField
? {
field: nextSortField,
direction: (searchParams.get('order') as 'asc' | 'desc') ?? 'desc',
}
: initialSort;
const nextFilters = deserializeFiltersFromParams(searchParams, filterDefinitions);
/* eslint-disable react-hooks/set-state-in-effect --
Deliberate, guarded URL->state sync on Back/Forward: the URL is the
external source of truth here. The two early-returns above ensure this
runs ONLY on a genuine external navigation (never on our own write), and
each setState is diff-guarded, so there is no cascading-render loop
(audit H14). This is the documented external-store-subscription case the
rule otherwise blocks. */
if (nextPage !== page) setPageState(nextPage);
if (nextPageSize !== pageSize) setPageSizeState(nextPageSize);
if (nextSort?.field !== sort?.field || nextSort?.direction !== sort?.direction) {
setSortState(nextSort);
}
if (JSON.stringify(nextFilters) !== JSON.stringify(filters)) {
setFiltersState(nextFilters);
}
/* eslint-enable react-hooks/set-state-in-effect */
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [searchParams]);
// Build query string for API
const apiParams = useMemo(() => {
const params = new URLSearchParams();
@@ -196,6 +318,7 @@ export function usePaginatedQuery<T>({
filters,
setFilter,
setAllFilters,
applyView,
clearFilters,
optimisticRemove,
};