Skip to content

Commit c25039d

Browse files
interacseanclaude
authored andcommitted
fix: round-trip object-valued filters in useUrlCollectionState
decodeFilterValue only parsed JSON arrays back, so object-valued filters (the `between` operator's { min, max } shape) round-tripped to a raw string on reload and silently broke. Decode objects too; primitives like "5"/"true" still fall through to strings, preserving string-vs-numeric filter semantics. Also harden the write-effect bail-out: compare on a sorted, JSON-encoded snapshot (stableQueryString) rather than `.toString()`, which is sensitive to filter key-insertion order and to `&`/`=` inside values. Folds in fixes already running in the Denim Tears IMS copy of this hook. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1bb782c commit c25039d

2 files changed

Lines changed: 85 additions & 3 deletions

File tree

packages/core/src/hooks/use-url-collection-state.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,21 @@ describe("useUrlCollectionState", () => {
124124
]);
125125
});
126126

127+
it("hydrates object filter values (between {min,max} JSON format)", () => {
128+
mockParams = new URLSearchParams(
129+
'f.createdAt:between={"min":"2026-01-01","max":"2026-12-31"}',
130+
);
131+
const control = makeControl();
132+
renderHook(() => useUrlCollectionState(control));
133+
expect(control.setFilters).toHaveBeenCalledWith([
134+
{
135+
field: "createdAt",
136+
operator: "between",
137+
value: { min: "2026-01-01", max: "2026-12-31" },
138+
},
139+
]);
140+
});
141+
127142
it("skips filters with missing value", () => {
128143
mockParams = new URLSearchParams("f.status:eq=");
129144
const control = makeControl();
@@ -204,6 +219,30 @@ describe("useUrlCollectionState", () => {
204219
}
205220
});
206221

222+
it("treats reordered params as unchanged (order-insensitive compare)", () => {
223+
// prev URL holds filters in b,a order; control emits the same multiset in
224+
// a,b order. The delete-then-set rebuild flips key order, but the param
225+
// multiset is identical, so the write must bail out and return prev.
226+
// A plain `.toString()` compare would see a difference here and navigate.
227+
mockParams = new URLSearchParams("f.b:eq=2&f.a:eq=1&p=20");
228+
const filters = [
229+
{ field: "a", operator: "eq" as never, value: "1" },
230+
{ field: "b", operator: "eq" as never, value: "2" },
231+
];
232+
const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), {
233+
initialProps: { control: makeControl({ pageSize: 20, filters }) },
234+
});
235+
// Re-render with a fresh control (new array refs) to fire the write effect.
236+
rerender({ control: makeControl({ pageSize: 20, filters: [...filters] }) });
237+
const lastCallIndex = mockSetParams.mock.calls.length - 1;
238+
const [updater] = mockSetParams.mock.calls[lastCallIndex];
239+
expect(typeof updater).toBe("function");
240+
if (typeof updater === "function") {
241+
const result = updater(mockParams);
242+
expect(result).toBe(mockParams);
243+
}
244+
});
245+
207246
it("uses replace: true for setParams", () => {
208247
const { rerender } = renderHook(({ control }) => useUrlCollectionState(control), {
209248
initialProps: { control: makeControl({ pageSize: 20 }) },
@@ -262,6 +301,22 @@ describe("decodeFilterValue", () => {
262301
expect(decodeFilterValue('["Smith, John","Doe, Jane"]')).toEqual(["Smith, John", "Doe, Jane"]);
263302
});
264303

304+
it("decodes JSON objects (between {min,max} shape)", () => {
305+
expect(decodeFilterValue('{"min":1,"max":10}')).toEqual({ min: 1, max: 10 });
306+
});
307+
308+
it("round-trips an object value through encode → decode", () => {
309+
const value = { min: "2026-01-01", max: "2026-12-31" };
310+
expect(decodeFilterValue(encodeFilterValue(value))).toEqual(value);
311+
});
312+
313+
it("does not coerce numeric- or boolean-looking strings to primitives", () => {
314+
// These parse as JSON (number/boolean) but must stay strings so operator
315+
// implementations keep their string-vs-numeric semantics.
316+
expect(decodeFilterValue("5")).toBe("5");
317+
expect(decodeFilterValue("true")).toBe("true");
318+
});
319+
265320
it("returns plain string for non-array values", () => {
266321
expect(decodeFilterValue("hello")).toBe("hello");
267322
});

packages/core/src/hooks/use-url-collection-state.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,12 @@ export function useUrlCollectionState<
118118
}
119119

120120
// Bail out if nothing changed — avoids a no-op navigation that could
121-
// still trigger re-renders in some react-router versions.
122-
if (next.toString() === prev.toString()) return prev;
121+
// still trigger re-renders in some react-router versions. Compare on a
122+
// sorted snapshot rather than `.toString()`: the filter rebuild above is
123+
// delete-then-set, so key order can shift between renders even when the
124+
// param multiset is identical (and `.toString()` is additionally
125+
// sensitive to `&`/`=` characters inside values).
126+
if (stableQueryString(next) === stableQueryString(prev)) return prev;
123127
return next;
124128
},
125129
{ replace: true },
@@ -154,14 +158,37 @@ function stringifyPrimitive(value: unknown): string {
154158
/**
155159
* Decodes a filter value from URL storage.
156160
* - JSON arrays are parsed back into arrays.
157-
* - All other values are returned as plain strings.
161+
* - JSON objects are parsed back into objects (e.g. the `between` operator's
162+
* `{ min, max }` shape).
163+
* - All other values — including numeric/boolean-looking strings such as `"5"`
164+
* or `"true"` — are returned as plain strings, preserving the string-vs-
165+
* numeric distinction that operator implementations rely on.
158166
*/
159167
export function decodeFilterValue(raw: string): unknown {
160168
try {
161169
const parsed: unknown = JSON.parse(raw);
162170
if (Array.isArray(parsed)) return parsed;
171+
// Objects (e.g. a `between` filter's `{ min, max }`) are JSON-encoded by
172+
// encodeFilterValue and must be decoded back here; without this, object-
173+
// valued filters round-trip to a raw string on reload and silently break.
174+
if (parsed && typeof parsed === "object") return parsed;
163175
} catch {
164176
// Not valid JSON — fall through to plain string
165177
}
166178
return raw;
167179
}
180+
181+
/**
182+
* Order-insensitive, unambiguous snapshot of a URLSearchParams for equality
183+
* checks. Entries are sorted by key then value so a reordered-but-equivalent
184+
* param set compares equal, and the pairs are JSON-encoded so a key or value
185+
* containing `&`/`=` can't collide with the entry boundary the way a re-joined
186+
* query string would (e.g. `[["a","x"],["b","y"]]` vs `[["a","x&b=y"]]`).
187+
*/
188+
function stableQueryString(sp: URLSearchParams): string {
189+
return JSON.stringify(
190+
Array.from(sp.entries()).toSorted(
191+
([ak, av], [bk, bv]) => ak.localeCompare(bk) || av.localeCompare(bv),
192+
),
193+
);
194+
}

0 commit comments

Comments
 (0)