feat: Add support for the time crate #1006
Conversation
tyranron
added
enhancement
There was a problem hiding this comment.
@scott-wilson thank you for bringing this up and putting effort into implementing this.
However, now it's quite a mess with our scalar types regarding date and time. While chrono provides DateTimeFixedOffset, DateTimeUtc and bunch of Naive* stuff, this time integration diverges in naming, and now we have PrimitiveDateTime and the others. This definitely begs for some standartization, and, fortunately, we have GraphQL Scalars for that.
I've adjusted your implementation to map time types to the appropriate scalars from GraphQL Scalars collection:
| Rust type | Format | GraphQL scalar |
|---|---|---|
Date |
yyyy-MM-dd |
Date |
Time |
HH:mm[:ss[.SSS]] |
LocalTime |
PrimitiveDateTime |
yyyy-MM-dd HH:mm:ss |
LocalDateTime |
OffsetDateTime |
RFC 3339 string | DateTime |
UtcOffset |
±hh:mm |
UtcOffset |
I'd like also to integrade Duration from time, but it lacks parsing required by Duration scalar.
@ilslv we need to do the similar adjusting for chrono types in a separate PR.
juniper/src/integrations/time.rs
Outdated
| | `Date` | YYYY-MM-DD | | | ||
| | `PrimitiveDateTime` | YYYY-MM-DD HH-MM-SS | | | ||
| | `Time` | H:M:S | Optional. Use the `scalar-naivetime` | | ||
| | | | feature. | |
There was a problem hiding this comment.
That's prety standard one. I guess we can go without additional feature here.
juniper/src/integrations/time.rs
Outdated
| { | ||
| fn resolve(&self) -> Value { | ||
| let description = | ||
| parse("[year]-[month]-[day]").expect("Failed to parse format description"); |
There was a problem hiding this comment.
Doing format parsing every time is not quite performance wise. We better use Lazy here.
There was a problem hiding this comment.
Oh, we do actually have const one.
|
I was just about to take a look at the two notes, and it looks like you already tackled that, thanks! Anything you need me to polish up? |
|
@scott-wilson it's approved, I'm about to merge this PR. Thanks again! 🍻 |
This adds support for the
timecrate due to concerns with safety in thechronocrate (see rustsec/advisory-db#1082).So, I added support for time, since it looks like that might become the "preferred" date/time crate in the future.