Zaid-Ajaj / Fable.Remoting

Type-safe communication layer (RPC-style) for F# featuring Fable and .NET Apps
https://zaid-ajaj.github.io/Fable.Remoting/
MIT License
272 stars 54 forks source link

Serialization of DateTime loses Kind and TimeZone #273

Open MikaelUmaN opened 2 years ago

MikaelUmaN commented 2 years ago

Hi.

This should be related to: https://github.com/Zaid-Ajaj/Fable.SimpleJson/pull/21 and https://github.com/Zaid-Ajaj/Fable.Remoting/issues/108

Setup

Expected

Actual

Why this is a problem for me Basically I am on dotnet 5 and eagerly awaiting the "DateOnly" struct.

As it is now, I expect to be able to send string(myDateParameter) to an external http API on the server-side but instead of todays date (e.g. 2021-10-12) I get yesterdays.

Suggestions Probably it's not possible to keep the "Kind" type across serialisation, but then I think it should explicitly call "ToUniversalTime" prior to serialization and explicitly read it as Kind=UTC on the server side when deserializing.

The the application layer can handle the rest.

Zaid-Ajaj commented 2 years ago

Hi @MikaelUmaN,

In order not to lose the time zone information it is recommended to use DateTimeOffset which preserves the timezone offset of the server and propagates it to the client (that is represented by Date).

As for the Kind, I believe we had that implemented before but I need to double check

MikaelUmaN commented 2 years ago

Hi. Thanks for feedback.

I can confirm that DateTimeOffset works as expected, except it loses "kind" information. It is set to Unspecified.

I still think it would be nice to have DateTime working as well, though. Note that I am just trying to pass dates and not actually times; so it's doubly confusing that the server-side receives another date when transferring a DateTime.

The cleanest solution I think is to do it the way MessagePack does it; and just always rely on UTC when transferring; for example by going to ToUniversalTime and using Ticks.

From my side I will use DateTimeOffset as a workaround until DateOnly arrives in net 6.0 GA. Thanks for an excellent library.

kerams commented 2 years ago

DateTimeOffset does not have any explicit Kind. If the offset is 0, you know it's UTC. Otherwise it's local.

until DateOnly arrives in net 6.0 GA

Yes, I'm looking forward to it too. However, it needs to be supported by Fable as well, and only then can it be implemented in SimpleJson and MsgPack.

MikaelUmaN commented 2 years ago

Yes you are right of course, it does not have Kind. Sorry my mistake, it was because I converted it myself and looked at it.

The comment on DateTime still stands though. However, for my purposes you can close this issue in case it's not prioritized to fix.

Zaid-Ajaj commented 2 years ago

I think we can resolve this issue by documenting the current behavior and recommending people to use DateTimeOffset when the timezone is relevant