Skip to content

Why not to use Java default ISO_OFFSET_DATE_TIME in DateTimeScalar #121

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

Closed
sadv1r opened this issue Nov 16, 2023 · 3 comments
Closed

Why not to use Java default ISO_OFFSET_DATE_TIME in DateTimeScalar #121

sadv1r opened this issue Nov 16, 2023 · 3 comments

Comments

@sadv1r
Copy link

sadv1r commented Nov 16, 2023

I needed to fix ZoneId in my dates from server and found something that I don't see a reason for, can some one help?

In DateTimeScalar

Is there any reason why this code is used:

private static DateTimeFormatter getCustomDateTimeFormatter() {
        return new DateTimeFormatterBuilder()
                .parseCaseInsensitive()
                .append(ISO_LOCAL_DATE)
                .appendLiteral('T')
                .appendValue(HOUR_OF_DAY, 2)
                .appendLiteral(':')
                .appendValue(MINUTE_OF_HOUR, 2)
                .appendLiteral(':')
                .appendValue(SECOND_OF_MINUTE, 2)
                .appendFraction(NANO_OF_SECOND, 3, 3, true)
                .appendOffset("+HH:MM", "Z")
                .toFormatter();
}

and not something like this:

private static DateTimeFormatter getCustomDateTimeFormatter() {
        return DateTimeFormatter.ISO_OFFSET_DATE_TIME;
}
@dondonz
Copy link
Member

dondonz commented Nov 20, 2023

Good question!

There is a story behind this. DateTime has been implemented differently across different languages, because the original RFC 3339 wasn't specific enough. At work we had a problem where the number of digits used to express nanoseconds was different in two implementations. It was a subtle difference, but it can have an impact.

After this happened we created a way to standardise GraphQL Scalars across implementations. Here's a blog explaining more background behind the GraphQL Scalars initiative https://graphql.org/blog/2023-01-14-graphql-scalars/

And you can see the DateTime spec here: https://scalars.graphql.org/andimarek/date-time.html. The crucial part of this spec is requiring always three digits for nanoseconds.

So to answer your question, we have a verbose way to express DateTime so we are certain to conform to this DateTime specification.

@dondonz
Copy link
Member

dondonz commented Nov 20, 2023

In the code, we have linked to the DateTime specification page via the @specifiedBy built-in directive

@sadv1r
Copy link
Author

sadv1r commented Nov 21, 2023

Oh @dondonz thank you so much for such detailed explanation, now it's really makes sense for me!😊

@sadv1r sadv1r closed this as completed Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants