Closed sean256 closed 6 years ago
Good catch! I wish I'd read your issue more closely the first go-round :( ... the solution I eventually arrived at (after a hack that didn't really get at the core issue), follows the same logic :)
The problem was that op=replace
is like op=remove
followed by op=add
, but op=remove
is supposed to fail if the target location doesn't "exist". It's not exactly clear when a "target location" exists or not; is it when there's a value defined there? or simply when the path to get there exists?
Suppose we have a JSON Pointer, p
, with path "/k"
, in the context of an object obj
. Then, the target location indicated by p
"exists" iff:
obj.hasOwnProperty('k')
returns true
(in which case the existence check in patch.remove
and patch.replace
should fail on !parent.hasOwnProperty(key)
instead of value === undefined
)or, alternatively:
obj.hasOwnProperty('k')
returns true and obj.k
is not strictly equal to undefined
(which is based on how JSON treats undefined
— as you pointed out with your workaround that cleans the value with a JSON.parse ∘ JSON.stringify
cycle)The solution in a87214a implements the latter (2). To reuse your sample, the new behavior looks like this:
const {createPatch, applyPatch} = require('rfc6902')
const a = {name: 'bob', image: undefined, cat: null}
const b = {name: 'bob', image: 'foo.jpg', cat: 'nikko'}
createPatch(a, b)
//=> [ { op: 'add', path: '/image', value: 'foo.jpg' },
//=> { op: 'replace', path: '/cat', value: 'nikko' } ]
applyPatch(a, createPatch(a, b))
//=> [ null, null ]
a
//=> { name: 'bob', image: 'foo.jpg', cat: 'nikko' }
I haven't published a new version yet, but I think a behavior change this deep (albeit subtle, and clearly an improvement in correctness) merits incrementing the major version. Lemme think on that.
Nice! I'm surprised no-one came across this yet. I would agree that this is probably a major version despite how simple it is :-/ I could totally see this "fixing" something that someone wasn't even aware was broken and it causing trouble.
By the way, I want to call out how awesome this module is. I tried almost every patch library out there and yours consistently generates the smallest patches!
runkit: https://runkit.com/embed/t341yubb8g39
After applying the patch to
a
image is NOT updated tofoo.jpg
. If I excludeimage
instead of having it asundefined
it works as it creates anadd
operation instead ofreplace
. This is causing me some headaches right now as a framework I'm stuck with generatesundefined
values. When passing these objects over JSON theundefined
values are stripped which really makes thereplace
nonsense.If anyone else is having this issue I found a workaround by purging all undefined properties