Promote System.Text.Json as default JSON serializer

[esbenbjerre]

Description

This PR removes the Newtonsoft JSON serializer and promotes System.Text.Json as the default. I think System.Text.Json is the better choice these days. Among other things the default serializer

• Uses camel-case
• Serializes Option as value or null
• Supports ignoring Option or null values using the [<JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)>] attribute

Any other specific behaviour should IMO be implemented by users, using attributes (e.g., [<JsonPropertyName("someName")>], [<JsonConverter(typeof<SomeCustomJsonConverter>)>]), as there's no sensible defaults for most user-defined types.

How to test

Run tests and try to serialize/deserialize a JSON payload in a HttpHandler.

Related issues

#481
#560
#451

[esbenbjerre]

@dustinmoris

[TheAngryByrd]

Why are you removing Newtonsoft entirely? I can see maybe swapping the default but not outright removal.

[esbenbjerre]

It was my impression from discussion in #451 that it would be preferable to have it in a separate NuGet package and I agree with that sentiment. No reason to include the dependency by default if it's not going to be used. I also think System.Text.Json have become much better for F# with .NET 6/7 updates.

[esbenbjerre]

I also think this switch would get us closer to finally having a tryBindJson function. I'll start working on that when/if this PR is approved.

[dbrattli]

Great work. Thanks for doing this!!

[64J0]

I'll re-review this PR during this week. Thanks for the updates @esbenbjerre!

[TheAngryByrd]

Thanks for the work on this!

---

@64J0 I want to mention I think this should be considered as a major version bump. Swapping JSON serialization can be a drastic change and shouldn't be taken lightly. A major bump should be a big signal that something has changed. It should also be very loud and clear in release notes with migration links to STJ or how to put back the Newtonsoft serializer.

[64J0]

Thanks for the heads-up @TheAngryByrd! Since this PR is already open for a while, and we have many approvals, I'm going to merge it right now. Thanks for the contribution @esbenbjerre, great job!

[aspnetde]

JFYI – That turns out to be a massive breaking change to one of my applications which runs on Giraffe since 4 years. Not blaming anyone, but having some change log which makes things like that more explicit than the release page on GH would be nice ✌️

[64J0]

Hi @aspnetde, sorry for this breaking change. @abelbraaksma gave some suggestions at this issue on how to improve for future updates.

but having some change log which makes things like that more explicit than the release page on GH would be nice

Can you please expand this comment? What would you like to have found regarding this change?