diff --git a/__tests__/patch.js b/__tests__/patch.js index a7d8750f..e58a7310 100644 --- a/__tests__/patch.js +++ b/__tests__/patch.js @@ -76,19 +76,6 @@ describe("applyPatches", () => { applyPatches({}, [patch]) }).toThrowError(/^Cannot apply patch, path doesn't resolve:/) }) - it("throws when a patch tries to splice an array", () => { - // Pop is ok - expect(() => { - const patch = {op: "remove", path: [0]} - applyPatches([1], [patch]) - }).not.toThrowError() - - // Splice is unsupported - expect(() => { - const patch = {op: "remove", path: [0]} - applyPatches([1, 2], [patch]) - }).toThrowError(/^Only the last index of an array can be removed/) - }) }) describe("simple assignment - 1", () => { @@ -111,6 +98,16 @@ describe("simple assignment - 2", () => { ) }) +describe("simple assignment - 3", () => { + runPatchTest( + {x: [{y: 4}]}, + d => { + d.x[0].y++ + }, + [{op: "replace", path: ["x", 0, "y"], value: 5}] + ) +}) + describe("delete 1", () => { runPatchTest( {x: {y: 4}}, @@ -198,11 +195,20 @@ describe("arrays - prepend", () => { d => { d.x.unshift(4) }, + [{op: "add", path: ["x", 0], value: 4}] + ) +}) + +describe("arrays - multiple prepend", () => { + runPatchTest( + {x: [1, 2, 3]}, + d => { + d.x.unshift(4) + d.x.unshift(5) + }, [ - {op: "replace", path: ["x", 0], value: 4}, - {op: "replace", path: ["x", 1], value: 1}, - {op: "replace", path: ["x", 2], value: 2}, - {op: "add", path: ["x", 3], value: 3} + {op: "add", path: ["x", 0], value: 5}, + {op: "add", path: ["x", 1], value: 4} ] ) }) @@ -213,8 +219,35 @@ describe("arrays - splice middle", () => { d => { d.x.splice(1, 1) }, + [{op: "remove", path: ["x", 1]}] + ) +}) + +describe("arrays - multiple splice", () => { + runPatchTest( + [0, 1, 2, 3, 4, 5, 0], + d => { + d.splice(4, 2, 3) + d.splice(1, 2, 3) + }, + [ + {op: "replace", path: [1], value: 3}, + {op: "replace", path: [2], value: 3}, + {op: "remove", path: [5]}, + {op: "remove", path: [4]} + ] + ) +}) + +describe("arrays - modify and shrink", () => { + runPatchTest( + {x: [1, 2, 3]}, + d => { + d.x[0] = 4 + d.x.length = 2 + }, [ - {op: "replace", path: ["x", 1], value: 3}, + {op: "replace", path: ["x", 0], value: 4}, {op: "replace", path: ["x", "length"], value: 2} ] ) @@ -287,6 +320,44 @@ describe("arrays - push multiple", () => { ) }) +describe("arrays - splice (expand)", () => { + runPatchTest( + {x: [1, 2, 3]}, + d => { + d.x.splice(1, 1, 4, 5, 6) + }, + [ + {op: "replace", path: ["x", 1], value: 4}, + {op: "add", path: ["x", 2], value: 5}, + {op: "add", path: ["x", 3], value: 6} + ], + [ + {op: "replace", path: ["x", 1], value: 2}, + {op: "remove", path: ["x", 3]}, + {op: "remove", path: ["x", 2]} + ] + ) +}) + +describe("arrays - splice (shrink)", () => { + runPatchTest( + {x: [1, 2, 3, 4, 5]}, + d => { + d.x.splice(1, 3, 6) + }, + [ + {op: "replace", path: ["x", 1], value: 6}, + {op: "remove", path: ["x", 3]}, + {op: "remove", path: ["x", 2]} + ], + [ + {op: "replace", path: ["x", 1], value: 2}, + {op: "add", path: ["x", 2], value: 3}, + {op: "add", path: ["x", 3], value: 4} + ] + ) +}) + describe("simple replacement", () => { runPatchTest({x: 3}, _d => 4, [{op: "replace", path: [], value: 4}]) }) diff --git a/src/patches.js b/src/patches.js index 7846d2fc..b147a6da 100644 --- a/src/patches.js +++ b/src/patches.js @@ -7,49 +7,75 @@ export function generatePatches(state, basePath, patches, inversePatches) { } function generateArrayPatches(state, basePath, patches, inversePatches) { - const {base, copy, assigned} = state - const minLength = Math.min(base.length, copy.length) + let {base, copy, assigned} = state - // Look for replaced indices. - for (let i = 0; i < minLength; i++) { - if (assigned[i] && base[i] !== copy[i]) { - const path = basePath.concat(i) - patches.push({op: "replace", path, value: copy[i]}) - inversePatches.push({op: "replace", path, value: base[i]}) - } + // Guarantee `base` is never longer than `copy` + if (copy.length < base.length) { + ;[base, copy] = [copy, base] + ;[patches, inversePatches] = [inversePatches, patches] + } + + const delta = copy.length - base.length + + // Find the first changed index. + let start = 0 + while (base[start] === copy[start] && start < base.length) { + ++start + } + + // Find the last changed index. + let baseEnd = base.length + while (baseEnd > start && base[baseEnd - 1] === copy[baseEnd + delta - 1]) { + --baseEnd } - // Did the array expand? - if (minLength < copy.length) { - for (let i = minLength; i < copy.length; i++) { + // Look for replaced indices. + const replaceCount = baseEnd - start + for (let i = 0; i < replaceCount; ++i) { + const index = start + i + const path = basePath.concat([index]) + if (assigned[index] && copy[index] !== base[index]) { patches.push({ - op: "add", - path: basePath.concat(i), - value: copy[i] + op: "replace", + path, + value: copy[index] + }) + inversePatches.push({ + op: "replace", + path, + value: base[index] }) } - inversePatches.push({ - op: "replace", - path: basePath.concat("length"), - value: base.length - }) } + start += replaceCount - // ...or did it shrink? - else if (minLength < base.length) { - patches.push({ - op: "replace", - path: basePath.concat("length"), - value: copy.length - }) - for (let i = minLength; i < base.length; i++) { + // For "add" patches that extend the array, we can use a single "replace" + // patch (on the `length` property) as the inverse patch, instead of multiple + // "remove" patches. + const useRemove = start != base.length + const replacePatchesCount = patches.length + for (let i = delta - 1; i >= 0; --i) { + const path = basePath.concat([start + i]) + patches[i + replacePatchesCount] = { + op: "add", + path, + value: copy[start + i] + } + if (useRemove) { inversePatches.push({ - op: "add", - path: basePath.concat(i), - value: base[i] + op: "remove", + path }) } } + + if (!useRemove) { + inversePatches.push({ + op: "replace", + path: basePath.concat(["length"]), + value: base.length + }) + } } function generateObjectPatches(state, basePath, patches, inversePatches) { @@ -87,15 +113,19 @@ export function applyPatches(draft, patches) { const key = path[path.length - 1] switch (patch.op) { case "replace": - case "add": - // TODO: add support is not extensive, it does not support insertion or `-` atm! base[key] = patch.value break + case "add": + if (Array.isArray(base)) { + // TODO: support "foo/-" paths for appending to an array + base.splice(key, 0, patch.value) + } else { + base[key] = patch.value + } + break case "remove": if (Array.isArray(base)) { - if (key !== base.length - 1) - throw new Error(`Only the last index of an array can be removed, index: ${key}, length: ${base.length}`) // prettier-ignore - base.length -= 1 + base.splice(key, 1) } else { delete base[key] }