From e3df956dca6f62c9053ab750b2c8b8a518f3e001 Mon Sep 17 00:00:00 2001 From: jaybhade Date: Wed, 5 Aug 2026 23:19:58 +0530 Subject: [PATCH] fix: key inserted array indices by name in the array-methods plugin `handleInsertedValues` recorded inserted indices in `assigned_` as numbers, but `assigned_` is keyed by property name everywhere else, because the proxy traps only ever see strings. Patch generation looks indices up with `assigned_?.get(i.toString())`, so a numeric key was never found. The added indices loop in `generateArrayPatches` covers everything past `base_.length` regardless of `assigned_`, which hid the mismatch for a plain push. It surfaced when an insert reuses an index that already existed in the base, e.g. dropping the oldest entry and appending a new one: const [next, patches] = produceWithPatches(["a", "b", "c"], draft => { draft.pop() draft.push("z") }) // next -> ["a", "b", "z"] // patches -> [] (expected: replace /2 with "z") The result was correct but no patch described it, so undo/redo and state sync built on the patch stream silently dropped the change. Stringifying the index also keeps the key consistent for the finalization lookup in `handleCrossReference`, which reads `assigned_` with the same key this function passes it. --- __tests__/base.js | 17 +++++++++++++++++ src/plugins/arrayMethods.ts | 7 ++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/__tests__/base.js b/__tests__/base.js index c293eefa..51a7a29a 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -423,6 +423,23 @@ function runBaseTest( expect(newLength).toBe(4) }) }) + + test("push onto an index freed by pop() is patched", () => { + const base = {items: ["a", "b", "c"]} + const [result, patches, inverse] = produceWithPatches( + base, + draft => { + draft.items.pop() + draft.items.push("z") + } + ) + expect(result.items).toEqual(["a", "b", "z"]) + expect(patches).toEqual([ + {op: "replace", path: ["items", 2], value: "z"} + ]) + expect(applyPatches(base, patches)).toEqual(result) + expect(applyPatches(result, inverse)).toEqual(base) + }) }) describe("unshift()", () => { diff --git a/src/plugins/arrayMethods.ts b/src/plugins/arrayMethods.ts index a4d3d019..4c11fc33 100644 --- a/src/plugins/arrayMethods.ts +++ b/src/plugins/arrayMethods.ts @@ -202,6 +202,11 @@ export function enableArrayMethods() { * Without this, values containing draft proxies (like `{...state[0]}`) * pushed via the array methods plugin would have their nested drafts * revoked during finalization without being replaced by final values. + * + * The index is stringified because the proxy traps only ever see property + * names, so `assigned_` is keyed by string everywhere else. A numeric key + * would be invisible to the readers that look indices up by name, such as + * patch generation. */ function handleInsertedValues( state: ProxyArrayState, @@ -209,7 +214,7 @@ export function enableArrayMethods() { values: any[] ) { for (let i = 0; i < values.length; i++) { - const index = startIndex + i + const index = "" + (startIndex + i) state.assigned_!.set(index, true) handleCrossReference(state, index, values[i]) }