thoth-org / Thoth.Json

Library for working with JSON in a type safe manner, this libs is targeting Fable
https://thoth-org.github.io/Thoth.Json/
MIT License
151 stars 34 forks source link

Allow user to customize even the default encoders #57

Open kqr opened 4 years ago

kqr commented 4 years ago

When the default JSON encoder in ASP.NET 3.1 encodes option fields, it does it quite literally. I get:

{
    "case": "Some",
    "fields": [ "abc123" ]
}

Since Thoth expects to deal with erased options (which indeed is the sensible default) it fails to parse this as the option<string> it was meant to represent.

Of course, one could use Thoth also on the ASP.NET side to encode JSON and thus avoid this conflict, but I also have a hard time imagining it being a bad thing to just parse also this type of encoding alongside the erased one, increasing the compatibility between Thoth and ASP.NET 3.1.

Thoughts?

MangelMaxime commented 4 years ago

Well, one problem we can have is that we start supporting others serialization representation for one type I might have to support it for all.

And if I do it for on serializer here the one from ASP.NET 3.1, people might also ask to support their own serializer.

So for now, I think you should use manual decoders because they can work for any type as you are the one controlling the logic :)

However, it can be interesting in Thoth.Json v5 to add a way to support several representations for one types.

It will be possible out of the box thanks to this feature:

Allow the user to override the standard decoder. For example, perhaps they want to represent the DateTime in a specific manner

But perhaps, we could make it even easier to do.

MangelMaxime commented 4 years ago

Hello @kqr,

could you please tell me how they serialize a None case?

Note that only the decoder will be adapted, the encoder will still generate Erased version.

Partial implementation to make option support both erased style and ASP.NET style:

    let option (decoder : Decoder<'value>) : Decoder<'value option> =
        oneOf [
            fun value ->
                if Helpers.isNullValue value then Ok None
                else decoder value |> Result.map Some

            field "case" string
            |> andThen (
                function
                | "Some" ->
                    field "fields" (index 0 decoder) |> map Some
                | caseValue ->
                    fail ("`" + caseValue + "` is not a valid value for option<'T>")
            )
        ]
kqr commented 4 years ago

Hm. I worked around that problem a while ago, and I no longer remember how to reproduce it. When I try to use System.Text.Json on option values in F# interactive, this happens:

> open System.Text.Json;;
> open System.Text.Json.Serialization;;
> JsonSerializer.Serialize (Some "abc123");;
val it : string = "{"Value":"abc123"}"

> JsonSerializer.Serialize (None);;
val it : string = "null"

and that's not what happened when I used it in an ASP.NET 3.1 application before...

MangelMaxime commented 4 years ago

Sorry for the delay, didn't have much time at the beginning of the year.

Hum, perhaps you were using Newtonsoft at the time?

I guess instead of trying to support all the possible JSON representation I will focus on allowing people to override all the default decoders in the Auto modules. Like that they will not be blocked as they can add new supported representation for a given type.

kqr commented 4 years ago

That sounds like a great idea!