Skip to content

Commit

Permalink
fix: Ensure values in patches are never a draft, fixes #559
Browse files Browse the repository at this point in the history
  • Loading branch information
mweststrate authored May 5, 2020
2 parents 0ddddfc + 2b54ac4 commit 33ecbd6
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 6 deletions.
36 changes: 35 additions & 1 deletion __tests__/patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import produce, {
setUseProxies,
applyPatches,
produceWithPatches,
enableAllPlugins
enableAllPlugins,
isDraft
} from "../src/immer"

enableAllPlugins()
Expand Down Expand Up @@ -1014,3 +1015,36 @@ test("#521", () => {
new Map([["hello", new Set(["world", "immer"])]])
)
})

test("#559 patches works in a nested reducer with proxies", () => {
setUseProxies(true)

const state = {
x: 1,
sub: {
y: [{a: 0}, {a: 1}]
}
}

const changes = []
const inverseChanges = []

const newState = produce(state, draft => {
draft.sub = produce(
draft.sub,
draft => {
draft.y.pop()
},
(patches, inversePatches) => {
expect(isDraft(inversePatches[0].value)).toBeFalsy()
expect(inversePatches[0].value).toMatchObject({a: 1})
changes.push(...patches)
inverseChanges.push(...inversePatches)
}
)
})

const reversedSubState = applyPatches(newState.sub, inverseChanges)

expect(reversedSubState).toMatchObject(state.sub)
})
21 changes: 16 additions & 5 deletions src/plugins/patches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
ArchtypeArray,
die
} from "../internal"
import {isDraft} from "../utils/common"

export function enablePatches() {
const REPLACE = "replace"
Expand Down Expand Up @@ -98,12 +99,14 @@ export function enablePatches() {
patches.push({
op: REPLACE,
path,
value: copy_[i]
// Need to maybe clone it, as it can in fact be the original value
// due to the base/copy inversion at the start of this function
value: clonePatchValueIfNeeded(copy_[i])
})
inversePatches.push({
op: REPLACE,
path,
value: base_[i]
value: clonePatchValueIfNeeded(base_[i])
})
}
}
Expand All @@ -116,7 +119,9 @@ export function enablePatches() {
patches[replaceCount + i - end] = {
op: ADD,
path,
value: copy_[i]
// Need to maybe clone it, as it can in fact be the original value
// due to the base/copy inversion at the start of this function
value: clonePatchValueIfNeeded(copy_[i])
}
inversePatches.push({
op: REMOVE,
Expand Down Expand Up @@ -144,8 +149,8 @@ export function enablePatches() {
op === ADD
? {op: REMOVE, path}
: op === REMOVE
? {op: ADD, path, value: origValue}
: {op: REPLACE, path, value: origValue}
? {op: ADD, path, value: clonePatchValueIfNeeded(origValue)}
: {op: REPLACE, path, value: clonePatchValueIfNeeded(origValue)}
)
})
}
Expand Down Expand Up @@ -287,6 +292,12 @@ export function enablePatches() {
return cloned
}

function clonePatchValueIfNeeded<T>(obj: T): T {
if (isDraft(obj)) {
return deepClonePatchValue(obj)
} else return obj
}

loadPlugin("Patches", {
applyPatches_,
generatePatches_,
Expand Down

0 comments on commit 33ecbd6

Please sign in to comment.