Inconsistent behavior of nulls?

[GBirkel]

Now that WithSkippableOptionFields is implemented (thank you!) I can generate what looks like confusing results:

The structure I'm deserializing:
      { "stringOption": null,
        "seqOfStringOption1": [null],
        "seqOfStringOption2": []
       }

Deserializing with WithSkippableOptionFields:

{ stringOption = Some null
  seqOfStringOption1 = seq [None]
  seqOfStringOption2 = seq [] }

Deserializing without WithSkippableOptionFields:

{ stringOption = None
  seqOfStringOption1 = seq [None]
  seqOfStringOption2 = seq [] }

What looks strange to me is the first stringOption. Passing null renders "Some null". How do I get "None" instead? Is my only choice to omit the field from the input? If so, why the inconsistency between that, and a null that appears inside a sequence?

The code I used:

#r "nuget: FSharp.SystemTextJson"

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

type TestInput = {
    stringOption: string option
    seqOfStringOption1: string option seq
    seqOfStringOption2: string option seq
}

let skipYes = JsonFSharpOptions.Default()
                    .WithSkippableOptionFields()
                    .ToJsonSerializerOptions()

let skipNo = JsonFSharpOptions.Default()
                    .ToJsonSerializerOptions()

let data = """
                { "stringOption": null,
                  "seqOfStringOption1": [null],
                  "seqOfStringOption2": []
                 }"""
printfn "\n%s\n" data

printfn "\nDeserializing with WithSkippableOptionFields:\n"
let deserializedA = JsonSerializer.Deserialize<TestInput>(data, skipYes)
printfn "%A" deserializedA

printfn "\nDeserializing without WithSkippableOptionFields:\n"
let deserializedB = JsonSerializer.Deserialize<TestInput>(data, skipNo)
printfn "%A" deserializedB

[SamuelBerger]

I can confirm the bug. I tried to fix it #153

[Tarmil]

The reason for the current behavior is to preserve the ability to round-trip: if you serialize { x = Some null }, then deserialize the result, you should get the same value back. With your proposed change, you would get { x = None }, and there would in fact be no JSON that deserializes to { x = Some null } (or { x = Some None }).

But I see your point and I am open to allow this as an option. I'd rather make it an explicit one though. I think this deserves a new fluent option. Tentative name: WithNullAsSkip?

(We could also make it an optional value passed to WithSkippableOptionFields(), but I think a separate fluent option is better because this would not only affect skipped options but also the Skippable<'t> type.)

[SamuelBerger]

Ok, I see.

First, this bug is about type = { Name: string option } when deserialized from { "Name": null } becomes { Name: Some null } instead of { Name: None }.
This is obviously a bug.

In our case it hit customers using our public API - so my reasoning about the second point, nested option types, is also seen through that lens: using the serializer at application boundary to be called from third party:

In my experience, modeling application code by nesting options is rare, but of course, valid (like having two reasons for presence/absence of something, like table having column yes/no and the value itself can be absent). But even then, beeing more explicit by using different types, probably makes things easier - like you did with the Skippable<'t> type.

On the other hand when serializing your model to JSON, nested option types cannot be represented with nulls. { Name = Some (Some (Some None)) } will just become null and the information on what level it was is lost. And that's fine. When going to JSON with option types as null, flattening nested options is just the nature of things.
If you wanted to preserve the nested options, you would need something else than the 'null shortcut'.

In my case, fixing the bug in either way would be fine, as nested option types in our public API would never occur.

But still my opinion is: serializing options as nulls implies 'loss of information' and that is intended and fine :)

Is the round-trip ability based on a real use case scenario? Would be a happy to learn about it.

[IanWitham]

Any progress on this? I'm consuming an API where I don't care about round-trip serialization at all.
Perhaps the name of the configuration option could provide a hint that it breaks round-trip serialization. Something like "WithNullAsNoneUNSAFE". Obviously, that's terrible, but you get the idea.

[Tarmil]

This is released in v1.3.13:

JsonFSharpOptions.Default()
    .WithSkippableOptionFields(SkippableOptionFields.Always, deserializeNullAsNone = true)