Initial proposal for the experimental GraphQLDateTime scalar #557

excitement-engineer · 2016-11-05T20:26:01Z

This PR contains an initial proposal for a graphQL DateTime scalar following the discussion in issue #550. Let me know what you think:)

The scalar has the following properties:

It follows the ISO 8601 date specification.
All dates are represented in UTC.
It serializes javascript Date instances into ISO 8601 date strings in the format YYYY-MM-DDThh:mm:ss.sssZ.
It parses date strings into javascript Date instances. The following formats are supported for the date strings:

'YYYY',
'YYYY-MM',
'YYYY-MM-DD',
'YYYYMMDD',
'YYYY-MM-DDThhZ',
'YYYY-MM-DDThh:mmZ',
'YYYY-MM-DDThhmmZ',
'YYYY-MM-DDThh:mm:ssZ',
'YYYY-MM-DDThhmmssZ',
'YYYY-MM-DDThh:mm:ss.sssZ',
'YYYY-MM-DDThhmmss.sssZ',
'YYYY-Www',
'YYYYWww',
'YYYY-Www-D',
'YYYYWwwD',
'YYYY-DDD',
'YYYYDDD'

I had to include moment as a dependency in order to perform all the date operations.

The list of supported date formats was retrieved from wikipedia.

On wikipedia they mentioned that ±YYYYY is used "To represent years before 0000 or after 9999, the standard also permits the expansion of the year representation but only by prior agreement between the sender and the receiver.". From this description I concluded that this format is not in the standard ISO 8601 spec so I did not include it in the scalar.

stubailo · 2016-11-08T01:29:46Z

Is there any way for a client to know that something is a Date without needing to import the schema ahead of time? I suppose ID is already a precedent for a type that is indistinguishable from String.

Would it be better to serialize as something like:

{
  "type": "DateTime",
  "value": "ISO date here"
}

I guess REST APIs often just return dates as ISO strings so this isn't really necessary, but I wonder what people's thoughts are.

excitement-engineer · 2016-11-08T07:26:06Z

What would be your use-case for giving the client this information?

I could imagine a cool use case where graphql client libraries such as apollo and relay could parse a DateTime scalar and automatically convert it to the native date type. For example, a swift client could automatically create and return NSDate instances. This would prevent developers from having to convert string dates to native date types themselves.

langpavel · 2016-11-09T00:37:35Z

Citing myself from #550:

History show us that there is no Date without (Time or Timezone)...

Most bad thing is date without explicit timezone.

All debates around with or without timezone are problematic because we all have local perception of spoken (=> written into form => stored in DB) time.

For me only acceptable format is timestamp expressed as number of (milli)seconds from (UNIX) big bang.

So, consider real number as one possible input for coercion..

I have something for this theme here: GraphQLTimestamp

langpavel · 2016-11-09T00:42:50Z

BTW @excitement-engineer I must appreciate your intention but I really don't like adding new dependencies to reference implementation.

Reason: It introduces uncertainty to this reference implementation

leebyron · 2016-11-09T19:54:27Z

package.json

@@ -36,7 +36,8 @@
    "prepublish": ". ./resources/prepublish.sh"
  },
  "dependencies": {
-    "iterall": "1.0.2"
+    "iterall": "1.0.2",
+    "moment": "^2.15.2"


I think depending on the moment library for this one experimental feature is probably not what we want. Is it possible to get the same behavior with just a little bit of specific code atop the built-in Date object (accepting that super old versions of Node and browsers might warp behavior)?

For example, could we just use Date's toISOString() instead of converting it through moment first? And could we use Date.parse() which claims to parse ISO format, but protect it from parsing the other formats with a validating regex?

I completely agree with you on this one! I had some doubts about whether to include moment as a dependency because it is only used for a very small proportion of the code. There are some features of moment that I used:

Checking for a valid date is very easy using the isValid() function. I ran into the problem that the invalid date 2015-02-29 was regarded as a valid date in the Date object (because days are numbered from 1-31).

Checking for correctly formatted date strings is very easy in moment (you can specify an array of valid date formats). I initially tried using a single validating regex to validate all the supported date formats but it quickly became too complex to understand.

I will try to overcome these problems:) I will try to write multiple easy to understand validating regex for verifying that the date string being parsed is in ISO date format and remove the moment dependency.

I think that the important thing when doing these operations that the code remains readable and understandable. So I will make sure to write code this way:)

leebyron · 2016-11-09T20:17:57Z

src/type/scalars.js

+  name: 'DateTime',
+  description: 'An ISO-8601 encoded UTC date string.',
+  serialize(value: mixed): string {
+    if (!(value instanceof Date)) {


I think serialize should be more accepting of input than just asserting Date instances. For example, iso date strings should probably also be supported here.

Also, I'm curious how convenient we should try to be for unix timestamps? It's really common for datetimes to actually be stored as unix times in databases, and I fear that if we don't make it easy to represent these as DateTime that they'll be lazily be represented as Int instead. What do you think?

Good point! We should promote the use of this scalar by making it very convenient to link to any dateTime representation that are used by the underlying systems: Date, ISO string or unix timestamp. I will include support for all of these!

Thanks for the great tips!

I will update the PR in the weekend:)

leebyron · 2016-11-09T20:39:58Z

Is there any way for a client to know that something is a Date without needing to import the schema ahead of time?

@stubailo, there isn't a way for a client to know anything without looking at the schema, so I'm not sure it makes sense to try to create new wrapper types for something like this. If the alternative is using the String type and happening to send ISO dates, that doesn't leave a client any worse or better off. You should think of this purely as an improvement to the schema (which means improvement to validation and error messages as well). So if you're building a schema-less client, then such a change should be invisible.

Most bad thing is date without explicit timezone.
For me only acceptable format is timestamp expressed as number of (milli)seconds from (UNIX) big bang.

@langpavel I disagree. Unix timestamps are great for capturing exact machine time for the present day, but terrible for human time that we often have to represent in products. The datetime ISO standard was designed to represent the real ambiguity that needs to exist to represent a full range of times described by humans when talking about relative times, events, and things that will happen in the future.

We have this a similar kind of DateTime representation at Facebook for our Events/Calendar product, and use it all the time. For example:

What time is Happy Hour? It's 5 o'clock. What timezone? Whichever you're standing in. What's the unix timestamp for that? There isn't one.
When does Christmas start? December 25th just after midnight. What year? Whichever year you're in now. What timezone? Whatever timezone you'll be in during christmas. What's the unix timestamp for that? Undefined!
You're planning on retiring in 2050? What's the unix timestamp for that? Not enough bits to represent.

Not to say that unix timestamps aren't great or aren't suitable for APIs. When your API is depicting moments by machines and for machines, unix time can be a great choice. But if your API is depicting human times, then you need something like ISO time.

langpavel · 2016-11-10T12:36:06Z

@leebyron Really nice examples, thanks for them!
But as always when talking about date and time it brings more question then answers.
Does this mean if GraphQL will support DateTime it should support Timestamp too (like databases do)?

leebyron · 2016-11-10T19:18:23Z

That's a great question, I'm not sure! I think part of what evaluating this PR should entail is how this kind of scalar should be introduced. If it should be considered a candidate for addition to the spec, then it should be included in this reference implementation. If not, then perhaps we fold this out into another npm module that just makes it easy to get common behavior.

stubailo · 2016-11-11T00:28:26Z

I'd say one way to resolve the question of Date/DateTime/Timestamp is to map the type onto what it's most likely to turn into on the client, since GraphQL seems to be client-focused. In JavaScript, I'd suggest that Date is the most common way to represent dates. Is there an equivalent in Swift/Obj-C/Java?

excitement-engineer · 2016-11-12T21:29:00Z

I have updated the PR:

Removed the 'moment' dependency.
Used javascript Date for parsing ISO date strings. The formats that Date can parse is more limited than was possible with moment (see this document for the supported formats). This limitation has forced me to reduce the supported formats. However, I think that this is not such a problem because it makes the API simpler will make it easier for other languages to implement this scalar if this were to be included in the spec. The following formats are supported:

YYYY
YYYY-MM
YYYY-MM-DD,
YYYY-MM-DDThh:mmZ
YYYY-MM-DDThh:mm:ssZ
YYYY-MM-DDThh:mm:ss.sssZ

The Date class in javascript assumes that each month has 31 days. This means that dates like 30 February are considered 'valid' dates. I think that the GraphQL spec should not include this behaviour but rather throw an error when an invalid date is being parsed or serialized. I wrote some custom logic to handle this extra validation.
The serialize function now accepts date string, unix timestamps and Date instances.
- Unix timestamps are defined in the code as: the number of seconds since January 1st 1970 however the javascript Date class defines it as the number of milliseconds since January 1st 1970. Therefore, I wrote a simple conversion from seconds to milliseconds to overcome this issue. Do you agree with the definition of a Unix timestamp that is used in this scalar or should we use milliseconds?
- When serializing a date string a check is performed to see whether it is a valid ISO date. If this check passes then the date string is simply returned. This means that if 2016-01 is passed as input then it is also returned by the serialize function. This causes an inconsistency in the date format returned by the serialize function; the format YYYY-MM-DDThh:mm:ss.sssZ is returned when a Date or timestamp is serialized, for date string any of the supported formats may be returned. What do you guys think, do we always want to output a consistent format or is the current behaviour ok?
We have discussed adding a Unix timestamp as a possible input for the parseValue function. Should we support this in the scalar? I like the consistency of the current implementation because a client making requests and receiving responses from a GraphQL layer will only ever have to deal with ISO 8601 compliant date strings; adding a timestamp will break this consistency. Furthermore, I am wondering how many valid use cases there are for client applications sending timestamps to a graphQL layer, I have never represented dates as Unix timestamps on a client so I can't overlook how useful such a feature would be.

Curious to hear your comments on the updates:)

