microsoft/fsharplu

Support single-case discriminated union

Opened this issue · 4 comments

drhumlen commented

It would be nice if single-case discriminated unions was serialized directly:

type PersonId = PersonId of int
type Person = 
  { Id: PersonId
    Name: string }
{"id": 123, "name": "Foo"}

instead of

{"id": {"PersonId": 123}, "name": "Foo"}

Edit: I realize there's some cases where this will do something very different from what's intended; e.g. if you have

type AuthType =
   | ApiToken of string

With the intention of adding more cases in the future, and then you'll break the serializer when you add the next case.

So I'm propising that we only do the single-case DU "optimization" when: There's one and only one case of course, and that case only has 1 field(?), and one or both of the following:

  1. The name of the case is the same as the type. So type UserId = UserId of string. If it's type UserId = PersonId of string, then that's a no-match. (?)
  2. When the argument is a "primitive" – string, number, long, etc. I can't immediatly think of a case where you want a single-case DU for something else, but I might be wrong about that (?).

I think I like 1 more, even though it's slightly more "magical", but I think it's most pragmatic way of doing it. What do you think?

drhumlen commented

Or is FSharpLu.Json abandoned? What should I use instead? Newtonsoft is 100% useless – it doesn't even encode Option correctly. I've actually been struggling really hard for the past 6 months to find a json library for F# that works and has users. Does F# even have other users when you can't even find a library for json? Scala , by comparison, has at least 5 that are all really good.

NinoFloris commented

Could be easily added, why not send a PR. (Note I'm not a maintainer and have fixed similar things in the lib)

  • 👍1
drhumlen commented

I've actually hacked together a fix locally based on the source code of this library. I might need to do some refactoring, but I could try to make a PR out of it when I get the time.

  • 👍1
blumu commented

@drhumlen Project is still maintained and is open to suggestion/PR.
As for this specific suggestion, I can see how this could be useful in certain cases, and perhaps less desired in other cases. I would recommend making this optional, and also adding unit tests to make sure that it's backward compatible. Looking forward to reviewing your PR!

  • 👍1