Skip to content

Commit 37656de

Browse files
committed
improve: simplify and harden useUrlCollectionState
- Remove params from write effect deps using function updater to eliminate feedback loop (setParams → params change → effect re-runs) - Fix filter value encoding: use JSON for arrays to correctly handle values containing commas (e.g. "Smith, John") - Fix hydration race condition: introduce SyncPhase lifecycle (pending → hydrated → ready) to prevent writing stale defaults to URL before hydrated state propagates - Simplify decodeFilterValue: remove startsWith check, rely on JSON.parse + Array.isArray for correctness - Replace Array.from(next.keys()) with spread syntax
1 parent 32b7df4 commit 37656de

3 files changed

Lines changed: 445 additions & 0 deletions

File tree

Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
import { renderHook } from "@testing-library/react";
2+
import { describe, it, expect, vi, beforeEach } from "vitest";
3+
import type { CollectionControl } from "@/types/collection";
4+
import {
5+
useUrlCollectionState,
6+
encodeFilterValue,
7+
decodeFilterValue,
8+
} from "./use-url-collection-state";
9+
10+
// ---------------------------------------------------------------------------
11+
// Mock react-router's useSearchParams
12+
// ---------------------------------------------------------------------------
13+
let mockParams: URLSearchParams;
14+
const mockSetParams = vi.fn();
15+
16+
vi.mock("react-router", () => ({
17+
useSearchParams: () => [mockParams, mockSetParams],
18+
}));
19+
20+
/**
21+
* Helper: the hook now calls setParams with a function updater.
22+
* This helper resolves the updater against mockParams to get the resulting params.
23+
*/
24+
function resolveSetParamsCall(callIndex: number): URLSearchParams {
25+
const [updaterOrValue] = mockSetParams.mock.calls[callIndex];
26+
if (typeof updaterOrValue === "function") {
27+
const result = updaterOrValue(mockParams);
28+
return result instanceof URLSearchParams ? result : new URLSearchParams(result);
29+
}
30+
return updaterOrValue instanceof URLSearchParams
31+
? updaterOrValue
32+
: new URLSearchParams(updaterOrValue);
33+
}
34+
35+
function makeControl(overrides?: Partial<CollectionControl>): CollectionControl {
36+
return {
37+
filters: [],
38+
addFilter: vi.fn(),
39+
setFilters: vi.fn(),
40+
removeFilter: vi.fn(),
41+
clearFilters: vi.fn(),
42+
sortStates: [],
43+
setSort: vi.fn(),
44+
clearSort: vi.fn(),
45+
pageSize: 20,
46+
setPageSize: vi.fn(),
47+
goToNextPage: vi.fn(),
48+
goToPrevPage: vi.fn(),
49+
resetPage: vi.fn(),
50+
goToFirstPage: vi.fn(),
51+
goToLastPage: vi.fn(),
52+
getHasPrevPage: vi.fn(),
53+
getHasNextPage: vi.fn(),
54+
resetCount: 0,
55+
...overrides,
56+
};
57+
}
58+
59+
describe("useUrlCollectionState", () => {
60+
beforeEach(() => {
61+
mockParams = new URLSearchParams();
62+
mockSetParams.mockClear();
63+
});
64+
65+
// ---------------------------------------------------------------------------
66+
// Hydration from URL
67+
// ---------------------------------------------------------------------------
68+
describe("hydration from URL", () => {
69+
it("hydrates pageSize from URL param", () => {
70+
mockParams = new URLSearchParams("p=50");
71+
const control = makeControl();
72+
renderHook(() => useUrlCollectionState(control));
73+
expect(control.setPageSize).toHaveBeenCalledWith(50);
74+
});
75+
76+
it("ignores invalid pageSize values", () => {
77+
mockParams = new URLSearchParams("p=abc");
78+
const control = makeControl();
79+
renderHook(() => useUrlCollectionState(control));
80+
expect(control.setPageSize).not.toHaveBeenCalled();
81+
});
82+
83+
it("ignores non-positive pageSize values", () => {
84+
mockParams = new URLSearchParams("p=-5");
85+
const control = makeControl();
86+
renderHook(() => useUrlCollectionState(control));
87+
expect(control.setPageSize).not.toHaveBeenCalled();
88+
});
89+
90+
it("hydrates sort from URL param (asc)", () => {
91+
mockParams = new URLSearchParams("s=name:asc");
92+
const control = makeControl();
93+
renderHook(() => useUrlCollectionState(control));
94+
expect(control.setSort).toHaveBeenCalledWith("name", "Asc");
95+
});
96+
97+
it("hydrates sort from URL param (desc)", () => {
98+
mockParams = new URLSearchParams("s=createdAt:desc");
99+
const control = makeControl();
100+
renderHook(() => useUrlCollectionState(control));
101+
expect(control.setSort).toHaveBeenCalledWith("createdAt", "Desc");
102+
});
103+
104+
it("hydrates filters from URL params", () => {
105+
mockParams = new URLSearchParams("f.status:eq=active&f.priority:gt=3");
106+
const control = makeControl();
107+
renderHook(() => useUrlCollectionState(control));
108+
expect(control.setFilters).toHaveBeenCalledWith([
109+
{ field: "status", operator: "eq", value: "active" },
110+
{ field: "priority", operator: "gt", value: "3" },
111+
]);
112+
});
113+
114+
it("hydrates array filter values (JSON format)", () => {
115+
mockParams = new URLSearchParams('f.status:in=["active","pending","closed"]');
116+
const control = makeControl();
117+
renderHook(() => useUrlCollectionState(control));
118+
expect(control.setFilters).toHaveBeenCalledWith([
119+
{
120+
field: "status",
121+
operator: "in",
122+
value: ["active", "pending", "closed"],
123+
},
124+
]);
125+
});
126+
127+
it("skips filters with missing value", () => {
128+
mockParams = new URLSearchParams("f.status:eq=");
129+
const control = makeControl();
130+
renderHook(() => useUrlCollectionState(control));
131+
expect(control.setFilters).not.toHaveBeenCalled();
132+
});
133+
134+
it("does not hydrate twice on re-render", () => {
135+
mockParams = new URLSearchParams("p=30");
136+
const control = makeControl();
137+
const { rerender } = renderHook(() => useUrlCollectionState(control));
138+
rerender();
139+
expect(control.setPageSize).toHaveBeenCalledTimes(1);
140+
});
141+
});
142+
143+
// ---------------------------------------------------------------------------
144+
// Writing state to URL
145+
// ---------------------------------------------------------------------------
146+
describe("writing state to URL", () => {
147+
it("writes pageSize to URL when control state changes", () => {
148+
// Start with default, then simulate a user changing pageSize
149+
const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), {
150+
initialProps: { control: makeControl({ pageSize: 20 }) },
151+
});
152+
// First render: write effect is skipped (skipWriteRef).
153+
// Simulate user changing pageSize → triggers dep change → write effect fires.
154+
rerender({ control: makeControl({ pageSize: 25 }) });
155+
const nextParams = resolveSetParamsCall(mockSetParams.mock.calls.length - 1);
156+
expect(nextParams.get("p")).toBe("25");
157+
});
158+
159+
it("writes sort to URL when control state changes", () => {
160+
const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), {
161+
initialProps: { control: makeControl() },
162+
});
163+
rerender({
164+
control: makeControl({
165+
sortStates: [{ field: "name", direction: "Desc" }],
166+
}),
167+
});
168+
const nextParams = resolveSetParamsCall(mockSetParams.mock.calls.length - 1);
169+
expect(nextParams.get("s")).toBe("name:desc");
170+
});
171+
172+
it("writes filters to URL when control state changes", () => {
173+
const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), {
174+
initialProps: { control: makeControl() },
175+
});
176+
rerender({
177+
control: makeControl({
178+
filters: [{ field: "status", operator: "eq" as never, value: "active" }],
179+
}),
180+
});
181+
const nextParams = resolveSetParamsCall(mockSetParams.mock.calls.length - 1);
182+
expect(nextParams.get("f.status:eq")).toBe("active");
183+
});
184+
185+
it("returns prev params unchanged when URL is already in sync", () => {
186+
mockParams = new URLSearchParams("p=20");
187+
const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), {
188+
initialProps: { control: makeControl({ pageSize: 20 }) },
189+
});
190+
// Change a non-relevant field to trigger the effect
191+
rerender({
192+
control: makeControl({
193+
pageSize: 20,
194+
sortStates: [],
195+
filters: [],
196+
}),
197+
});
198+
const lastCallIndex = mockSetParams.mock.calls.length - 1;
199+
const [updater] = mockSetParams.mock.calls[lastCallIndex];
200+
if (typeof updater === "function") {
201+
const result = updater(mockParams);
202+
// When no change, it should return the original prev instance
203+
expect(result).toBe(mockParams);
204+
}
205+
});
206+
207+
it("uses replace: true for setParams", () => {
208+
const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), {
209+
initialProps: { control: makeControl({ pageSize: 20 }) },
210+
});
211+
rerender({ control: makeControl({ pageSize: 30 }) });
212+
const lastCallIndex = mockSetParams.mock.calls.length - 1;
213+
const [, options] = mockSetParams.mock.calls[lastCallIndex];
214+
expect(options).toEqual({ replace: true });
215+
});
216+
});
217+
});
218+
219+
// ---------------------------------------------------------------------------
220+
// Utility functions
221+
// ---------------------------------------------------------------------------
222+
describe("encodeFilterValue", () => {
223+
it("encodes strings", () => {
224+
expect(encodeFilterValue("hello")).toBe("hello");
225+
});
226+
227+
it("encodes numbers", () => {
228+
expect(encodeFilterValue(42)).toBe("42");
229+
});
230+
231+
it("encodes booleans", () => {
232+
expect(encodeFilterValue(true)).toBe("true");
233+
});
234+
235+
it("encodes arrays as JSON to avoid comma ambiguity", () => {
236+
expect(encodeFilterValue(["a", "b", "c"])).toBe('["a","b","c"]');
237+
});
238+
239+
it("preserves values containing commas in arrays", () => {
240+
expect(encodeFilterValue(["Smith, John", "Doe, Jane"])).toBe('["Smith, John","Doe, Jane"]');
241+
});
242+
243+
it("encodes null as empty string", () => {
244+
expect(encodeFilterValue(null)).toBe("");
245+
});
246+
247+
it("encodes undefined as empty string", () => {
248+
expect(encodeFilterValue(undefined)).toBe("");
249+
});
250+
251+
it("encodes objects as JSON", () => {
252+
expect(encodeFilterValue({ foo: "bar" })).toBe('{"foo":"bar"}');
253+
});
254+
});
255+
256+
describe("decodeFilterValue", () => {
257+
it("decodes JSON arrays", () => {
258+
expect(decodeFilterValue('["a","b","c"]')).toEqual(["a", "b", "c"]);
259+
});
260+
261+
it("decodes JSON arrays with values containing commas", () => {
262+
expect(decodeFilterValue('["Smith, John","Doe, Jane"]')).toEqual(["Smith, John", "Doe, Jane"]);
263+
});
264+
265+
it("returns plain string for non-array values", () => {
266+
expect(decodeFilterValue("hello")).toBe("hello");
267+
});
268+
269+
it("returns plain string with commas as-is (not split)", () => {
270+
expect(decodeFilterValue("Smith, John")).toBe("Smith, John");
271+
});
272+
273+
it("returns plain string for malformed JSON", () => {
274+
expect(decodeFilterValue("[not valid json")).toBe("[not valid json");
275+
expect(decodeFilterValue("{broken")).toBe("{broken");
276+
});
277+
});

0 commit comments

Comments
 (0)