From 37656decb784ed3a577924a5e93bd2026cb8480e Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Wed, 27 May 2026 16:44:54 +0900 Subject: [PATCH] improve: simplify and harden useUrlCollectionState MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../hooks/use-url-collection-state.test.ts | 277 ++++++++++++++++++ .../src/hooks/use-url-collection-state.ts | 167 +++++++++++ packages/core/src/index.ts | 1 + 3 files changed, 445 insertions(+) create mode 100644 packages/core/src/hooks/use-url-collection-state.test.ts create mode 100644 packages/core/src/hooks/use-url-collection-state.ts diff --git a/packages/core/src/hooks/use-url-collection-state.test.ts b/packages/core/src/hooks/use-url-collection-state.test.ts new file mode 100644 index 00000000..b1ddced4 --- /dev/null +++ b/packages/core/src/hooks/use-url-collection-state.test.ts @@ -0,0 +1,277 @@ +import { renderHook } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import type { CollectionControl } from "@/types/collection"; +import { + useUrlCollectionState, + encodeFilterValue, + decodeFilterValue, +} from "./use-url-collection-state"; + +// --------------------------------------------------------------------------- +// Mock react-router's useSearchParams +// --------------------------------------------------------------------------- +let mockParams: URLSearchParams; +const mockSetParams = vi.fn(); + +vi.mock("react-router", () => ({ + useSearchParams: () => [mockParams, mockSetParams], +})); + +/** + * Helper: the hook now calls setParams with a function updater. + * This helper resolves the updater against mockParams to get the resulting params. + */ +function resolveSetParamsCall(callIndex: number): URLSearchParams { + const [updaterOrValue] = mockSetParams.mock.calls[callIndex]; + if (typeof updaterOrValue === "function") { + const result = updaterOrValue(mockParams); + return result instanceof URLSearchParams ? result : new URLSearchParams(result); + } + return updaterOrValue instanceof URLSearchParams + ? updaterOrValue + : new URLSearchParams(updaterOrValue); +} + +function makeControl(overrides?: Partial): CollectionControl { + return { + filters: [], + addFilter: vi.fn(), + setFilters: vi.fn(), + removeFilter: vi.fn(), + clearFilters: vi.fn(), + sortStates: [], + setSort: vi.fn(), + clearSort: vi.fn(), + pageSize: 20, + setPageSize: vi.fn(), + goToNextPage: vi.fn(), + goToPrevPage: vi.fn(), + resetPage: vi.fn(), + goToFirstPage: vi.fn(), + goToLastPage: vi.fn(), + getHasPrevPage: vi.fn(), + getHasNextPage: vi.fn(), + resetCount: 0, + ...overrides, + }; +} + +describe("useUrlCollectionState", () => { + beforeEach(() => { + mockParams = new URLSearchParams(); + mockSetParams.mockClear(); + }); + + // --------------------------------------------------------------------------- + // Hydration from URL + // --------------------------------------------------------------------------- + describe("hydration from URL", () => { + it("hydrates pageSize from URL param", () => { + mockParams = new URLSearchParams("p=50"); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setPageSize).toHaveBeenCalledWith(50); + }); + + it("ignores invalid pageSize values", () => { + mockParams = new URLSearchParams("p=abc"); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setPageSize).not.toHaveBeenCalled(); + }); + + it("ignores non-positive pageSize values", () => { + mockParams = new URLSearchParams("p=-5"); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setPageSize).not.toHaveBeenCalled(); + }); + + it("hydrates sort from URL param (asc)", () => { + mockParams = new URLSearchParams("s=name:asc"); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setSort).toHaveBeenCalledWith("name", "Asc"); + }); + + it("hydrates sort from URL param (desc)", () => { + mockParams = new URLSearchParams("s=createdAt:desc"); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setSort).toHaveBeenCalledWith("createdAt", "Desc"); + }); + + it("hydrates filters from URL params", () => { + mockParams = new URLSearchParams("f.status:eq=active&f.priority:gt=3"); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setFilters).toHaveBeenCalledWith([ + { field: "status", operator: "eq", value: "active" }, + { field: "priority", operator: "gt", value: "3" }, + ]); + }); + + it("hydrates array filter values (JSON format)", () => { + mockParams = new URLSearchParams('f.status:in=["active","pending","closed"]'); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setFilters).toHaveBeenCalledWith([ + { + field: "status", + operator: "in", + value: ["active", "pending", "closed"], + }, + ]); + }); + + it("skips filters with missing value", () => { + mockParams = new URLSearchParams("f.status:eq="); + const control = makeControl(); + renderHook(() => useUrlCollectionState(control)); + expect(control.setFilters).not.toHaveBeenCalled(); + }); + + it("does not hydrate twice on re-render", () => { + mockParams = new URLSearchParams("p=30"); + const control = makeControl(); + const { rerender } = renderHook(() => useUrlCollectionState(control)); + rerender(); + expect(control.setPageSize).toHaveBeenCalledTimes(1); + }); + }); + + // --------------------------------------------------------------------------- + // Writing state to URL + // --------------------------------------------------------------------------- + describe("writing state to URL", () => { + it("writes pageSize to URL when control state changes", () => { + // Start with default, then simulate a user changing pageSize + const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), { + initialProps: { control: makeControl({ pageSize: 20 }) }, + }); + // First render: write effect is skipped (skipWriteRef). + // Simulate user changing pageSize → triggers dep change → write effect fires. + rerender({ control: makeControl({ pageSize: 25 }) }); + const nextParams = resolveSetParamsCall(mockSetParams.mock.calls.length - 1); + expect(nextParams.get("p")).toBe("25"); + }); + + it("writes sort to URL when control state changes", () => { + const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), { + initialProps: { control: makeControl() }, + }); + rerender({ + control: makeControl({ + sortStates: [{ field: "name", direction: "Desc" }], + }), + }); + const nextParams = resolveSetParamsCall(mockSetParams.mock.calls.length - 1); + expect(nextParams.get("s")).toBe("name:desc"); + }); + + it("writes filters to URL when control state changes", () => { + const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), { + initialProps: { control: makeControl() }, + }); + rerender({ + control: makeControl({ + filters: [{ field: "status", operator: "eq" as never, value: "active" }], + }), + }); + const nextParams = resolveSetParamsCall(mockSetParams.mock.calls.length - 1); + expect(nextParams.get("f.status:eq")).toBe("active"); + }); + + it("returns prev params unchanged when URL is already in sync", () => { + mockParams = new URLSearchParams("p=20"); + const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), { + initialProps: { control: makeControl({ pageSize: 20 }) }, + }); + // Change a non-relevant field to trigger the effect + rerender({ + control: makeControl({ + pageSize: 20, + sortStates: [], + filters: [], + }), + }); + const lastCallIndex = mockSetParams.mock.calls.length - 1; + const [updater] = mockSetParams.mock.calls[lastCallIndex]; + if (typeof updater === "function") { + const result = updater(mockParams); + // When no change, it should return the original prev instance + expect(result).toBe(mockParams); + } + }); + + it("uses replace: true for setParams", () => { + const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), { + initialProps: { control: makeControl({ pageSize: 20 }) }, + }); + rerender({ control: makeControl({ pageSize: 30 }) }); + const lastCallIndex = mockSetParams.mock.calls.length - 1; + const [, options] = mockSetParams.mock.calls[lastCallIndex]; + expect(options).toEqual({ replace: true }); + }); + }); +}); + +// --------------------------------------------------------------------------- +// Utility functions +// --------------------------------------------------------------------------- +describe("encodeFilterValue", () => { + it("encodes strings", () => { + expect(encodeFilterValue("hello")).toBe("hello"); + }); + + it("encodes numbers", () => { + expect(encodeFilterValue(42)).toBe("42"); + }); + + it("encodes booleans", () => { + expect(encodeFilterValue(true)).toBe("true"); + }); + + it("encodes arrays as JSON to avoid comma ambiguity", () => { + expect(encodeFilterValue(["a", "b", "c"])).toBe('["a","b","c"]'); + }); + + it("preserves values containing commas in arrays", () => { + expect(encodeFilterValue(["Smith, John", "Doe, Jane"])).toBe('["Smith, John","Doe, Jane"]'); + }); + + it("encodes null as empty string", () => { + expect(encodeFilterValue(null)).toBe(""); + }); + + it("encodes undefined as empty string", () => { + expect(encodeFilterValue(undefined)).toBe(""); + }); + + it("encodes objects as JSON", () => { + expect(encodeFilterValue({ foo: "bar" })).toBe('{"foo":"bar"}'); + }); +}); + +describe("decodeFilterValue", () => { + it("decodes JSON arrays", () => { + expect(decodeFilterValue('["a","b","c"]')).toEqual(["a", "b", "c"]); + }); + + it("decodes JSON arrays with values containing commas", () => { + expect(decodeFilterValue('["Smith, John","Doe, Jane"]')).toEqual(["Smith, John", "Doe, Jane"]); + }); + + it("returns plain string for non-array values", () => { + expect(decodeFilterValue("hello")).toBe("hello"); + }); + + it("returns plain string with commas as-is (not split)", () => { + expect(decodeFilterValue("Smith, John")).toBe("Smith, John"); + }); + + it("returns plain string for malformed JSON", () => { + expect(decodeFilterValue("[not valid json")).toBe("[not valid json"); + expect(decodeFilterValue("{broken")).toBe("{broken"); + }); +}); diff --git a/packages/core/src/hooks/use-url-collection-state.ts b/packages/core/src/hooks/use-url-collection-state.ts new file mode 100644 index 00000000..2eb7d398 --- /dev/null +++ b/packages/core/src/hooks/use-url-collection-state.ts @@ -0,0 +1,167 @@ +import { useEffect, useRef } from "react"; +import { useSearchParams } from "react-router"; +import type { CollectionControl, Filter } from "@/types/collection"; + +const KEY_PAGE_SIZE = "p"; +const KEY_SORT = "s"; +const FILTER_PREFIX = "f."; + +/** + * Lifecycle phase of `useUrlCollectionState`. + */ +type SyncPhase = + /** Initial state. Hydration has not run yet. */ + | "pending" + /** Hydration complete. The first write effect should be skipped because + * control state set during hydration (via setState) won't be reflected + * until the next render. */ + | "hydrated" + /** Normal operation. The write effect actively syncs control state to the URL. */ + | "ready"; + +/** + * Persists CollectionControl state (filters, sort, page size) to the URL query + * string so pages are bookmarkable and the browser back button works. + * + * Designed to be entity-agnostic: keys are short and the operator/value + * encoding is derived from the current Filter shape, not hard-coded per field. + * + * Cursor/direction state is intentionally NOT persisted — `CollectionControl` + * manages cursor state internally and no longer exposes it publicly. We accept + * the regression that a refresh resets to page 1. + */ +export function useUrlCollectionState< + TFieldName extends string, + TFilter extends Filter, +>(control: CollectionControl): void { + const [params, setParams] = useSearchParams(); + const phaseRef = useRef("pending"); + + // Hydrate control from URL on first render. + useEffect(() => { + if (phaseRef.current !== "pending") return; + phaseRef.current = "hydrated"; + + const pageSize = params.get(KEY_PAGE_SIZE); + if (pageSize) { + const n = Number(pageSize); + if (Number.isFinite(n) && n > 0) control.setPageSize(n); + } + + const sort = params.get(KEY_SORT); + if (sort) { + const [field, rawDir] = sort.split(":"); + if (field) { + const direction = rawDir === "desc" ? "Desc" : "Asc"; + control.setSort(field as TFieldName, direction); + } + } + + const nextFilters: Filter[] = []; + for (const [key, value] of params.entries()) { + if (!key.startsWith(FILTER_PREFIX) || !value) continue; + const [field, operator] = key.slice(FILTER_PREFIX.length).split(":"); + if (!field || !operator) continue; + nextFilters.push({ + field: field as TFieldName, + operator, + value: decodeFilterValue(value), + } as Filter); + } + if (nextFilters.length > 0) { + control.setFilters(nextFilters); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + // Write control state back to URL whenever it changes. + // Uses the setParams function updater to avoid depending on `params` in the + // dependency array, which would cause a feedback loop: + // setParams → params change → effect re-runs → setParams (no-op but wasteful) + useEffect(() => { + if (phaseRef.current === "pending") return; + // Skip the first write cycle that fires immediately after hydration. + // Control state set during hydration (setPageSize, setSort, etc.) is async + // and won't be reflected until the next render. + if (phaseRef.current === "hydrated") { + phaseRef.current = "ready"; + return; + } + + setParams( + (prev) => { + const next = new URLSearchParams(prev); + + if (control.pageSize) { + next.set(KEY_PAGE_SIZE, String(control.pageSize)); + } else { + next.delete(KEY_PAGE_SIZE); + } + + if (control.sortStates.length > 0) { + const { field, direction } = control.sortStates[0]; + next.set(KEY_SORT, `${field}:${direction === "Desc" ? "desc" : "asc"}`); + } else { + next.delete(KEY_SORT); + } + + // Snapshot keys before iterating — we delete entries during the loop. + // eslint-disable-next-line unicorn/no-useless-spread + for (const key of [...next.keys()]) { + if (key.startsWith(FILTER_PREFIX)) next.delete(key); + } + for (const filter of control.filters) { + next.set( + `${FILTER_PREFIX}${filter.field}:${filter.operator}`, + encodeFilterValue(filter.value), + ); + } + + // Bail out if nothing changed — avoids a no-op navigation that could + // still trigger re-renders in some react-router versions. + if (next.toString() === prev.toString()) return prev; + return next; + }, + { replace: true }, + ); + }, [control.pageSize, control.sortStates, control.filters, setParams]); +} + +/** + * Encodes a filter value for URL storage. + * - Arrays are JSON-encoded to avoid ambiguity with values that contain commas. + * - Objects are JSON-encoded. + * - Primitives are stringified directly. + */ +export function encodeFilterValue(value: unknown): string { + // Arrays use JSON so that individual elements containing commas are preserved + // correctly during round-trip (encode → decode). + if (Array.isArray(value)) return JSON.stringify(value.map((v) => stringifyPrimitive(v))); + if (value == null) return ""; + if (typeof value === "object") return JSON.stringify(value); + return stringifyPrimitive(value); +} + +function stringifyPrimitive(value: unknown): string { + if (typeof value === "string") return value; + if (typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") { + return value.toString(); + } + if (value == null) return ""; + return JSON.stringify(value); +} + +/** + * Decodes a filter value from URL storage. + * - JSON arrays are parsed back into arrays. + * - All other values are returned as plain strings. + */ +export function decodeFilterValue(raw: string): unknown { + try { + const parsed: unknown = JSON.parse(raw); + if (Array.isArray(parsed)) return parsed; + } catch { + // Not valid JSON — fall through to plain string + } + return raw; +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6c26da3c..bd4bb961 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -183,6 +183,7 @@ export { type DataTableContextValue, } from "./components/data-table"; export { useCollectionVariables } from "./hooks/use-collection-variables"; +export { useUrlCollectionState } from "./hooks/use-url-collection-state"; export { CollectionControlProvider, useCollectionControl,