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

Serializing int64 inside discriminated unions generates irreversable JSON representation #1411

Closed
Zaid-Ajaj opened this issue May 29, 2018 · 6 comments

Comments

@Zaid-Ajaj
Copy link
Member

Description

Serializing int64 inside discriminated unions generates incompatible JSON, in other words, the generated JSON cannot be deserialized back to original type.

After tracking down the issue at Fable.Remoting, I narrowed it down to the following code that doesn't work:

open Fable.Core
open Fable.Core.JsInterop

type SingleCase  = SingleCase of int64

let input = SingleCase 20L
printfn "%A" (ofJson<SingleCase> (toJson input))

You will get the error:

Uncaught Error: Cannot deserialize into Test.SingleCase - Error (inflate): Input string was not in a correct format. - Data: '{"SingleCase":{"low":20,"high":0,"unsigned":false}}'

Note: the same issue happens even with unions of multiple cases.

The Fable JsonConverter on the server is also unable to deserialize the date {"low":20,"high":0,"unsigned":false} into a proper int64 and throws this exception:

System.Exception: Expecting int64 but got StartObject.
/*
rest of irrelevant stack strace
*/ 

where the StartObject indicates that the deserializer encountered an object literal where it was expecting int64

Related information

  • Fable version (dotnet fable --version): 1.3.17 && repl
  • Operating system: Windows 10 && linux on Travis CI
@alfonsogarciacaro
Copy link
Member

Hi @Zaid-Ajaj! Thanks a lot for the report and sorry for replying late. At the end we're deprecating JSON serialization in Fable 2, and instead we're adding auto-decoders to Thoth so we'll have to solve this issue there (if it's still happening).

BTW, the good news is Reflection is already implemented in Fable 2, so Fable.Remoting will likely work with the alpha release 😄

@Zaid-Ajaj
Copy link
Member Author

Thanks for the reply, maybe I missed something so I have a couple of questions

At the end we're deprecating JSON serialization in Fable 2

Should I assume that ofJson<'t> and toJson will be completely removed from Fable.Core?

We're adding auto-decoders to Thoth

Were these the code-generated specialized functions to serialize/deserialize objects?

Reflection is already implemented in Fable 2

Awesome, I will soon be running the tests against Fable 2 to see how things are going, though if toJson and ofJson<'t> are/will be removed, I need to think of some else to make Fable.Remoting "just work"

Lastly, if is it not a big issue, can it still be fixed to the Fable 1.3.x branch?

@MangelMaxime
Copy link
Member

@Zaid-Ajaj Indeed ofJson and toJson will be removed from Fable.Core.

But we will provide equivalent via Thoth.Json, the idea is by using Thoth.Json we will check the data of JSON and output nice error message if an error occur.

For example, if you have a property : Firstname : string then, we will check that the value in the JSON is indeed set and that it's a string. With current ofJson if the value of Firstname is null in the JSON you don't know it until you use it and get an undefined error in your code.

@Zaid-Ajaj
Copy link
Member Author

@MangelMaxime I took a closer look, very interesting way for deserialization, I wonder if it can handle the exhaustive test suite of Fable.Remoting, I will try it soon and otherwise I will be swarming you with issues 😂 😜

@MangelMaxime
Copy link
Member

Thank you :)

I think it will not pass the test suite yet. I need to had more primitive type like Dates for example :)

@Zaid-Ajaj
Copy link
Member Author

@alfonsogarciacaro This issue looks exactly like #1374 where the value (of type int64) inside the union is not toString()-ed when serialized and instead only the internal representation (the object literal with high/low/unsigned properties) is stringified

It has something to do with this block of code and the deflateValue can extended to solve the same issue for int64:

import { isLong } from './Long'

function deflateValue(v: any) {
  if (isDate(v)) {
    return deflateDate(v);
  }

  if (isLong(v)) {
      return v.toJSON();
  }

  return v;
}

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

3 participants