RexSkz / json-diff-kit

A better JSON differ & viewer, support LCS diff for arrays and recognise some changes as "modification" apart from simple "remove"+"add".
https://json-diff-kit.js.org
MIT License
138 stars 12 forks source link

`undefined` results in error #28

Closed hanselsen closed 9 months ago

hanselsen commented 9 months ago

Thanks for the library. I am very happy with it. There was one small error that was quite hard to find.
Sometimes I had undefined as a value in my object. This caused the library to crash with the message:

Cannot read properties of undefined (reading 'split')

Could you treat undefined as null?

I have created a reproduceable demo https://codepen.io/hanselsen/pen/oNVERGv

RexSkz commented 9 months ago

It's not difficult to treat undefined as null, but considering undefined fields will not appear in the stringify result or request body, it should be an invalid scenario.

I suggest we stringify it to a string like "%!invalid(undefined)". We can handle other invalid scenarios using the same method:

undefined       -> '%!invalid(undefined)'
() => alert(1)  -> '%!invalid(function)' // there's no easy way to format a function, so just use "function" instead of the real value
1n              -> '%!invalid(1n)'
Symbol.iterator -> '%!invalid(Symbol.iterator)'
hanselsen commented 9 months ago

If I may suggest something else. How about an extra configuration parameter to setup the undefined behaviour.

const UndefinedBehavior = {
    ignore: 0, // Considers the key/value as non-existent
    replaceWithNull: 1, // Makes it `null`
    replaceWithString: 2 // Makes it `'undefined'`
}

const differ = new Differ({
    detectCircular: true,
    maxDepth: Infinity,
    showModifications: true,
    arrayDiffMethod: 'lcs', 
    undefinedBehavior: UndefinedBehavior.ignore
});
RexSkz commented 9 months ago

It seems that undefinedBehavior can also be applied to other types that are not part of JSON, it's a good idea.

I think the default value of it should be replaceWithString since undefined and null have different meanings and should not be confused.

RexSkz commented 9 months ago

Actually when I started to write doc comments, copilot suggests me to use these options throw, ignore, stringify.

interface DifferOptions {
  /**
   * The behavior when encountering values that are not part of the JSON spec, e.g. `undefined`, `NaN`, `Infinity`, `123n`, `() => alert(1)`, `Symbol.iterator`.
   *
   * - `UndefinedBehavior.throw`: throw an error
   * - `UndefinedBehavior.ignore`: ignore the key-value pair
   * - `UndefinedBehavior.stringify`: try to stringify the value
   */
  undefinedBehavior?: UndefinedBehavior;
}

It makes sense, and I think the default value should be UndefinedBehavior.stringify.

hanselsen commented 9 months ago

That works for me.

RexSkz commented 9 months ago

Supports in 1.0.22