diff --git a/packages/core/src/state-store.test.ts b/packages/core/src/state-store.test.ts index 47d6fa08..856e0309 100644 --- a/packages/core/src/state-store.test.ts +++ b/packages/core/src/state-store.test.ts @@ -30,6 +30,32 @@ describe("createStateStore", () => { expect(store.getSnapshot()).toEqual({ x: 1 }); }); + it.each(["1foo", "01", "1.5", "-1", "999999999999999999999"])( + "set ignores invalid array index %s", + (index) => { + const store = createStateStore({ items: ["a", "b", "c"] }); + const snapshot = store.getSnapshot(); + const listener = vi.fn(); + store.subscribe(listener); + + store.set(`/items/${index}`, "x"); + + expect(store.getSnapshot()).toBe(snapshot); + expect(store.getSnapshot().items).toEqual(["a", "b", "c"]); + expect(listener).not.toHaveBeenCalled(); + }, + ); + + it("preserves unsafe numeric tokens as object keys", () => { + const store = createStateStore({ obj: {} }); + + store.set("/obj/999999999999999999999/name", "value"); + + expect(store.getSnapshot()).toEqual({ + obj: { "999999999999999999999": { name: "value" } }, + }); + }); + it("update notifies subscribers once", () => { const store = createStateStore({}); const listener = vi.fn(); diff --git a/packages/core/src/state-store.ts b/packages/core/src/state-store.ts index 1964e055..74ce96b9 100644 --- a/packages/core/src/state-store.ts +++ b/packages/core/src/state-store.ts @@ -5,6 +5,17 @@ import { type StateStore, } from "./types"; +function isNumericIndex(segment: string): boolean { + return /^(0|[1-9]\d*)$/.test(segment); +} + +function parseArrayIndex(segment: string): number | undefined { + if (!isNumericIndex(segment)) return undefined; + + const index = Number(segment); + return Number.isSafeInteger(index) ? index : undefined; +} + /** * Immutably set a value at a JSON Pointer path using structural sharing. * Only objects along the path are shallow-cloned; untouched branches keep @@ -23,16 +34,26 @@ export function immutableSetByPath( for (let i = 0; i < segments.length - 1; i++) { const seg = segments[i]!; - const child = current[seg]; + let key: string | number = seg; + if (Array.isArray(current)) { + const index = parseArrayIndex(seg); + if (index === undefined) return root; + key = index; + } + + const nextSeg = segments[i + 1]; + const nextIndex = + nextSeg === undefined ? undefined : parseArrayIndex(nextSeg); + + const child = current[key]; if (Array.isArray(child)) { - current[seg] = [...child]; + current[key] = [...child]; } else if (child !== null && typeof child === "object") { - current[seg] = { ...(child as Record) }; + current[key] = { ...(child as Record) }; } else { - const nextSeg = segments[i + 1]; - current[seg] = nextSeg !== undefined && /^\d+$/.test(nextSeg) ? [] : {}; + current[key] = nextIndex !== undefined ? [] : {}; } - current = current[seg] as Record; + current = current[key] as Record; } const lastSeg = segments[segments.length - 1]!; @@ -40,7 +61,9 @@ export function immutableSetByPath( if (lastSeg === "-") { (current as unknown[]).push(value); } else { - (current as unknown[])[parseInt(lastSeg, 10)] = value; + const index = parseArrayIndex(lastSeg); + if (index === undefined) return root; + (current as unknown[])[index] = value; } } else { current[lastSeg] = value; @@ -73,7 +96,9 @@ export function createStateStore(initialState: StateModel = {}): StateStore { set(path: string, value: unknown): void { if (getByPath(state, path) === value) return; - state = immutableSetByPath(state, path, value); + const next = immutableSetByPath(state, path, value); + if (next === state) return; + state = next; notify(); }, @@ -82,8 +107,11 @@ export function createStateStore(initialState: StateModel = {}): StateStore { let next = state; for (const [path, value] of Object.entries(updates)) { if (getByPath(next, path) !== value) { - next = immutableSetByPath(next, path, value); - changed = true; + const updated = immutableSetByPath(next, path, value); + if (updated !== next) { + next = updated; + changed = true; + } } } if (!changed) return; @@ -139,7 +167,9 @@ export function createStoreAdapter(config: StoreAdapterConfig): StateStore { set(path: string, value: unknown): void { const current = config.getSnapshot(); if (getByPath(current, path) === value) return; - config.setSnapshot(immutableSetByPath(current, path, value)); + const next = immutableSetByPath(current, path, value); + if (next === current) return; + config.setSnapshot(next); }, update(updates: Record): void { @@ -147,8 +177,11 @@ export function createStoreAdapter(config: StoreAdapterConfig): StateStore { let changed = false; for (const [path, value] of Object.entries(updates)) { if (getByPath(next, path) !== value) { - next = immutableSetByPath(next, path, value); - changed = true; + const updated = immutableSetByPath(next, path, value); + if (updated !== next) { + next = updated; + changed = true; + } } } if (!changed) return; diff --git a/packages/core/src/types.test.ts b/packages/core/src/types.test.ts index 31701a95..3b249547 100644 --- a/packages/core/src/types.test.ts +++ b/packages/core/src/types.test.ts @@ -215,6 +215,57 @@ describe("JSON Pointer escaping (RFC 6901)", () => { }); }); +describe("JSON Patch array indexes (RFC 6902)", () => { + it.each(["1foo", "01", "1.5", "-1"])( + "rejects invalid array index %s", + (index) => { + const source = { items: ["a", "b", "c"] }; + expect(getByPath(source, `/items/${index}`)).toBeUndefined(); + + const setData: Record = { items: ["a", "b", "c"] }; + setByPath(setData, `/items/${index}`, "x"); + expect(setData.items).toEqual(["a", "b", "c"]); + expect(Object.keys(setData.items as unknown[])).toEqual(["0", "1", "2"]); + + const addData: Record = { items: ["a", "b", "c"] }; + addByPath(addData, `/items/${index}`, "x"); + expect(addData.items).toEqual(["a", "b", "c"]); + + const removeData: Record = { items: ["a", "b", "c"] }; + removeByPath(removeData, `/items/${index}`); + expect(removeData.items).toEqual(["a", "b", "c"]); + }, + ); + + it("accepts canonical array indexes", () => { + const data: Record = { items: ["a", "b", "c"] }; + setByPath(data, "/items/0", "first"); + setByPath(data, "/items/2", "last"); + expect(data.items).toEqual(["first", "b", "last"]); + }); + + it("preserves invalid-index-shaped keys on objects", () => { + const data = { items: { "01": "object key" } }; + expect(getByPath(data, "/items/01")).toBe("object key"); + }); + + it("treats unsafe numeric tokens as object keys when creating containers", () => { + const setData: Record = {}; + setByPath(setData, "/items/999999999999999999999", "x"); + expect(setData).toEqual({ items: { "999999999999999999999": "x" } }); + + const addData: Record = {}; + addByPath(addData, "/items/999999999999999999999", "x"); + expect(addData).toEqual({ items: { "999999999999999999999": "x" } }); + + const nestedData: Record = { obj: {} }; + setByPath(nestedData, "/obj/999999999999999999999/name", "value"); + expect(nestedData).toEqual({ + obj: { "999999999999999999999": { name: "value" } }, + }); + }); +}); + // ============================================================================= // addByPath (RFC 6902 "add" semantics) // ============================================================================= diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index c051449a..93754faa 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -298,7 +298,8 @@ export function getByPath(obj: unknown, path: string): unknown { } if (Array.isArray(current)) { - const index = parseInt(segment, 10); + const index = parseArrayIndex(segment); + if (index === undefined) return undefined; current = current[index]; } else if (typeof current === "object") { current = (current as Record)[segment]; @@ -348,7 +349,14 @@ function joinStatePath(basePath: string, childPath: string): string { * Check if a string is a numeric index */ function isNumericIndex(str: string): boolean { - return /^\d+$/.test(str); + return /^(0|[1-9]\d*)$/.test(str); +} + +function parseArrayIndex(str: string): number | undefined { + if (!isNumericIndex(str)) return undefined; + + const index = Number(str); + return Number.isSafeInteger(index) ? index : undefined; } /** @@ -369,12 +377,13 @@ export function setByPath( for (let i = 0; i < segments.length - 1; i++) { const segment = segments[i]!; const nextSegment = segments[i + 1]; - const nextIsNumeric = - nextSegment !== undefined && - (isNumericIndex(nextSegment) || nextSegment === "-"); + const nextArrayIndex = + nextSegment === undefined ? undefined : parseArrayIndex(nextSegment); + const nextIsNumeric = nextArrayIndex !== undefined || nextSegment === "-"; if (Array.isArray(current)) { - const index = parseInt(segment, 10); + const index = parseArrayIndex(segment); + if (index === undefined) return; if (current[index] === undefined || typeof current[index] !== "object") { current[index] = nextIsNumeric ? [] : {}; } @@ -392,7 +401,8 @@ export function setByPath( if (lastSegment === "-") { current.push(value); } else { - const index = parseInt(lastSegment, 10); + const index = parseArrayIndex(lastSegment); + if (index === undefined) return; current[index] = value; } } else { @@ -419,12 +429,13 @@ export function addByPath( for (let i = 0; i < segments.length - 1; i++) { const segment = segments[i]!; const nextSegment = segments[i + 1]; - const nextIsNumeric = - nextSegment !== undefined && - (isNumericIndex(nextSegment) || nextSegment === "-"); + const nextArrayIndex = + nextSegment === undefined ? undefined : parseArrayIndex(nextSegment); + const nextIsNumeric = nextArrayIndex !== undefined || nextSegment === "-"; if (Array.isArray(current)) { - const index = parseInt(segment, 10); + const index = parseArrayIndex(segment); + if (index === undefined) return; if (current[index] === undefined || typeof current[index] !== "object") { current[index] = nextIsNumeric ? [] : {}; } @@ -442,7 +453,8 @@ export function addByPath( if (lastSegment === "-") { current.push(value); } else { - const index = parseInt(lastSegment, 10); + const index = parseArrayIndex(lastSegment); + if (index === undefined) return; current.splice(index, 0, value); } } else { @@ -466,7 +478,8 @@ export function removeByPath(obj: Record, path: string): void { const segment = segments[i]!; if (Array.isArray(current)) { - const index = parseInt(segment, 10); + const index = parseArrayIndex(segment); + if (index === undefined) return; if (current[index] === undefined || typeof current[index] !== "object") { return; // path does not exist } @@ -481,7 +494,8 @@ export function removeByPath(obj: Record, path: string): void { const lastSegment = segments[segments.length - 1]!; if (Array.isArray(current)) { - const index = parseInt(lastSegment, 10); + const index = parseArrayIndex(lastSegment); + if (index === undefined) return; if (index >= 0 && index < current.length) { current.splice(index, 1); }