giraffe-fsharp / Giraffe

A native functional ASP.NET Core web framework for F# developers.
https://giraffe.wiki
Apache License 2.0
2.11k stars 267 forks source link

Move Utf8Json and Newtonsoft into separate packages? #451

Open dustinmoris opened 3 years ago

dustinmoris commented 3 years ago

Should Giraffe make the new System.Text JSON serializer the default and then add support for Utf8Json and Newtonsoft via two separate NuGet packages?

Most Giraffe applications will only require a single JSON serializer and therefore it feels unnecessary to create a dependency to all three serializers if only one will ever be used.

Trying to collect some feedback before making any decisions.

@TheAngryByrd @slang25 @Krzysztof-Cieslak @JonCanning @jchannon @isaacabraham

Krzysztof-Cieslak commented 3 years ago

Sounds like a good idea to me.

isaacabraham commented 3 years ago

The UTF8 one (IIRC) has a load of downstream dependencies on stuff like System.ValueTuple which shouldn't be needed on netcore since (I think) netcore2 so pulling that one out at a minimum would be a good thing.

I think most people in the F# world currently will do one of the following:

  1. Use whatever the default serializer is. I imagine this will account for the vast majority of Giraffe users, or at least until they hit an issue in terms of serialization.
  2. Use a specific serializer - probably Newtonsoft.Json or Thoth. Both work well with F# types.

System.Text.Json didn't seem quite ready for F# usage when we last looked at it, particularly for deserialization. My gut feel says to stick with Newtonsoft.JSON as the default serializer but to remove the UTF8 one (although ironically if I remember looking at the Techempower benchmarks, isn't that the fastest one!).

jchannon commented 3 years ago

The one thing I did with Carter was move it to System.Text.Json then had to add back a Json.Net serializer for people to use if they wanted to fall back on it.

However, I think for F# there are some converters etc that can make STJ workable. From a dependency perspective, STJ would be nicer but obviously there pros/cons in this area

slang25 commented 3 years ago

I think System.Text.Json will never "be ready" for F# on its own, that's what FSharp.SystemTextJson provides. I think it's still a good default though, even with the FSharp.SystemTextJson caveat, as we get System.Text.Json "for free" as a dependency, in .NET 5 it's part of the FrameworkReference.

So maybe we default it to System.Text.Json, but the docs make it clear how to extend it with FSharp.SystemTextJson, or switch it to Newtonsoft.Json or Utf8Json?

isaacabraham commented 3 years ago

If you switch to System.Text.Json by default, you would need (IMHO) to include the FSharp.SystemTextJson package as well - an F# web library must be able to serialize and deserialize F# types out of the box. That would work for me. You'll still have an extra dependency (FSharp.SystemTextJson) but at least you won't have the larger chain from UTF8Json, and we'll get the perf benefits that System Text Json brings as well.

FYI: The "full" SAFE template nowadays uses the SimpleJSON (I think!) component that Fable.Remoting wraps, whilst the minimal version of the template uses whatever Giraffe picks by default.

jcmrva commented 1 year ago

The Utf8Json repo is now archived - should it be supported at all?

TheAngryByrd commented 1 year ago

The Utf8Json repo is now archived - should it be supported at all?

Probably not. Happy to take a PR to remove it!

64J0 commented 4 months ago

Some recent updates:

We still don't have instructions on how to add FSharp.SystemTextJson. Docs on JSON serialization: https://github.com/giraffe-fsharp/Giraffe/blob/master/DOCUMENTATION.md#json.