Date becomes object after patching

[tiye]

in Chome Canary, OS X

"jiff": "^0.6.0",

code:

    console.log diff, store
    store = jiff.patch diff, store
    console.log store

its log:

diff:

[{"op":"add","path":"/messages","value":[{"id":"7ksRCQ--","time":"2014-12-03T11:40:16.816Z","userId":"7yjrlN0x","text":"df","thread":"default","isThread":false},{"id":"m1xaT7Z-","time":"2014-12-03T11:35:34.723Z","userId":"7yjrlN0x","text":"\nd","thread":"default","isThread":false},{"id":"XJ_As7bW","time":"2014-12-03T11:27:26.617Z","userId":"7yjrlN0x","text":"df","thread":"default","isThread":false},{"id":"Qktysm--","time":"2014-12-03T11:23:27.602Z","userId":"7yjrlN0x","text":"s","thread":"default","isThread":false},{"id":"71SWqm--","time":"2014-12-03T11:19:39.281Z","userId":"7yjrlN0x","text":"x","thread":"default","isThread":false},{"id":"Qkk2KmZb","time":"2014-12-03T11:18:13.355Z","userId":"7yjrlN0x","text":"d","thread":"default","isThread":false},{"id":"mJnUOX-b","time":"2014-12-03T11:12:34.643Z","userId":"7yjrlN0x","text":"f","thread":"default","isThread":false},{"id":"myvuw7Z-","time":"2014-12-03T11:08:44.971Z","userId":"7yjrlN0x","text":"sdfsf","thread":"default","isThread":false},{"id":"mJo1D7-b","time":"2014-12-03T11:06:25.369Z","userId":"7yjrlN0x","text":"che","thread":"default","isThread":false},{"id":"7J8LIXWZ","time":"2014-12-03T11:03:56.608Z","userId":"7yjrlN0x","text":"sdf","thread":"default","isThread":false}]},{"op":"add","path":"/threads","value":[]},{"op":"add","path":"/user","value":{"name":"chen","avatar":"","nickname":"","thread":"default","id":"7yjrlN0x","online":true}}]

result:

{"messages":[{"id":"7ksRCQ--","time":"2014-12-03T11:40:16.000Z","userId":"7yjrlN0x","text":"df","thread":"default","isThread":false},{"id":"m1xaT7Z-","time":"2014-12-03T11:35:34.000Z","userId":"7yjrlN0x","text":"\nd","thread":"default","isThread":false},{"id":"XJ_As7bW","time":"2014-12-03T11:27:26.000Z","userId":"7yjrlN0x","text":"df","thread":"default","isThread":false},{"id":"Qktysm--","time":"2014-12-03T11:23:27.000Z","userId":"7yjrlN0x","text":"s","thread":"default","isThread":false},{"id":"71SWqm--","time":"2014-12-03T11:19:39.000Z","userId":"7yjrlN0x","text":"x","thread":"default","isThread":false},{"id":"Qkk2KmZb","time":"2014-12-03T11:18:13.000Z","userId":"7yjrlN0x","text":"d","thread":"default","isThread":false},{"id":"mJnUOX-b","time":"2014-12-03T11:12:34.000Z","userId":"7yjrlN0x","text":"f","thread":"default","isThread":false},{"id":"myvuw7Z-","time":"2014-12-03T11:08:44.000Z","userId":"7yjrlN0x","text":"sdfsf","thread":"default","isThread":false},{"id":"mJo1D7-b","time":"2014-12-03T11:06:25.000Z","userId":"7yjrlN0x","text":"che","thread":"default","isThread":false},{"id":"7J8LIXWZ","time":"2014-12-03T11:03:56.000Z","userId":"7yjrlN0x","text":"sdf","thread":"default","isThread":false}],"threads":[],"user":{"name":"chen","avatar":"","nickname":"","thread":"default","id":"7yjrlN0x","online":true}}

[briancavalier]

Hi @jiyinyiyong, hmmm yes, I see what is happening. Dates are a general problem in JSON, since they have to be represented as strings or numbers. The issue here is that jiff.clone uses JSON.parse + JSON.stringify to clone things, and provides a custom date reviver that uses a regex to recognize ISO formatted dates. Since, in this case, patch object contains a string that matches that regex, JSON.parse revives it into a Date object.

One option might be for us to rewrite jiff.clone to use a brute force recursive copy algorithm, rather than relying on JSON.parse + JSON.stringify. Pinging @unscriptable to get his thoughts, too.

[briancavalier]

For discussion, here's a working brute force implementation. In limited testing, it avoids the Date reviver problem, and seems to be at least 3x faster than using JSON.

[tiye]

Thanks for replying.

Meanwhile I'm also trying jsondiffpatch and it made me confused about JSON diffing. So I want to ask questions how you designed this library, like for what purpose?

You see, the reason that leads me to JSON diffing is I want to copy the idea of React's DOM diffing to data. So I can regard data in the client as part of that on the server. And by JSON diffing/patching, clients are synced to the server. As it turns out, I render JSON on server from time to time, and do diff on every change, then send that diff to client, and the client just patches to be synced.

However as I tried, jiff behanves strangely sometimes(I didn't figure out why), and jsondiffpatch only diffs object references(rather than the whole text). It appears these libraries are designed for other purpose except for mine?

By the way, I used to call lodash's cloneDeep to copy JSON, it might be helpful. https://github.com/lodash/lodash/blob/master/lodash.js#L7261

[briancavalier]

So I want to ask questions how you designed this library, like for what purpose? You see, the reason that leads me to JSON diffing is I want to copy the idea of React's DOM diffing to data

jiff is intended to be a compliant implementation of the JSON Patch RFC, which defines the patch format that jiff uses. Data synchronization, along with JSON Patch over HTTP Patch, are two of the primary use cases that jiff was designed for :) In fact, I used it to implement a prototype client-server synchronization system at my last job.

