undefined in comparison creates invalid diff

[daynekilheffer]

For the following example:

jiff.diff(
    {
        Field1: '1'
    },
    {
        Field1: undefined
    }
);

The diff creates:

[
  { op: 'test', path: '/Field1', value: '1' },
  { op: 'replace', path: '/Field1', value: undefined },
  { op: 'test', path: '/Field1', value: '1' },
  { op: 'remove', path: '/Field1' }
]

I see two things potentially wrong with this:

1. The replace op with {... value: undefined}, when stringified, will result in a replace op with no value field which isn't valid for json-patch.
2. Even if the first two ops pass, the third op will fail since the value has been changed to undefined.

Is the expectation that I JSON.stringify and JSON.parse before jiff.diff?

[briancavalier]

Hi @daynekilheffer. I think it's kind of a subtle issue due to the fact that the inputs to jiff.diff must be valid JSON (see diff docs in README here, and undefined isn't a legal JSON value. Internally, jiff considers undefined to mean "missing" (which can be more efficient than using in or hasOwnProperty). It's unfortunately subtle since JS and JSON are so easily mixed in JS.

So, the "fix" here would be to do whatever you need to do to ensure valid JSON input to diff. Sorry for any confusion, and if you have suggestions for making that clearer in the docs, please let me know.

[unscriptable]

In javascript, we often use undefined to mean nil/nothing, when that's actually what null means. Do you think it's possible to use null instead of undefined for your use case, @daynekilheffer ?

[daynekilheffer]

@unscriptable Thank you for the suggestion. If I was trying to represent nil/nothing, then yes, you're solution would work well. Unfortunately, my issue was more in my lack of knowledge about JSON and undefined.

@briancavalier Any reason that typeof checks wouldn't be a better way to determine if the op needs to be a replaced or removed? Either way, I'll code around this to clean the js object before invoking diff. Thanks for the prompt response.

[briancavalier]

Any reason that typeof checks wouldn't be a better way to determine if the op needs to be a replaced or removed?

Good question. Unfortunately, I don't think typeof helps in the specific case of object properties, since typeof x.foo === 'undefined' and x.foo === undefined should always evaluate to the same result. I say "should" because weird host objects on older IE or misbehaving getters could actually behave differently. JavaScript is messy, so constraining the problem space to JSON allows the code to make assumptions that keep it simpler and help with performance. So, I feel like the simplest thing is just to require valid JSON input.

Thanks for the prompt response.

No problem. I appreciate the detailed original post with examples. Closing, but please feel free to continue discussing, or reopen if you feel there's a need.