Opened 10 years ago

Closed 7 years ago

Last modified 7 years ago

#5726 closed enhancement (fixed)

dojo.toJson() doesn't serialize dates

Reported by: kriszyp Owned by: Kris Zyp
Priority: high Milestone: 1.7
Component: General Version: 1.0
Keywords: json Cc: alex, Nathan Toone, eugene
Blocked By: Blocking:

Description

Both ES4 (http://wiki.ecmascript.org/doku.php?id=proposals:json_encoding_and_decoding&s=json) and Douglas Crockford's JSON library serialize dates to ISO 8601 strings. Currently Dojo serializes a Date to {}, which does not seem that helpful. I think we should use the same format as the others.

Attachments (2)

json.diff (605 bytes) - added by Kris Zyp 9 years ago.
json.js patch
MSApproachJsonDate.patch (1.1 KB) - added by 7twenty 9 years ago.
Another approach to serializing Date objects to JSON

Download all attachments as: .zip

Change History (35)

comment:1 Changed 10 years ago by Adam Peller

see #4970. If you could attach the files as patches against svn, that would be great.

comment:2 Changed 10 years ago by Adam Peller

the thing that's always bothered me about Crockford's solution, though, is that it's asymmetrical. You can't have a Date object in the return data, of course. It seems to lend itself more to a registry approach, where certain data can be tagged, perhaps by property naming conventions or explicit wiring. Furthermore, it's unclear whether the ISO format is intended to be more capable, like the one in dojo.date.stamp. Did that ever get resolved in ES4?

comment:3 Changed 10 years ago by kriszyp

Yes, it is unfortunate that the Date->ISO string is asymmetric, but it certainly isn't as asymmetric as serializing Date -> {}. It does require some knowledge or pattern matching to parse date strings back to Date objects. I am hoping to write a json parse/stringify augmenter that could help with this, being advised by JSON Schema, but for base functionality, I still believe it is best to follow Crockford's approach here. I am not sure what is meant by "more capable". I will attach my changes as diff/patches.

comment:4 Changed 10 years ago by Adam Peller

Kris, have you looked at dojo.date.stamp? It allows partial dates/times and is a compromise between the ISO spec and what's used in the W3C specs, with an eye towards performance and code size. It would be nice to reuse that functionality, if appropriate, and use that for an augmented json schema. Note that there used to be a 'registry' approach to the json code, but I guess that was abandoned. I'd prefer a pluggable approach rather than baking it in base.

comment:5 Changed 10 years ago by kriszyp

I have looked at dojo.date.stamp. Do you want an alternate serialization of Dates for json? Still hesitant to diverge from Crockford here. Or do you want the json serializer to call dojo.date.stamp.toISOString? dojo._base.json is a _base file, and dojo.data.stamp is not. Would you really want that dependency? As far augmenting json parsing/serialization with json schema, I would certainly do it as a plugin (in dojox). I would use AOP to add functionality when desired (still thinking about this).

comment:6 Changed 10 years ago by Adam Peller

dojo.date.stamp is a superset of what Crockford has, and as you say, his proposal is not final and I don't think it was even finalized by ES4.

base code can't call core code, but if you had an augmentation in dojox or dojo core, it could certainly call it. It could also be a 'mixin' in dojo core (see dojo.fx for an example) Easier to move code into base than out.

comment:7 Changed 10 years ago by kriszyp

I think the fancy stuff, like date parsing should be an augmentation from dojox. However, basic Date serialization seems like it is currently broken (outputting {}), and should be fixed in base, and it seems like avoiding dependency issues with dojo.data.stamp and simply using the providing patch would be the easiest solution. Or are you concerned about the patch being too big?

comment:8 Changed 10 years ago by Adam Peller

Cc: alex added

A little concerned about code size, but mostly arguing that an asymmetrical solution is potentially confusing and that it's best to solve in one place, not two. But I don't feel all that strongly about it.

Marking #4970 as a dup of this.

comment:9 Changed 10 years ago by bill

Component: GeneralDate
Milestone: 1.11.2
Owner: changed from anonymous to Adam Peller

Move all milestone 1.1 tickets to 1.2, except for reopened tickets and tickets opened after 1.1RC1 was released.

comment:10 Changed 10 years ago by Adam Peller

Component: DateGeneral
Owner: changed from Adam Peller to alex

I don't really own this. Alex probably needs to look at this and decide whether to resurrect AdapterRegistry? or something of that sort.

Changed 9 years ago by Kris Zyp

Attachment: json.diff added

json.js patch

comment:11 Changed 9 years ago by Kris Zyp

I attached a new patch. The patch reduces the code size of json.js, and provides at least a semi-coherent serialization of dates (still not symmetrical, but less assymetrical than it used to be).

comment:12 Changed 9 years ago by Adam Peller

I'm +1 on the code size change, -1 on the date. It should be ISO or nothing. Date.toString() is horrible and does implementation- (browser) specific things. I'd rather have something that causes an error than provide data that's inconsistent. Also, should it be it === null ?

comment:13 Changed 9 years ago by Adam Peller

nm, you meant ==

comment:14 Changed 9 years ago by Kris Zyp

The code size improvement could be done without the Date change. The date change costs (approximate bytes increase after minification):

  1. Date -> {}: no change
  1. Date -> it.toString(): +18 bytes
  1. Date -> ISO String: +210 bytes
  1. Date -> "new Date(" + it.getTime() + ")": +61 bytes (this isn't valid JSON, but it is perfectly symmetric)

Any preference?

comment:15 Changed 9 years ago by Adam Peller

Option (e) is some sort of mixin to provide this functionality, like

if(dojo.date.stamp){ ... }

or do a mixin module, which is kind of weird, I know. Is there precedence for that?

comment:16 Changed 9 years ago by Kris Zyp

Yes, you are right, options a,b, and d can be combined with option e "ISO if available": if(dojo.date.stamp){

return dojo.date.stamp.toISOString(it);

} It adds an additional ~60 more bytes, and it has different behavior based upon what modules are loaded.

comment:17 Changed 9 years ago by Kris Zyp

Here is a variation on ISO that reduces the weight to 124 bytes (it is not perfectly ISO, it doesn't convert to double digit for single digit dates and months): if(it instanceof Date){

return it.toUTCString().replace(/(.*?(\d+).*?(\d+)) (.*) \w*/,"$3-" + it.getUTCMonth() + "-$2T$4Z")

}

comment:18 Changed 9 years ago by Dustin Machi

Type: defectenhancement

comment:19 Changed 9 years ago by bill

Milestone: 1.21.3

bump enhancements to next milestone, as we prepare to close out 1.2

comment:20 Changed 9 years ago by bill

Milestone: 1.3future
Summary: Serializing Dates to JSON should in ISO 8601 format (UTC time)dojo.toJson() doesn't serialize dates

Also #4970

comment:21 Changed 9 years ago by Nathan Toone

Cc: Nathan Toone added

comment:22 Changed 9 years ago by Eugene Lazutkin

Cc: eugene added

comment:23 Changed 9 years ago by 7twenty

Microsoft uses an approach that is both symmetric and standard JSON: http://weblogs.asp.net/bleroy/archive/2008/01/18/dates-and-json.aspx

I've created a patch that adds very little code and correctly round trips Date objects:

dojo.fromJson(dojo.toJson({date: new Date()}))

I've done minimal testing, but it worked correctly on FF 3.0.5, IE7, and Chrome.

Changed 9 years ago by 7twenty

Attachment: MSApproachJsonDate.patch added

Another approach to serializing Date objects to JSON

comment:24 Changed 9 years ago by Nathan Toone

BTW - using that new approach to serialization adds 135 bytes (57 bytes gzipped) to base.

comment:25 Changed 9 years ago by Adam Peller

Yikes. People have been wrestling with this problem for a while and while this may be the closest I've seen to a type-specific solution in JSON, well, there are a couple of reasons not to follow MS here. One is that Crockford does have a solution offered in ES31, so this would diverge from the standard. (We should probably consider supporting stringify/parse) MS does seem to rely on a real hack and not be generalizable, and by not using ISO format, it invites more timezone problems, etc. I know it's unfortunate that the reviver/replacer system offered in ES31 ends up doing this more on a schema (particular named properties or property patterns) rather than a type, but that's because unfortunately JS doesn't have a native Date type, so it really just doesn't fit well into JSON to do it any other way.

comment:26 Changed 9 years ago by Nathan Toone

Yeah - I don't figure that the patch will be good enough for inclusion into base any time soon - but at least it provides a solution for someone who it might be "good enough" for...and doesn't mind patching their own source.

However, I don't quite follow on the timezone issues that it brings...I thought that date.getTime() and new Date(seconds) are based off of GMT...

comment:27 Changed 9 years ago by Adam Peller

See also #8111. Matching these signatures would provide a solution the problem, though maybe not in a way that makes everyone happy.

@toonetown: well, one reason not to use the Unix epoch is because it assumes a particular time zone. If you want to represent a particular date, without regards to absolutes -- a common scenario -- you'll inevitably end up off by a day in some other timezone.

comment:28 Changed 9 years ago by Nathan Toone

Hmm - also, this patch seems to cause performance issues when using xhrGet with handleAs of json if the payload is large...

comment:29 Changed 7 years ago by Chris Mitchell

Owner: changed from alex to dylan

please review/triage

comment:30 Changed 7 years ago by dylan

Milestone: future1.8
Owner: changed from dylan to kriszyp

Decide what to do with this in 1.8, in time for 2.0.

comment:31 Changed 7 years ago by bill

Owner: changed from kriszyp to Kris Zyp

comment:32 Changed 7 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

Fixed with latest checkin. Note, that there are some differences between how JSON.stringify(new Date(12)) works in browsers: IE8: "1970-01-01T00:00:00Z" WebKit? & FF: "1970-01-01T00:00:00.012Z" IE6-7 (our code): "1970-01-01T00:00:00.01Z"

comment:33 Changed 7 years ago by Chris Mitchell

Milestone: 1.81.7
Note: See TracTickets for help on using tickets.