Potentially undesired behavior in duplex.ts with toJSON and Date objects

[dounan]

On line 194, there is a check to see if the object has a toJSON function, and sets obj = obj.toJSON() if there is. https://github.com/Starcounter-Jack/JSON-Patch/blob/406a6ddba0165271e4e767ff9c41bcbbebdf307e/src/duplex.ts#L194

The test for this case creates a function that returns an Object for the toJSON() function: https://github.com/Starcounter-Jack/JSON-Patch/blob/406a6ddba0165271e4e767ff9c41bcbbebdf307e/test/spec/duplexSpec.js#L586

However, the javascript Date object also has a toJSON method, but it returns a string instead of an Object. Here is the output of comparing two date objects:

jsonpatch.compare(new Date('2017-01-01'), new Date('2017-01-01'));
[ { op: 'add', path: '/0', value: '2' },
  { op: 'add', path: '/1', value: '0' },
  { op: 'add', path: '/2', value: '1' },
  { op: 'add', path: '/3', value: '7' },
  { op: 'add', path: '/4', value: '-' },
  { op: 'add', path: '/5', value: '0' },
  { op: 'add', path: '/6', value: '1' },
  { op: 'add', path: '/7', value: '-' },
  { op: 'add', path: '/8', value: '0' },
  { op: 'add', path: '/9', value: '1' },
  { op: 'add', path: '/10', value: 'T' },
  { op: 'add', path: '/11', value: '0' },
  { op: 'add', path: '/12', value: '0' },
  { op: 'add', path: '/13', value: ':' },
  { op: 'add', path: '/14', value: '0' },
  { op: 'add', path: '/15', value: '0' },
  { op: 'add', path: '/16', value: ':' },
  { op: 'add', path: '/17', value: '0' },
  { op: 'add', path: '/18', value: '0' },
  { op: 'add', path: '/19', value: '.' },
  { op: 'add', path: '/20', value: '0' },
  { op: 'add', path: '/21', value: '0' },
  { op: 'add', path: '/22', value: '0' },
  { op: 'add', path: '/23', value: 'Z' } ]

Maybe this is the intended behavior, but just want to point it out just in case.

[alshakero]

Thank you so much for reporting the issue this nicely! While I see the issue with this, removing or changing this line wouldn't do Date objects any good. Dates are Javascript objects. Not JSON objects. We only support serializable JSON objects, and if you look at the RFC we implement it says:

JSON Patch defines a JSON document structure for expressing a sequence of operations to apply to a JavaScript Object Notation (JSON) document; it is suitable for use with the HTTP PATCH method. The "application/json-patch+json" media type is used to identify such patch documents.

So even if we remove this call, Dates will still be unserialzable and unsupported by JSON-Patch protocol and accordingly our library.