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

DateTimeOffset is transfered as UTC DateTime (losing information about offset) #108

Closed
Dzoukr opened this issue Jan 18, 2019 · 8 comments
Closed

Comments

@Dzoukr
Copy link
Contributor

Dzoukr commented Jan 18, 2019

Hi Zaid,

I found interesting thing. When sending DateTimeOffset to server from Fable app, what is actually sent to server is DateTime in UTC format, which leads to loosing information about client offset. I checked the Converter (https://github.com/Zaid-Ajaj/Fable.Remoting/blob/master/Fable.Remoting.Json/FableConverter.fs#L27-L39) and there is no DateTimeOffset case.

Is it bug or is it on purpose?

@Dzoukr
Copy link
Contributor Author

Dzoukr commented Jan 18, 2019

Btw, this seems not to hard to fix, so I'll try to send PR (at least I'll get more into your great library :) )

@Zaid-Ajaj
Copy link
Owner

Hello there Roman,

This might be a bug I am not sure, if I don't have a special case specified, it means I am just letting Newtonsoft.Json handle it the default way. Can you please tell me more about how the DateTimeOffset gets serialized to JSON and what you expected it to be? turning on the Diagnostics logger is the easiest way but you can also just look at your network tab.

JSON stuff happen at two places, on the server using FableJsonConverter and on the client using SimpleJson so you have to consider how things are deserialized as well.

In the case of DateTimeOffset I am just reading it as string and calling DateTimeOffset.Parse on that string, which in turn calls parse from fable-core.

Because we have full control over the process you can change the representation of what gets sent from the server and match the result on the client, for example you can send { ticks: <int64>, offset: <int64> }as JSON and convert to correct DateTimeOffset from the client by matching on a JObject with these properties instead just JString (supporting both cases of course for backwards compatibility)

Let me know if you have any questions!

@Dzoukr
Copy link
Contributor Author

Dzoukr commented Jan 18, 2019

Ok, need to change my words - it won't be so easy to fix (however the libs seems to be written really in a clean way and great for external contribution! 👏 )

I did some investigation and sending DateTimeOffset from server to client works ok. The problem seems to be when converting javascript Date to string interpretation (the serialization). If I got it right, the function which does that is in Proxy - https://github.com/Zaid-Ajaj/Fable.Remoting/blob/master/Fable.Remoting.Client/Proxy.fs#L111 which calls toJson in Fable.Core.JsInterop

To be honest, now I am quite confused where to dig next. Thanks for any hint.

@Zaid-Ajaj
Copy link
Owner

The problem seems to be when converting javascript Date to string interpretation (the serialization). If I got it right, the function which does that is in Proxy

Almost! you should be looking into the Fable.Remoting.ClientV2 directory (for Fable 2) instead of Fable.Remoting.Client (for Fable 1)

In Fable 2, I am using Json.stringify from the SimpleJson which uses the default serialization function JSON.stringify from the browser along with little modifications.

If serialization of DateTimeOffset is incorrect from the client then the only way we could change it is by having toJSON function attached in the prototype of IDateTimeOffset internally in Fable itself that converts the value into a JSON representation without information loss (i.e. { ticks: <int64>, offset: <int64> }) only then we can adjust the JSON deserialization on the server to account for this representation as well.

This is similar to the BigInt issue from latest Fable

Can you please open an issue in Fable with details (expected vs, actual)?

Maybe there is a way to use reflection to control how everything is serialized instead of relying of JSON.stringify, I will investigate soon-ish ;)

@Dzoukr
Copy link
Contributor Author

Dzoukr commented Jan 18, 2019

Oh, my fault! So it is in Client2. Sorry for misunderstanding and thanks for all the patience. Yes, the JSON.stringify seems to be a big fun - I found another related question on SO (https://stackoverflow.com/questions/31096130/how-to-json-stringify-a-javascript-date-and-preserve-timezone)

I'll create an issue in Fable repo.

Thanks Zaid!

@Zaid-Ajaj
Copy link
Owner

Roman I think I was able to hack my way around DateTimeOffset forcing it to call ToString("O") instead of just ToString() during serialization which preserves the timezone information see SimpleJson/#21 for details. I published a new version for the client (v4.5.5) can you please update and check if everything works as expected? 😄

If the problem persists, please re-open the issue

@Dzoukr
Copy link
Contributor Author

Dzoukr commented Jan 19, 2019

You are magician, Zaid! It works! 👍 Thanks a lot again! 🙏

@Zaid-Ajaj
Copy link
Owner

Awesome! glad it worked 😄

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