From c2e69d9ea9c7d4a50abe502cd0be805b74732cab Mon Sep 17 00:00:00 2001 From: Shuumatsu Date: Wed, 28 Aug 2019 15:46:16 +0800 Subject: [PATCH 1/2] fix: ensure patches are reusable --- __tests__/patch.js | 28 +++++++++++++++++++++++++++- src/patches.js | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/__tests__/patch.js b/__tests__/patch.js index 32aa526d..7fc1fef1 100644 --- a/__tests__/patch.js +++ b/__tests__/patch.js @@ -1,5 +1,9 @@ "use strict" -import produce, {setUseProxies, applyPatches} from "../src/index" +import produce, { + setUseProxies, + applyPatches, + produceWithPatches +} from "../src/index" jest.setTimeout(1000) @@ -82,6 +86,28 @@ describe("applyPatches", () => { applyPatches({}, [patch]) }).toThrowErrorMatchingSnapshot() }) + it("applied patches cannot be modified", () => { + // see also: https://github.com/immerjs/immer/issues/411 + const s0 = { + items: [1] + } + + const [s1, p1] = produceWithPatches(s0, draft => { + draft.items = [] + }) + + const replaceValueBefore = p1[0].value.slice() + + const [s2, p2] = produceWithPatches(s1, draft => { + draft.items.push(2) + }) + + applyPatches(s0, [...p1, ...p2]) + + const replaceValueAfter = p1[0].value.slice() + + expect(replaceValueAfter).toStrictEqual(replaceValueBefore) + }) }) describe("simple assignment - 1", () => { diff --git a/src/patches.js b/src/patches.js index 0d5b3d5e..ee74bd45 100644 --- a/src/patches.js +++ b/src/patches.js @@ -95,29 +95,45 @@ function generateObjectPatches(state, basePath, patches, inversePatches) { }) } -export function applyPatches(draft, patches) { - // First, find a patch that replaces the entire state, if found, we don't have to apply earlier patches and modify the state - for (let i = 0; i < patches.length; i++) { - const patch = patches[i] - const {path} = patch +// used to clone patch to ensure original patch is not modified +const clone = obj => { + if (typeof obj !== "object") return obj + + if (Array.isArray(obj)) return obj.map(clone) + + const cloned = {} + for (const key in obj) cloned[key] = clone(obj[key]) + + return cloned +} + +export const applyPatches = (draft, patches) => { + for (const patch of patches) { + const {path, op, value} = clone(patch) + if (!path.length) throw new Error("Illegal state") + let base = draft for (let i = 0; i < path.length - 1; i++) { base = base[path[i]] if (!base || typeof base !== "object") - throw new Error("Cannot apply patch, path doesn't resolve: " + path.join("/")) // prettier-ignore + throw new Error("Cannot apply patch, path doesn't resolve: " + path.join("/")) // prettier-ignore } + const key = path[path.length - 1] - switch (patch.op) { + switch (op) { case "replace": - base[key] = patch.value + // if value is an object, then it's assigned by reference + // in the following add or remove ops, the value field inside the patch will also be modifyed + // so we use value from the cloned patch + base[key] = value break case "add": if (Array.isArray(base)) { // TODO: support "foo/-" paths for appending to an array - base.splice(key, 0, patch.value) + base.splice(key, 0, value) } else { - base[key] = patch.value + base[key] = value } break case "remove": @@ -128,8 +144,9 @@ export function applyPatches(draft, patches) { } break default: - throw new Error("Unsupported patch operation: " + patch.op) + throw new Error("Unsupported patch operation: " + op) } } + return draft } From 49b2e7bc654c3f2b79babfbef9e51fc27a505567 Mon Sep 17 00:00:00 2001 From: Shuumatsu Date: Sat, 31 Aug 2019 10:13:38 +0800 Subject: [PATCH 2/2] fix: ensure patches are reusable --- src/patches.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/patches.js b/src/patches.js index ee74bd45..2c803e40 100644 --- a/src/patches.js +++ b/src/patches.js @@ -97,7 +97,7 @@ function generateObjectPatches(state, basePath, patches, inversePatches) { // used to clone patch to ensure original patch is not modified const clone = obj => { - if (typeof obj !== "object") return obj + if (obj === null || typeof obj !== "object") return obj if (Array.isArray(obj)) return obj.map(clone) @@ -109,7 +109,8 @@ const clone = obj => { export const applyPatches = (draft, patches) => { for (const patch of patches) { - const {path, op, value} = clone(patch) + const {path, op} = patch + const value = clone(patch.value) if (!path.length) throw new Error("Illegal state")