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

Support for Temporal JS serialization format #76

Open
sam-mfb opened this issue Sep 11, 2021 · 8 comments
Open

Support for Temporal JS serialization format #76

sam-mfb opened this issue Sep 11, 2021 · 8 comments
Assignees

Comments

@sam-mfb
Copy link

sam-mfb commented Sep 11, 2021

It looks like the ECMA/JavaScript world is about to accept the Temporal JS proposal, which has a slightly different serialization format than that used by nodatime. https://tc39.es/proposal-temporal/docs/iso-string-ext.html

In particular, for ZonedDateTime, the proposed Temporal object is going to serialization it to a format like 2007-12-03T10:15:30+01:00[Europe/Paris]

It doesn’t look like it will be that difficult to write a custom converter to deserialize this to nodatime’s ZonedDateTime, but it would be nice if that was supported as an option out of the box given that I imagine the JS world (or at least the part of the JS world that is intentional about capturing time zone information) will start using this format.

Is there any plan to support that? (Or reason not to?)

@jskeet
Copy link
Member

jskeet commented Sep 11, 2021

Yes, I'd be fine with that being one of the out-of-the-box patterns, using the default TZDB provider.

I think we'll need to put a bit more thought into how to make that easy to use from the JSON serialization frameworks though. I don't have any time to do this right now (heck, I've wanted to do a 3.1 release for a while, and haven't got round to that) but it's definitely a worthy feature request.

@jskeet jskeet self-assigned this Sep 11, 2021
@sam-mfb
Copy link
Author

sam-mfb commented Sep 11, 2021

Thanks for considering this.

To throw in my two cents on JSON issue: What if the current ZonedDateTime converter just accepted both formats without any additional configuration? In other words, it would parse and deserialize either the current format with z at the end or the Temporal format with [z] at the end, in both cases with or without nanoseconds and with any timezone provider. From the consumer's perspective, that would mean no additional configuration--they could just start sending Temporal serialized values for ZonedDateTime.

@jskeet
Copy link
Member

jskeet commented Sep 11, 2021

What if the current ZonedDateTime converter just accepted both formats without any additional configuration?

I'm not sure what you mean by "the current ZonedDateTime converter". In order to parse a value, you need to create a ZonedDateTimePattern, and that specifies the format. I don't think it's a good idea to start changing the behavior of any of the current standard patterns, especially as we definitely wouldn't want to change how that pattern formats a value - and it would be weird to be able to parse but not format values in the Temporal format.

@sam-mfb
Copy link
Author

sam-mfb commented Sep 11, 2021

Oh, right. Sorry. It's early here. I forgot you have to serialize as well as deserialize.

Perhaps overloading the ConfigureForNodaTime extension with a third parameter to accept an enum that allowed serializations other than the default?

BTW, if anyone has this need now, the easiest way to do that I could find is manually configuring serializers along the lines of your SO answer here: https://stackoverflow.com/questions/47311121/getting-nodatime-serialization-jsonnet-to-work-with-a-custom-date-format

@jskeet
Copy link
Member

jskeet commented Sep 11, 2021

Rather than adding more overloads, I think it's probably time for a WithNodaTimePattern method like this:

public static JsonSerializer WithNodaTimePattern<T>(this JsonSerializer serializer, IPattern<T> pattern)

(ditto for settings etc)

That would replace the pattern for T. So you'd write:

settings => settings.ConfigureForNodaTime().WithNodaTimePattern(ZonedDateTimePattern.TemporalJs);

@sam-mfb
Copy link
Author

sam-mfb commented Sep 11, 2021

i see. that doesn't look like a terrible amount of work. if you think that's the way you want to go, i'd be happy to take a crack at a PR. it would be helpful in one of our apps so it would almost count as work...

@jskeet
Copy link
Member

jskeet commented Sep 11, 2021

i see. that doesn't look like a terrible amount of work. if you think that's the way you want to go, i'd be happy to take a crack at a PR. it would be helpful in one of our apps so it would almost count as work...

Sure, that would be helpful. You'll still need to wait for an actual release of course, but I can try harder to make some time for that...

@sam-mfb
Copy link
Author

sam-mfb commented Sep 11, 2021

right. and i guess to be precise, it's probably more about the acceptance of the proposed extension to RFC3339, rather than Temporal. https://www.ietf.org/archive/id/draft-ryzokuken-datetime-extended-02.html#name-syntax-extensions-to

it looks like Temporal is following (and gated by) that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants