-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
System.Text.Json : Consider supporting F# discriminated unions #55744
Comments
Tagging subscribers to this area: @eiriktsarpalis, @layomia Issue DetailsConcerning support for discriminated unions, it might be worth pointing out that there is no one canonical way to encode DUs in JSON. DUs are used in many different ways in F# code, including the following:
In light of the above, I'm increasingly starting to think that System.Text.Json should not be providing a default mechanism for serializing DUs. Users would still be able to use available custom converters that provide the union encoding that suits their use case or perhaps use unions in their data contracts in a way that bypasses the serialization layer altogether. Originally posted by @eiriktsarpalis in #29812 (comment) If we do decide to support F# DUs in the future, it would likely be in the form of a publicly available converter factory à la JsonStringEnumConverter that configures one or more of the above alternative serialization formats.
|
Tagging @bartelink @Zaid-Ajaj @NinoFloris who might have opinions on the matter. |
IMO just because there are multiple ways to represent DUs in JSON doesn't imply that a library shouldn't pick a default. I think many F# devs would appreciate a choice that works out of the box with the possibility of customization where required, not having to customize or having to understand library internals to get started with it. This is the approach I implemented in type Shape =
| Point
| Circle of radius:float
| Rectangle of width:float * length:float The following JSON is generated
of course there could be a setting to choose to write out the property names when provided
|
I agree [with the OP] that there cannot and should not be a default canonical implementation; As I tend to take view that individual converters that do easy to describe things are the way to go. Thus I would ideally like to see the following in the box:
|
While
In order to guard against things that IME happen in the wild, I'd propose the following rules:
Again, IME, maintaining symmetry is valuable for nullary cases too. The main reason for this being that any consumer will be able to adapt to me adding a field to the case payload if it's rendered as I'd also mention another reason a canonical default handing is likely to run aground even if it was to be defined:
|
I agree with @bartelink that the compatibility of json encoded data can be a minefield, that requires attention when writing migrations, and when making API changes. I still think that having a default canonical implementation provides a good user experience. I think it is too early to force the programmer understand and choose between serialisation strategies just because they wish to turn information into JSON. Rather, when somebody has durable data, or needs to concern themselves with wire compatibility, then the way things are serialised may get important. In my case, when I upgrade durable data migrations, I transform the data, using the default canonical implementation, but I map from one type to another type, and write again using the default canonical implementation. If there is a large number of types involved, then the burden of having to specify a per type serialisation strategy, as a lot of extra work. Surely, yes, it will be more optimised, and possible upgrade proof, but since I'm writing migrations and making new endpoints, that point is moot, for me. Thus I'm forced to do more work, without any apparent benefit. By having a default canonical implementation, and allowing overrides, it is possible to use json with a minimum amount of effort, and allow freedom when it has benefit. By the way, what would be the developer experience when using Type Providers, such as https://fsprojects.github.io/SQLProvider/ when there is no default canonical implementation? From my perspective, having a default canonical implementation, allows usage without con |
@jhf @Zaid-Ajaj While I protested a lot, being able to have Unions convert without having to litter the code with Attributes is definitely something that's hard to give up once you've experienced it ;) I implemented opt-in selection of such a policy jet/FsCodec#69 - would appreciate any thoughts you might have and/or whether the fact that one can define an explicit encoding policy in ~200 LOC without the risk of forcing a necessarily opinionated encoding without people being aware |
What is the best (performance-wise) way currently (Jan 2022) to serialize/deserialize F# DUs which are "Type-safe enums" as per the initial post (not Single case DUs, no fields attached to the cases)? We use currently .NET Enums in our Api Dto layer because we had severe performance issues with DU serialization/deserialization performance in the past ... however we are missing the exhaustive match (at compilation) of the DUs ... |
@deyanp I've never benchmarked, but putting So I'd say its worth a benchmark - I'd be surprised if its in the same realm of perf as enums but can't imagine it being a disaster. (would accept the benchmark as a PR if you see fit) |
@bartelink , hmm, I think I am doing something wrong, because the benchmark Enum <> DU brings very similar results (even though the code I saw in FsCodec.SystemTextJson is full of reflection and stuff): Serialization:
Deserialization:
Code (as F# Console app, as FSX give error [^1]
[^1] Error
|
I can't see anything blatantly wrong; note the One thing though: doing One other thing to look at might be The other thing is that for the DU cases, you could use Options without the |
@bartelink, I did run with the stuff with I will re-run with One question - I assume there is no other (centralized) way but having this attribute on all DUs, right?
Even the type must be specified (it seems that is in contrast to NewtonSoft.Json) ...
but I guess that is not possible? |
Re Debug stuff, I see
There's a factory in FsCodec that enables this via the Note that this also switches on the automatic application of |
@bartelink , there was actually an error in my benchmarks, I was comparing Enum Serilization to Enum Serialization ;) "Correct" results (until proven otherwise): Serialization - I see 10-50% overhead of DUs compared to Enums ... Deserialization - I see 5-10% overhead of DUs compared to Enums ... This For |
Ah; cool you found the discrepancy ;) I'm sure the perf can be improved, but I guess its not bad as it is.
See the issue I wrote about this (I originally assumed this issue was in the FsCodec repo when I responded to your first question; best to take this off to the side!) (ASIDE: I'd love to know if there are any outline plans/designs for how any prospective support in .NET 7 might work out - I'd like to align with that if at all possible) |
No concrete plans for the moment (it's not clear we'll have the bandwidth to pull it off in time for 7). Ideally though it should be possible to build it on top of the infrastructure to be introduced by #63747. |
I had initially hoped that it would be possible to implement support for unions using the infrastructure from polymorphism (#63747), however on closer inspection of F# union codegen, it turns out that this is not as simple as I had originally thought for a number of reasons:
Based on the above, there appear to be two possible approaches we could follow if we decide to implement support for unions in the future:
|
Is this still being investigated and worked on? As I'm quite interested in seeing STJ support F# DU's. |
It is not being worked on currently. We will update this issue as soon as something changes. |
1- Do you think it will ever be possible? I mean, should we keep hope or do all by hand? 2- While waiting: Would you recommend Wlaschin DTO approach? 2a- Else what would be your recommend, preferred approach in most cases? Thank you |
@YkTru why do it by hand when there are two perfectly viable answers to 2a:
|
Using a third-party option is perfectly fine. One thing to note about any custom converter is that it necessarily loses the ability to do streaming serialization (only because streaming converters are internal for now). In other words, attempting to serialize something large like type MyUnion = | Values of int []
JsonSerializer.SerializeAsync(stream, Values [1 .. 1_000_000]) would necessarily result in the entire payload being buffered by the serializer. That shouldn't matter much as long as you're restricted to small-ish values. |
Concerning support for discriminated unions, it might be worth pointing out that there is no one canonical way to encode DUs in JSON. DUs are used in many different ways in F# code, including the following:
type Email = Email of string
, used to provide a type-safe wrapper for common values. A user might expect thatEmail("email")
should serialize as its payload,"email"
.type Suit = Heart | Spade | Diamond | Club
. Users might reasonably expect thatSpade
should serialize as the union case identifier,"Spade"
.type Shape = Point | Circle of radius:float | Rectangle of width:float * length:float
. A value likeCircle(42.)
would require an encoding similar to{ "shape" : "circle", "radius" : 42 }
. Users should be able to specify the case discriminator property name ("shape"
) as well as the identifier for the union case (Circle
mapping to"circle"
).In light of the above, I'm increasingly starting to think that System.Text.Json should not be providing a default mechanism for serializing DUs. Users would still be able to use available custom converters that provide the union encoding that suits their use case or perhaps use unions in their data contracts in a way that bypasses the serialization layer altogether.
Originally posted by @eiriktsarpalis in #29812 (comment)
If we do decide to support F# DUs in the future, it would likely be in the form of a publicly available converter factory à la JsonStringEnumConverter that configures one or more of the above alternative serialization formats.
The text was updated successfully, but these errors were encountered: