Tarmil / FSharp.SystemTextJson

System.Text.Json extensions for F# types
MIT License
323 stars 44 forks source link

WithUnwrapOption(false) still serializes None as null #171

Open cmeeren opened 10 months ago

cmeeren commented 10 months ago

When using WithUnwrapOption(false), None still serializes to null.

(I am serializing for logging/assertion/debugging purposes and want None/ValueNone to serialize to the strings "None"/"ValueNone", as I expect is the intended functionality with WithUnwrapOption(false).)

open System.Text.Json
open System.Text.Json.Serialization
open Xunit

let options = JsonSerializerOptions()
options.Converters.Add(
    JsonFSharpOptions
        .Default()
        .WithUnwrapOption(false)
    |> JsonFSharpConverter
)
Assert.Equal("\"None\"", JsonSerializer.Serialize(None))

This fails with:

Xunit.Sdk.EqualException
Assert.Equal() Failure
          ↓ (pos 0)
Expected: "None"
Actual:   null
          ↑ (pos 0)
cmeeren commented 10 months ago

Could this be related to Option<_> using [<CompilationRepresentation(CompilationRepresentationFlags.UseNullAsTrueValue)>]?

Tarmil commented 10 months ago

Yes, it is on purpose that None and ValueNone are serialized as null. I am open to adding an option to customize this though!

cmeeren commented 10 months ago

Would be great! I am not serializing for idiomatic JSON, but for displaying structures of F# data (in a better way than %A), so displaying None instead of null would help my users.

cmeeren commented 10 months ago

Note by the way that the docs for WithUnwrapOption are misleading:

https://github.com/Tarmil/FSharp.SystemTextJson/blob/486924448ff384a6121ec6141df61bd619f9e520/src/FSharp.SystemTextJson/Options.fs#L368-L372

It says that if set, None is serialized as null. While this technically doesn't say that if it's not set, then None is not serialized as null, it does give a strong impression that that's the intended meaning.

Also relevant here:

https://github.com/Tarmil/FSharp.SystemTextJson/blob/486924448ff384a6121ec6141df61bd619f9e520/src/FSharp.SystemTextJson/Options.fs#L56-L59

cmeeren commented 9 months ago

Hi! Just wondering if it would be possible to implement this soon-ish (say, during the coming week or two)? I need to decide whether to work around this, but the point is moot if this could be supported upstream. 🙂

Yes, it is on purpose that None and ValueNone are serialized as null.

By the way, ValueNone is serialized as ValueNone, not null (regardless of WithUnwrapOption).