Tarmil / FSharp.SystemTextJson

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

When DU cases are external, their naming should conform to the policy specified in the main options. #75

Open gdar91 opened 3 years ago

gdar91 commented 3 years ago

First, this is a great library! For me, it was the thing, that finally gave me the opportunity to leave newtonsoft.json and switch to system.text.json. And then, the title basically says it all: when DU cases are external, their naming should conform to the policy specified in the main options. As they are represented as property names, all property names should conform to the policy that comes from the main options (not from the converter one). Also, when there are no fields, the case name is just a string, so it should not be left unmodified, because it's not a property, it's just a string.

For example, say you have a discriminated union like this:

type State =
| Pending
| Available of int

let state1 = Pending
let state2 = Available 10

and you use a camel-case naming strategy in the main options, also external tags, and unwrap fieldless tags in the converter options. Then the correct output for state1 and state2 should be: "Pending" and { "available": 10 } respectively.

Tarmil commented 3 years ago

That's an interesting conundrum, because really they're both union tags and properties. I definitely see your point.

At the same time, having the same type print as "Pending" and {"available":10} feels somewhat inconsistent. But I suppose that if you want consistency for this, you'll pass the same naming policy for fields and tags. I would expect people who care about the exact format to use camelCase for both in most cases anyway.

So, no perfect solution, but I think your proposal is an improvement.