Skip to content

Commit

Permalink
Fix: don't unnecessary modify draft if replaying patches results in a…
Browse files Browse the repository at this point in the history
… new state anyway
  • Loading branch information
mweststrate committed Aug 2, 2019
1 parent 6620835 commit 1963da1
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 33 deletions.
46 changes: 45 additions & 1 deletion __tests__/patch.js
Expand Up @@ -456,7 +456,6 @@ describe("patch compressions yields correct results", () => {
const res = runPatchTest(
{},
d => {
debugger
applyPatches(d, [...p1, ...p2])
},
[]
Expand Down Expand Up @@ -485,3 +484,48 @@ describe("change then delete property", () => {
expect(res).toEqual({})
})
})

test("replaying patches with interweaved replacements should work correctly", () => {
const patches = []
const s0 = {x: 1}

const s1 = produce(
s0,
draft => {
draft.x = 2
},
p => {
patches.push(...p)
}
)

const s2 = produce(
s1,
draft => {
return {x: 0}
},
p => {
patches.push(...p)
}
)

const s3 = produce(
s2,
draft => {
draft.x--
},
p => {
patches.push(...p)
}
)

expect(s3).toEqual({x: -1}) // correct result
expect(applyPatches(s0, patches)).toEqual({x: -1}) // correct replay

// manual replay on a draft should also be correct
expect(
produce(s0, draft => {
return applyPatches(draft, patches)
})
).toEqual({x: -1})
})
17 changes: 15 additions & 2 deletions src/immer.js
Expand Up @@ -121,12 +121,25 @@ export class Immer {
assign(this, value ? modernProxy : legacyProxy)
}
applyPatches(base, patches) {
// Mutate the base state when a draft is passed.
// If a patch replaces the entire state, take that replacement as base
// before applying patches
let i
for (i = patches.length - 1; i >= 0; i--) {
const patch = patches[i]
if (patch.path.length === 0 && patch.op === "replace") {
base = patch.value
break
}
}

if (isDraft(base)) {
// N.B: never hits if some patch a replacement, patches are never drafts
return applyPatches(base, patches)
}
// Otherwise, produce a copy of the base state.
return this.produce(base, draft => applyPatches(draft, patches))
return this.produce(base, draft =>
applyPatches(draft, patches.slice(i + 1))
)
}
/** @internal */
processResult(result, scope) {
Expand Down
59 changes: 29 additions & 30 deletions src/patches.js
@@ -1,4 +1,5 @@
import {each} from "./common"
import {createDraft} from "./immer"

export function generatePatches(state, basePath, patches, inversePatches) {
Array.isArray(state.base)
Expand Down Expand Up @@ -95,41 +96,39 @@ 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
if (path.length === 0 && patch.op === "replace") {
draft = patch.value
} else {
let base = draft
for (let i = 0; i < path.length - 1; i++) {
base = base[path[i]]
if (!base || typeof base !== "object")
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
}
const key = path[path.length - 1]
switch (patch.op) {
case "replace":
}
const key = path[path.length - 1]
switch (patch.op) {
case "replace":
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 "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)) {
base.splice(key, 1)
} else {
delete base[key]
}
break
default:
throw new Error("Unsupported patch operation: " + patch.op)
}
}
break
case "remove":
if (Array.isArray(base)) {
base.splice(key, 1)
} else {
delete base[key]
}
break
default:
throw new Error("Unsupported patch operation: " + patch.op)
}
}
return draft
Expand Down

0 comments on commit 1963da1

Please sign in to comment.