Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Date comparison always yields a patch #4

Closed
Starcounter-Jack opened this issue Apr 30, 2013 · 4 comments
Closed

Date comparison always yields a patch #4

Starcounter-Jack opened this issue Apr 30, 2013 · 4 comments

Comments

@Starcounter-Jack
Copy link
Owner

Originally described here

When you do a comparison on an object that includes dates, all dates end up resulting in patches. This is because JSON.parse(JSON.stringify)) is used to generate an internal cache object, but that does not result in an object with identical values. Dates get converted to strings:

d = new Date();
foo = {x: 10, y: "test", z: d};
jsonpatch.observe(foo);
bar = jsonpatch.observe(foo);
jsonpatch.generate(bar);
@jrogelstad
Copy link

I did a little investigation into various deep copy methods, as I'm sure you have as well, and my conclusion was that the more important it is for the copy to be "correct" the greater the overhead involved to implement it. For example here's a pretty involved algorithm to do deep copy:

http://oranlooney.com/static/javascript/deepCopy.js
...that is described here:
http://oranlooney.com/deep-copy-javascript/

I didn't run any tests, but just looking at it I'm sure it will slow the generator down substantially, which kind of defeats the main stated benefit of this project over others. For my project I just implemented a simple work around that converts all dates to JSON strings before the object ever gets to the observer/generator:

https://github.com/xtuple/xtuple/blob/master/lib/backbone-x/source/model.js#L331
https://github.com/xtuple/xtuple/blob/master/lib/backbone-x/source/model_mixin.js#L238

If I stumble upon a better idea I will circle back on this.

Best,

John

@warpech
Copy link
Collaborator

warpech commented Jul 3, 2013

I tried to fix it in the shimmed version, but I abandoned it because it won't run in the native (Object.observe) version anyway.

Reason: Object.observe can't detect changes in the Date object (tested in Chrome 28 and Chrome 30 Canary) - http://jsfiddle.net/ytz93/

@Starcounter-Jack
Copy link
Owner Author

As Date is not a value type and as we want to be compatible with Ecmascript Next Object.observe, I'm now closing this. I'll reopen the issue if anyone can provide some compelling new insight on the subject.

@Toub
Copy link

Toub commented May 19, 2016

So, how do we generate patches for dates?
Maybe that custom comparison function could help: #92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants