-
Notifications
You must be signed in to change notification settings - Fork 9
Add LocalDateTime scalar #32
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # LocalDateTime — GraphQL Custom Scalar | ||
|
|
||
| "Author - ChilliCream" | ||
|
|
||
| "Date - 2025-11-27" | ||
|
|
||
| This is a String-based scalar. | ||
|
|
||
| **License and Copyright** | ||
|
|
||
| Copyright © GraphQL contributors. This specification is licensed under | ||
| [OWFa 1.0](https://www.openwebfoundation.org/the-agreements/the-owf-1-0-agreements-granted-claims/owfa-1-0). | ||
|
|
||
| # Overview | ||
|
|
||
| This scalar represents a date and time without a time-zone in the | ||
| [ISO-8601](https://en.wikipedia.org/wiki/ISO_8601) calendar system. | ||
|
|
||
| The pattern is "YYYY-MM-DDThh:mm:ss" with "YYYY" representing the year, "MM" the | ||
| month, "DD" the day, "hh" the hour, "mm" the minute, and "ss" the second. The | ||
| "T" may be lower case. | ||
|
|
||
| Valid examples are "1983-10-20T23:59:59" or "2023-04-01T23:59:59". An invalid | ||
| example would be "2011-13-10T23:59:59" because there isn't a 13th month in a | ||
| year. | ||
|
|
||
| The prefix "Local" comes from the fact that without a time-zone it is not a | ||
| specific point in time, but rather expresses a "local point of view". | ||
|
Comment on lines
+27
to
+28
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general, it'd be nice to refer to RFC 3339 section 5.6, like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had aligned this spec with the RFC 3339 mentions in its introduction:
So it may not make sense to reference it for a local date/time?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, I think we're bumping into the sad reality that is the mess of internet RFCs. I don't have a strong opinion whether we should use RFC 3339 (with this weird wording) or ISO 8601 (not publicly available). If anything, I would aim for consistency. |
||
|
|
||
| Because this scalar depends on the ISO-8601 calendar it is not recommended to | ||
| use for dates before the year 1582. | ||
|
Comment on lines
+30
to
+31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL. This is not mentioned in https://scalars.graphql.org/andimarek/date-time.html so I would either remove it here or add it there for consistency.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned, this spec was based on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would remove it there then ^^. Or put some more details, for an example from the JavaDoc: https://docs.oracle.com/javase/8/docs/api/java/time/LocalDate.html (Assuming we're OK with ISO 8601) My logaf is low though so your call. |
||
|
|
||
| # Name | ||
|
|
||
| The recommended name is "LocalDateTime". | ||
|
|
||
| # Result | ||
|
|
||
| Every result must follow the pattern described above, with the further | ||
| requirement that the divider character `T` is always uppercase, never `t`. | ||
|
|
||
| # Input | ||
|
|
||
| Every input must follow the pattern described above. | ||
|
|
||
| # References | ||
|
|
||
| [ISO-8601](https://en.wikipedia.org/wiki/ISO_8601) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would include a potential factional second here. RFC3339 supports this and Java too (with nanosecond precision).
The existing
DateTimesupports only 3 digits. Ping @andimarek, is there any context behind this decision?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I decided to err on the side of clarification and simplification vs ambiguity. Always providing 3 milli seconds seemed the obvious and clear choice.
For example with a fixed length parsing and validation of it is much easier than not.
Also: what does micro 1500 ms for example mean? This should be more one more second and just 500 ms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: RFC3339 is just underspecified in some details. For example
secfracis never explained in detail I believe. Is itmsornanoor something else? This is why we clarified it more.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand RFC3339 as specifying an arbitrary fractional second. The unit is always second, not
msornano.1.5is1.5second or1500milliseconds or1500000000nanoseconds.This could be represented as
1.500or1.5, I think we should allow all of them.In 2025 I think it's OK to allow variable length parsing. Java will happily parse
1.5or1.500and the results will be the same. I expect most language to do the same.The main question IMO is whether we limit the precision because some languages do not have an easy way to represent high precision dates. JavaScript for an example, has only millisecond precision. If we care about the future, nanosecond would make sense IMO.
As someone doing Kotlin mostly, I would love more precision:
But I also realize that a large portion of the ecosystem is on JavaScript so I'm fine with millisecond precision:
And a note explaining why this choice was made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with allowing different fraction sizes is that you need to switch between ms and tenths of seconds for example.
So you need to deal with different units which is not great and error prone imho. Requiring always 3 means it is always milliseconds and you have a fixed unit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I interpret the spec the same way that Martin does.
.1is not deciseconds, it's ".1 of a second". It's fractions of a second, and the unit doesn't change depending on the number of digits that are included.For example,
.100000000could be formatted as either 100 ms or 100,000,000 ns.For Hot Chocolate, we actually diverged from the
DateTimespec and went with fractional seconds of 0-7 digits, as opposed to exactly 3.This issue has been brought up before: andimarek/graphql-scalars.com#2.
In my opinion,
DateTimecould be simplified by removing the parts that differ from RFC 3339 – I don't see strong enough reasons for the differences.To the original question, we should probably finalize the discussion regarding
DateTimefirst, but I'm not opposed to adding fractional seconds. I did notice this in RFC 3339 though, but maybe it's okay since it's optional anyway.Finally, I fundamentally disagree with the structure of this repository, but I've opened an issue for that (#39).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds oddly specific. Why not 0-6 (microseconds) or 0-9 (nanoseconds)?
I think there is some value in limiting precision to make it easier on the implementation side. Just like
IntisInt32and notBigDecimalby defaut because in most languages, it's a lot easier to deal withInt32than it is to deal with arbitrary precision integers.I would probably limit precision to 0-3 digits (JavaScript) or 0-9 digits (Java), not 100% sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya, it's for C#, which has 100 nanosecond precision. Perhaps 9 (or arbitrary) would have been a better choice.
From Claude:
Then I asked:
With the response:
I followed up with:
To which it responded:
That last paragraph really resonates with me, especially given the fact that Temporal will support nanosecond precision.
I'm currently leaning toward 0-9 digits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or alternatively, not specifying the precision, to align with RFC 3339.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, JSON Schema simply references RFC 3339:
https://json-schema.org/understanding-json-schema/reference/type#built-in-formats