Skip to content

Commit

Permalink
fix specs
Browse files Browse the repository at this point in the history
  • Loading branch information
runnez committed Oct 25, 2019
1 parent 741e941 commit ff25a83
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
2 changes: 2 additions & 0 deletions src/common.js
Expand Up @@ -221,6 +221,8 @@ function latest(state) {
export function clone(obj) {
if (!isDraftable(obj)) return obj
if (Array.isArray(obj)) return obj.map(clone)
if (isMap(obj)) return new Map(obj)
if (isSet(obj)) return new Set(obj)
const cloned = Object.create(Object.getPrototypeOf(obj))
for (const key in obj) cloned[key] = clone(obj[key])
return cloned
Expand Down
17 changes: 8 additions & 9 deletions src/patches.js
Expand Up @@ -129,7 +129,6 @@ function generateSetPatches(state, basePath, patches, inversePatches) {
export const applyPatches = (draft, patches) => {
for (const patch of patches) {
const {path, op} = patch
const value = clone(patch.value) // used to clone patch to ensure original patch is not modified, see #411

if (!path.length) throw new Error("Illegal state")

Expand All @@ -140,21 +139,21 @@ export const applyPatches = (draft, patches) => {
throw new Error("Cannot apply patch, path doesn't resolve: " + path.join("/")) // prettier-ignore
}

const value = isSet(base) ? patch.value : clone(patch.value) // used to clone patch to ensure original patch is not modified, see #411

This comment has been minimized.

Copy link
@mweststrate

mweststrate Oct 28, 2019

Collaborator

I think the condition is incorrect; even if the base is a set, and the op is add, the patch should be defended against accidental mutation, as theoretically a next patch could change that value inside that set (and otherwise, if we are going to error anyway, it doesn't matter that we didn't optimize this clone anymore)


const key = path[path.length - 1]
switch (op) {
case "replace":
if (isMap(base)) {
base.set(key, value)
return
}
if (isSet(base)) {
} else if (isSet(base)) {
throw new Error('Sets cannot have "replace" patches.')
} else {
// 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
}
base[key] = 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":
Array.isArray(base)
Expand Down

0 comments on commit ff25a83

Please sign in to comment.