However as I tried, jiff behanves strangely sometimes(I didn't figure out why)

I'd be interested to hear more about the "strange" behavior. I found it to work quite reliably when building the synchronization system.

If you can provide some examples, I'll be happy to look into them.

[tiye]

Nice to found it already used in a client-server synchronization system, that's what I want to write. I'll send new issue if I found details about that :)

[briancavalier]

@jiyinyiyong I just merged the fix from #23. It works for me, so if you have a chance, could you verify your use case? If all is good, I'll release this as v0.7.0

[tiye]

The Date Object problem is gone.

---

might not related to this issue but I found some details here. As I switched to jsondiffpatch and I ran jiff again and found this:

Server side

my code on server side:

  console.log '---'
  console.log JSON.stringify(state.cacheStore)
  console.log JSON.stringify(data)
  diff = jiff.diff state.cacheStore, data, hash: (x) -> x.id

where state.cacheStore was:

{"user":{"name":"yong","avatar":"http://tp3.sinaimg.cn/1346257214/50/1269085478/0","nickname":"yong","id":"7JHrWpG-","thread":"default","online":true},"threads":[],"messages":[{"id":"QJ951IQ-","time":"2014-12-05T02:24:31.880Z","userId":"7yjrlN0x","text":"q","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}},{"id":"mJR6kUmW","time":"2014-12-05T02:25:23.932Z","userId":"7yjrlN0x","text":"x","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}},{"id":"X1-CJLQ-","time":"2014-12-05T02:25:26.900Z","userId":"7yjrlN0x","text":"t","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}}]}

and data was:

{"user":{"name":"yong","avatar":"http://tp3.sinaimg.cn/1346257214/50/1269085478/0","nickname":"yong","id":"7JHrWpG-","thread":"default","online":true},"threads":[],"messages":[{"id":"mJR6kUmW","time":"2014-12-05T02:25:23.932Z","userId":"7yjrlN0x","text":"x","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}},{"id":"X1-CJLQ-","time":"2014-12-05T02:25:26.900Z","userId":"7yjrlN0x","text":"t","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}},{"id":"71D0JL7b","time":"2014-12-05T02:25:33.729Z","userId":"7JHrWpG-","text":"r","thread":"default","isThread":false,"user":{"name":"yong","avatar":"http://tp3.sinaimg.cn/1346257214/50/1269085478/0","nickname":"yong","id":"7JHrWpG-","thread":"default","online":true}}]}

with jsondiffpatch I got this: http://benjamine.github.io/jsondiffpatch/demo/index.html

with jiff I got this:

[{"op":"test","path":"/messages/0","value":{"id":"QJ951IQ-","time":"2014-12-05T02:24:31.880Z","userId":"7yjrlN0x","text":"q","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}}},{"op":"remove","path":"/messages/0"},{"op":"add","path":"/messages/2","value":{"id":"71D0JL7b","time":"2014-12-05T02:25:33.729Z","userId":"7JHrWpG-","text":"r","thread":"default","isThread":false,"user":{"name":"yong","avatar":"http://tp3.sinaimg.cn/1346257214/50/1269085478/0","nickname":"yong","id":"7JHrWpG-","thread":"default","online":true}}}] 

Client side

on my client I have:

    console.log '---'
    console.log JSON.stringify diff
    console.log JSON.stringify store
    store = jiff.patch diff, store

where store was(it should be same with state.cacheStore and confirmed):

{"messages":[{"id":"QJ951IQ-","time":"2014-12-05T02:24:31.880Z","userId":"7yjrlN0x","text":"q","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}},{"id":"mJR6kUmW","time":"2014-12-05T02:25:23.932Z","userId":"7yjrlN0x","text":"x","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}},{"id":"X1-CJLQ-","time":"2014-12-05T02:25:26.900Z","userId":"7yjrlN0x","text":"t","thread":"default","isThread":false,"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}}],"threads":[],"user":{"name":"chen","avatar":"http://tp1.sinaimg.cn/1651843872/180/40048616024/1","nickname":"chen","thread":"default","id":"7yjrlN0x","online":true}} 

Then I got InvalidPatchOperationError in patching:

[briancavalier]

Thanks @jiyinyiyong. That seems like a regression, I'll look into it!

[briancavalier]

@jiyinyiyong I put together a test using the data in your post: https://gist.github.com/briancavalier/43b8d5df89093239ac34

It seems to work correctly for me with latest master (I tried both the store data and the state.cacheStore data you pasted above). I tried on node 0.10.33 and 0.11.14--I'll try chrome as well. Maybe I'm not quite capturing the same situation, though. If you could tweak my gist to create a failing case, that'd be greatly appreciated.

Also: are you sure you're applying the patch to a JSON document that is compatible with the patch? If the patch target has diverged significantly from the source from which the patch was created, then the patch typically can't be applied. If that's the case, jiff has 2 features that can help: patch rebasing and contextual patching.

[briancavalier]

Confirmed that the gist works in the version Chrome I have:

[tiye]

Oh, I found problem in my last repoly. The diff ops refers to "/messages/0" and "/messages/2", while in the screenshot there is "/text/1". So the error located at somewhere else. Sorry, my mistake.

My program with diffs is really fragile at this moment.

[briancavalier]

No worries at all @jiyinyiyong, I'm glad you figured it out. I'll get 0.7.0 ready for release :)

[briancavalier]

jiff 0.7.0 just landed with the Date fix.