From 8c8e514651cfd276dec38bb91d2307d41800e975 Mon Sep 17 00:00:00 2001 From: Osama Ansar Date: Tue, 6 Oct 2026 20:32:05 +0500 Subject: [PATCH] fix: skip nested patches for children of shifted or reordered arrays With enableArrayMethods(), unshift/shift/splice/reverse/sort run natively on copy_ and mark all indices as reassigned. A child drafted afterwards gets its key from the index it now sits at, so getPath accepts it and emits a nested patch such as ["arr", 3, "a"]. That patch is generated before the parent array's own patches. When the index is past the base length (e.g. unshift and then writing to the last element) it points at a path that does not exist yet, and applyPatches throws "path doesn't resolve". The array already emits a replace/add patch for every index that changed, carrying the finalized child, so nested patches add nothing there. Return no path for children of an array whose indices were reassigned. This also matches the patches produced without the plugin, where such children are not patched individually either. --- __tests__/base.js | 85 ++++++++++++++++++++++++++++++++++++++++++ src/plugins/patches.ts | 12 ++++++ 2 files changed, 97 insertions(+) diff --git a/__tests__/base.js b/__tests__/base.js index 208a11492..7e80b988e 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -716,6 +716,91 @@ function runBaseTest( }) }) + describe("patches for elements moved by shifting or reordering", () => { + const createBase = () => ({ + arr: [ + {id: 1, a: 1, meta: {n: 1}}, + {id: 2, a: 2, meta: {n: 2}}, + {id: 3, a: 3, meta: {n: 3}} + ] + }) + const added = {id: 9, a: 9, meta: {n: 9}} + + // Only the first four failed before: they write to an element that was + // shifted to an index at or past the base length. The rest are guards. + test.each([ + [ + "unshift, write to an element moved past the base length", + d => { + d.arr.unshift({...added}) + d.arr[3].a = 99 + } + ], + [ + "unshift, write to a nested object of an element moved past the base length", + d => { + d.arr.unshift({...added}) + d.arr[3].meta.n = 99 + } + ], + [ + "unshift of two items, write to the last element", + d => { + d.arr.unshift({...added}, {...added, id: 10}) + d.arr[4].a = 99 + } + ], + [ + "splice insert, write to an element moved past the base length", + d => { + d.arr.splice(1, 0, {...added}) + d.arr[3].a = 99 + } + ], + [ + "unshift, write to an element that stays within the base length", + d => { + d.arr.unshift({...added}) + d.arr[1].a = 99 + } + ], + [ + "shift, write to a shifted element", + d => { + d.arr.shift() + d.arr[1].a = 99 + } + ], + [ + "reverse, write to a relocated element", + d => { + d.arr.reverse() + d.arr[0].a = 99 + } + ], + [ + "push, write to the pushed item", + d => { + d.arr.push({...added}) + d.arr[3].a = 99 + } + ], + [ + "write, then unshift", + d => { + d.arr[2].a = 99 + d.arr.unshift({...added}) + } + ] + ])("%s", (_title, recipe) => { + const base = createBase() + const [result, patches, inverse] = produceWithPatches(base, recipe) + expect(base).toEqual(createBase()) + expect(applyPatches(base, patches)).toEqual(result) + expect(applyPatches(result, inverse)).toEqual(base) + }) + }) + describe("return values", () => { test("pop returns removed item", () => { const base = {items: [{id: 1}, {id: 2}]} diff --git a/src/plugins/patches.ts b/src/plugins/patches.ts index 08c7d0f62..4db8c3a99 100644 --- a/src/plugins/patches.ts +++ b/src/plugins/patches.ts @@ -90,6 +90,18 @@ export function enablePatches() { return null // Key deleted } + // Once an array's elements have been shifted or reordered, a child's index + // no longer matches its position in the base. The array generates a patch + // for every index that changed (carrying the finalized child), so a nested + // patch at the child's new index is redundant and, for an index past the + // base length, points at a path that does not exist yet when it is applied. + if ( + state.parent_!.type_ === ArchType.Array && + (state.parent_ as ProxyArrayState).allIndicesReassigned_ + ) { + return null + } + // Step 5: Add key to path path.push(key) }