excitement-engineer · 2016-11-12T21:41:50Z

Travis has failed to build because node version 4 and 0.12 consider `2016-01-01T24:00Z as an invalid date format while version 6 and 7 don't. I have updated the code to restrict the hour range from 0 to 23 to prevent these inconsistencies.

excitement-engineer · 2016-11-12T23:06:31Z

@stubailo you can use NSDate for this in obj-c and LocalDateTime in java. Would be great if the graphql clients could map straight to these types, would safe the developer a lot of hassle in performing conversions:)

OlegIlyenko · 2016-11-14T22:01:45Z

@excitement-engineer @stubailo on JVM it's a bit more involving... LocalDateTime and ZonedDateTime would be indeed a good choice on java 8, but AFAIK graphql-java is still keeping compatibility with java 6. this means that they only can use java.util.Date. Plus there is also a java.util.Calendar class which some people prefer.

In my experience though, java.util.Date is not the best choice to represent the date, so most project are actually using joda-time lib (at least on java < 8). I guess the standard GraphQL type can be expressed in terms of java.util.Date, but in most of the project people will still need to covert it to something else :)

For the moment being, I decided to just document it in sangria. So people need to implement it themselves.

On the other hand, some data transfer formats naively support dates (like Amazon Ion). For this, i implemented a mechanism to communicate these capabilities of a data format to a scalar type, so that it can output raw date objects (without any conversion to string), but only when data format supports it. (this is why you see if (caps.contains(DateSupport)) in a DateTime scalar type definition)

Also, considering all of the patterns you have described:

YYYY
YYYY-MM
YYYY-MM-DD,
YYYY-MM-DDThh:mmZ
YYYY-MM-DDThh:mm:ssZ
YYYY-MM-DDThh:mm:ss.sssZ

In my opinion, it would not be correct to represent them with a single data type. For example in java 8 java.time package defines following classes to represent things you have described with patterns:

Year
YearMonth
LocalDate
ZonedDateTime

I personally would model them in the same way in my GraphQL schema. In fact this is what I do in my projects. For example, I have following GraphQL types in my schema:

DateTime - date with timezone in ISO calendar system
Date - just date without a timezone in ISO calendar system
Time - time without a timezone in ISO calendar system

(I don't use Year or YearMonth yet)

excitement-engineer · 2016-11-17T09:56:31Z

I have used the 'java.time' package in java and it works quite well! I like the separation between the different date/time formats. It enforces a stronger contract when it comes to date/time representations.

@leebyron may I ask what conventions Facebook's GraphQL API maintains for representing dates/times, do you use a single scalar or several (for representing Date, Time, DateTime, YearMonth etc. individually)?

excitement-engineer · 2016-11-26T18:51:57Z

Any updates on the status of this PR?

jamesgorman2 · 2016-11-27T00:32:41Z

@excitement-engineer pending and answer from Lee, we deal with mixed LocalDates, YearMonths, Times and DateTimes in our internal service-side representation and being able to express the difference between them in our schema would be useful in cross-timezone work.*

We are doing financial reporting and we have a lot of transactions with timezones that are colated. The result of that colation is then timezone-less, ie everything that is reported for 2016-11-27 is the same regardless of your location around the world and 2016-11-27 is always 2016-11-27 regardless of location.

We use time with TZ to manage daily jobs at the local time.

I'm more or less with Oleg (and think Oleg's mention of Java8 is apropos, especially the Date -> joda -> java.time journey) and my gut feeling is you want with TZ:

DateTime - a date + time with minute-or-greater resolution at UTC
Time - a time with minute-or-greater resolution at UTC

and without TZ:

LocalDate - a date with day-of-month resolution and no TZ
LocalTime - a time with minute-or-greater resolution and no TZ
Year
YearMonth

I'd be explicit about the LocalDate and LocalTime to mark that they are zone-less. This does mean that Time and LocalTime are effectively indistinguishable from a data perspective but you are documenting the expected behaviour w.r.t. localisation - this is particularly important for read-write values.

The reason to put Year and YearMonth is similar - you can represent these as, eg, start of year and start of month (so 2016 -> 2016-01-01) but this is more like having hh -> hh:00, which is not generally accepted (and not in the ISO) than hh:mm -> hh:mm:00.

I've deliberately left off LocalDateTime since they make everyone sad except when you figure out how to get rid of them. Someone might have a good justification for having this type but I don't.

This all begs the question of whether this should go into the spec and then the reference implementation. To do this well I think it needs to be quite heavy-weight. I like the idea of it being in the spec, but I'm not sure what the maintainers position on it's weight is. One suggestion is to fit a strong-typed version into your graphql-iso-date lib in the interim.

tl;dr - strong typing of date-time objects makes an API easier to understand. Open question on how much to stuff into the graphql spec.

* we also have LocalDateTime in data we're receiving but get a timezone into it as soon as it hits our system.

excitement-engineer · 2016-11-27T12:12:35Z

Thanks for your thoughts @jamesgorman2 ! I really appreciate it your input:)

The reason to put Year and YearMonth is similar - you can represent these as, eg, start of year and start of month (so 2016 -> 2016-01-01)

tl;dr - strong typing of date-time objects makes an API easier to understand

I agree with you:)

Java does a great job of distinguishing between these two (and other date types) ! Having very specific types (compared to a single type for representing all dates) makes an API much easier to understand and prevents confusion from implicit assumptions about date representations in my opinion.

An issue I ran into with the current implementation in this PR is that the DateTime scalar always serializes to the full ISO string YYYY-MM-DDTHH:mm:ss.sssZ which cannot be interpreted without context about what the datetime represent (is it a date. year, month,...??). As an example, the string 2016-01-01T00:00:00.000Z can be interpreted in different ways:

Year 2016
Year month 2016-01
Date 2016-01-01
DateTime

It seems that java initially also started out with a single class (java.util.Date) for representing date/time and them moved to the current setup of multiple classes in the time package!

An idea would be to update the date/time representation in GraphQL to the following:

GraphQLYear

Serializes into string YYYY from:

A javascript Date
A string in one of the allowed ISO formats.