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

DateTime does not have toJson. #16628

Closed
DartBot opened this issue Feb 7, 2014 · 13 comments
Closed

DateTime does not have toJson. #16628

DartBot opened this issue Feb 7, 2014 · 13 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue library-core type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Feb 7, 2014

This issue was originally filed by @Emasoft


BUG: Dart’s DateTime class doesn't accomplies with ISO 8601 via its parse method.

For example, the following doesn't work:

var dt = DateTime.parse("2014-01-26T11:38:17");
print(JSON.encode(dt));

As of Dart 1.1.1, this will throw:

Unhandled exception:
Converting object to an encodable object failed.

­0 _JsonStringifier.stringifyValue (dart:convert/json.dart:416)

­1 _JsonStringifier.stringify (dart:convert/json.dart:336)

­2 JsonEncoder.convert (dart:convert/json.dart:177)

­3 JsonCodec.encode (dart:convert/json.dart:106)

FIX: fix ISO 8601 compliance for the default JSON serialization of DateTime.

@dgrove
Copy link
Contributor

dgrove commented Feb 7, 2014

Added Area-Library, Library-Core, Triaged labels.

@lrhn
Copy link
Member

lrhn commented Feb 7, 2014

There are several ways a DateTime object can be converted to a JSON serializable value.

I am guessing you want it to convert the DateTime to a simple String value.

This is possible, e.g., by adding a toJson method to DateTime.
I am wary at doing so because there is no corresponding way to make JSON.decode convert it back, that doesn't rely on inspecting every string value to see if its format matches.


Removed Type-Defect label.
Added Type-Enhancement label.

@floitschG
Copy link
Contributor

This issue doesn't seem to have anything to do with ISO8601 or DateTime.parse, so I changed the summary.

I agree with Lasse: there is no reason why DateTime should have a toJson method and silently convert to a string when using JSON. Users can very easily provide an extra argument to JSON.encode that encodes DateTimes.

The only classes that convert directly to JSON are the objects that can be read back. All the others (Set, Duration, Point, ...) must be handled by the user.

In other words: JSON.decode(JSON.decode(x)) should be equivalent to x.


Changed the title to: "DateTime does not have toJson.".

@lrhn
Copy link
Member

lrhn commented Feb 10, 2014

We have no current plan to add default JSON serialization to any types that are not basic JSON types (num, bool, String, Null, List and Map<String,?>), for the reasons Florian explained - it would not be deserializable.


Added NotPlanned label.

@kevmoo
Copy link
Member

kevmoo commented Feb 15, 2014

Issue #16843 has been merged into this issue.

1 similar comment
@kevmoo
Copy link
Member

kevmoo commented Feb 15, 2014

Issue #16843 has been merged into this issue.

@DartBot
Copy link
Author

DartBot commented Feb 15, 2014

This comment was originally written by @MaxHorstmann


"Users can very easily provide an extra argument to JSON.encode that encodes DateTimes"

True, but this means that different users will potentially come up with (slightly) different serialization formats for DateTime. And trust me, they will. Some will pick

  var json = JSON.encode(dt, toEncodable: (DateTime d) => d.millisecondsSinceEpoch);

while other will choose

  var json = JSON.encode(dt, toEncodable: (DateTime d) => d.toString());

(by the way, the latter does not produce ISO 8601 - but it's close).

This increases friction and decreases interoperability e.g. between different APIs. DateTime is such a common, ubiquitous data type (and unfortunately hasn't been included in ECMA-404) that IMO it deserves special treatment. The benefits of a default ISO 8601 serialization, IMO, by far outweigh the drawbacks.

For the roundtrip/deserialization issue, I think it can be mitigated by using types. Typically, users won't serialize primitives but entity objects such as

class User
{
  int id;
  DateTime created;
  // ... more ...
}

@lrhn
Copy link
Member

lrhn commented Feb 15, 2014

The problem is that no matter what default encoding we chose for DateTime, it will still need a custom reviver to make it decode to a DateTime object.
The JSON format does not have any syntax for a Date, so the plain JSON decoder will just see whatever string or integer the DateTime was encoded as.

Since it will always require a custom reviver to decode the DateTime, I think it's better to have a pair of custom toEncodable/revivier, instead of haveing one be default and the other not.

@DartBot
Copy link
Author

DartBot commented Feb 15, 2014

This comment was originally written by @Emasoft


You have your priorities inverted. We need a datetime default json serialization format in Dart, because we need a default object json serialization format. And we need a default object json serialization format because otherwise data interchange is gonna be another hell.
What good is creating a new language as Dart, if you don't learn anything from the errors of the previous languages?
I can assure you that in a couple of years you'll be forced to implement a default json serialization for all data types, because people will be tired of coping with the mess resulting from the lack of it.

Please read this article about the same problem faced by the JSON.NET library to see what I mean:

http://www.hanselman.com/blog/OnTheNightmareThatIsJSONDatesPlusJSONNETAndASPNETWebAPI.aspx

@floitschG
Copy link
Contributor

JSON is a limited format (not our fault). We encode what it supports, and let users decide how to deal with the rest. Contrary to other languages we allow to do this in an elegant way (toEncodable) that is flexible enough to support all conventions.

There is just no need to add a toJson() on DateTime.

However, I do agree that DateTime should have a toIso8601() so that this conversion becomes easier.

@lrhn
Copy link
Member

lrhn commented Feb 15, 2014

I don't disagree that we need to have some serialization library.
It doesn't have to serialize to JSON. If possible, it could have several interchange formats.
But JSON.encode is not a serialization library.
The goal of JSON.encode is to be able to recode the result of JSON.decode, which again has the goal of parsing the plain JSON format.
Plain JSON is a simple data format. It cannot handle cyclic structures. Any attempt at serializing object graphs to JSON will have to have an extra layer of abstraction on top of the plain JSON structure.

@DartBot
Copy link
Author

DartBot commented Feb 16, 2014

This comment was originally written by @MaxHorstmann


| However, I do agree that DateTime should have a toIso8601() so that this conversion becomes easier.

+1, this would already be a big improvement IMO.

@kevmoo
Copy link
Member

kevmoo commented Feb 17, 2014

See https://code.google.com/p/dart/issues/detail?id=16902 for toIso8601 feature request

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core closed-not-planned Closed as we don't intend to take action on the reported issue labels Feb 17, 2014
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed priority-unassigned labels Mar 1, 2016
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants