Replace Newtonsoft with System.Text.Json

[ghost]

Please describe the feature/enhancement in as much detail as possible. Recent Microsoft documents show some advantages of the System.Text.Json library such as high performance ... Although I do not deny that Newtonsoft.Json is still too good and popular, however We can replace it for a better Flurl library, limiting the dependencies of external packages

[maxkatz6]

Note that Flurl support net45;net46;netstandard1.1;netstandard1.3, which should be dropped to support System.Text.Json. Or use two json libs in same time for different targets.

[imtrobin]

No, there are still limitations with MS Json compared to newtonsoft

[ghost]

I have found an ISerialize interface, so can I implement members of this interface when using with System.Text.Json?

[tmenier]

I've heard this request before and there's definitely interest in it, but I agree with all the drawbacks as well. Aside from platform support, I'm worried I'd break a lot of people who use Json.NET's serialization attributes, global settings, etc. To do it right would probably mean releasing multiple packages. For now, I don't see the payoff being worthwhile. But I could see that changing at some point. I'll keep this issue open as a placeholder.

[snekbaev]

@tmenier agreed, no need for surprises related to serialization/deserialization for now :)

[tmenier]

@mincasoft Yes, you can definitely swap out Newtonsoft for System.Text.Json today: https://flurl.dev/docs/configuration/#serializers

For most people though, I'm pretty sure the motivation is to eliminate the Newtonsoft dependency and/or reduce the payload size. This doesn't accomplish that, but it's a way to use the MS's lib if you want to.

[rhyland]

Please leave Newtonsoft, as there is not enough support in Core 3.1, yet. Thanks.

[andrewdmay]

Using Flurl at enterprises has been difficult because of the Newtonsoft dependency being newer than the version they have been using for years in other applications and a fear of upgrades. A version that only depended upon System.Text.Json might be a way to avoid that dependency issue (even if it means that they have two JSON libraries instead of one).

[tmenier]

I'm confident this will happen in future major release, but it isn't gonna make the 3.0 cut. I could see an optional Flurl.Http.Newtonsoft package or something for those who want to stick with Json.NET.

[bonesoul]

+1 for the change.

[rcollette]

There are a couple "warnings" I see regarding the implementation suggested in https://github.com/tmenier/Flurl/issues/517#issuecomment-627961654

First is the fact that JsonSerializer.Deserialize methods are nullable but the ISerializable methods are not. Are we OK using the nullability forgiveness operator in this case?

Second is the fact that stream deserialization is asynchronous with JsonSerializer but not in ISerializable. Retrieving the Result of a ValueTask is not guaranteed to contain a value. Are we safe retrieving Result in this manner?

[panoukos41]

I came across this issue when I was searching for System.Text.Json, maybe include the following class in the library or as an extra package? It works on my application as expected.

public class SystemJsonSerializer : ISerializer
{
    private readonly JsonSerializerOptions? _options;

    public SystemJsonSerializer(JsonSerializerOptions? options = null)
    {
        _options = options;
    }

    public T Deserialize<T>(string s)
    {
        return JsonSerializer.Deserialize<T>(s, _options)!;
    }

    public T Deserialize<T>(Stream stream)
    {
        using var reader = new StreamReader(stream);
        return Deserialize<T>(reader.ReadToEnd());
    }

    public string Serialize(object obj)
    {
        return JsonSerializer.Serialize(obj, _options);
    }
}

[tmenier]

@panoukos41 Thanks, good to know that works. Hopefully others who want to use System.Text.Json today will find that useful.

[benwmills]

The mix is getting odd when developing in .NET 5. I built my POCOs with [JsonProperty] to support using Flurl, but now I want to use the same POCO for an incoming webhook and .NET 5 binding won't work.

I can add [JsonProperty] and [JsonPropertyName] to each property in the POCO, but that is a little icky.

I can avoid the binding and deserialize manually using JSON.NET.

I can also force ASP.NET to use JSON.NET across the board.

None of these options feel right. I've loved JSON.NET for a long time, but it seems like System.Text.Json is the future at some point.

I think this has been said, but ideally, Flurl wouldn't have the dependency on JSON.NET, but adding a separate bolt on package would make Flurl function as it does today.

[tmenier]

@benwmills You can also swap out the (default) Newtonsoft serializer for this one, which I think is a better option in your case than any of those you mentioned.

[jrgcubano]

We have created a package with the System.Text.Json serializer to simplify our life and for those who don't want to copy/paste all the time. https://github.com/YellowLineParking/Appy.Flurl

Thanks @panoukos41 for the code and @tmenier for the library.

[WatchDogsDev]

No Noooooooooooooooo

[dpbeaumont]

The mix is getting odd when developing in .NET 5. I built my POCOs with [JsonProperty] to support using Flurl, but now I want to use the same POCO for an incoming webhook and .NET 5 binding won't work.

I can add [JsonProperty] and [JsonPropertyName] to each property in the POCO, but that is a little icky.

I can avoid the binding and deserialize manually using JSON.NET.

I can also force ASP.NET to use JSON.NET across the board.

None of these options feel right. I've loved JSON.NET for a long time, but it seems like System.Text.Json is the future at some point.

I think this has been said, but ideally, Flurl wouldn't have the dependency on JSON.NET, but adding a separate bolt on package would make Flurl function as it does today.

Exactly the same issue here, .NET 5. Mixing both packages is causing strange behaviour and I would favour using System.Text.Json out of the box instead of including other dependencies.

[chuac]

If you're using the serializer above and are working with string representations of enums, you need to also pass in a JsonStringEnumConverter via JsonSerializerOptions.

Otherwise, you will get an exception: System.Text.Json.JsonException: The JSON value could not be converted to <Enum Type> when deserializing.

_options = new JsonSerializerOptions();
_options.Converters.Add(new JsonStringEnumConverter());

[Hawxy]

Just a note that S.T.J includes an overload that accepts a stream in .NET 6, so the example above can be simplified down to:

public class SystemJsonSerializer : ISerializer
{
    private readonly JsonSerializerOptions? _options;

    public SystemJsonSerializer(JsonSerializerOptions? options = null)
    {
        _options = options;
    }

    public T Deserialize<T>(string s) => JsonSerializer.Deserialize<T>(s, _options)!;

    public T Deserialize<T>(Stream stream) => JsonSerializer.Deserialize<T>(stream, _options)!;

    public string Serialize(object obj) => JsonSerializer.Serialize(obj, _options);

}

[tmenier]

My plan is to swap out Newtonsoft for System.Text.Json and release as 4.0-pre1 relatively soon. It should be considered reasonably stable since it will consist of virtually no other changes from the final 3.x. But if you're still not comfortable using a prerelease version, you'll need to wait a bit longer for me to break a few more things in follow-up prereleases before the final 4.0 :). Sound like a reasonable plan?

The only other thing I'm considering for prerelease 1 (since it's also related to reducing footprint) is dropping the SourceLink dependency. Not really my area of expertise, but now that symbols packages are being published to NuGet I don't think we need it anymore. If you have thoughts one way or the other, please weigh in in #513.

[gitfool]

Swapping out the json dependency sounds like a great idea to me. 😁

[tmenier]

Also adding a net6.0 target. From what I can tell, no version of .NET Standard supports System.Text.Json and therefore Flurl would always download the separate NuGet package, which probably wouldn't make the Blazor and Xamarin folks in this thread very happy I would guess 😄. (If anyone thinks I'm incorrect about that, please speak up.) If anyone needs a dependency-free version other than net6 please speak up, I'm starting with just that for now.

[Hawxy]

probably wouldn't make the Blazor and Xamarin folks in this thread very happy

Anyone using Blazor is using at minimum .NET Core 3.1 which already includes S.T.J so that isn't a problem.

Given Xamarin is on its way out in favor of MAUI (which also bundles S.T.J) I wouldn't worry about it.

.NET Standard is rapidly becoming an unpopular target in the ecosystem unless a library must work with .NET Framework.

[tmenier]

@Hawxy

Anyone using Blazor is using at minimum .NET Core 3.1 which already includes S.T.J so that isn't a problem.

The problem, and please correct me if this is wrong, is that if if someone is using .NET Core 3.1 and Flurl doesn't specifically target netcoreapp3.1, then it'll fall back on the netstandard2.0 version and pull down the STJ package from NuGet anyway, even though it's bundled, won't it? I honestly don't mind adding a netcoreapp3.1 target, I just want to make sure I fully understand whether it's necessary.

[Hawxy]

If the version consumed by Flurl is newer than the bundled version then yes, that's true. Adding the additional target would avoid the problem altogether.

[tmenier]

I've hit a bump in the road with this. Deserializing to a dynamic is a first-class feature of Flurl but isn't supported with STJ. Comment here best describes what I'm seeing in failing tests:

Deserializing into an ExpandoObject sort of works but only the root properties are "proper" expando-properties; any nested properties will be JsonElement so there is inconsistency in the programming model thus deserializing ExpandoObject should be avoided unless a custom converter is used for that.

As a side-note, this was at one time fixed with JsonNode.Parse but later removed. Related discussion here. Among other rationale, dynamic is now apparently considered "archived" tech.

Here are my options:

1. Drop support for dynamics in 4.0.
2. Add a bool property to ISerializer indicating dynamic support. If false, throw a runtime exception on those dynamic-returning methods, perhaps suggesting the NewtonsoftSerializer alternative.
3. Implement this solution, which gets us closer, but requires explicit casting of properties. So this would still fail: Assert(myDynmic.MyInt == 5), but this would pass: Assert((int)myDynmic.MyInt == 5). (The former passes with the current Newtonsoft solution.) There are other limitations as described.

Personally, I still like dynamics in some read scenarios, such as deserializing a complex object of which you only need one nested property value. I'm slightly leaning toward option 2, but I'm open to other opinions.

[cremor]

I'd go with option 1. I've never used dynamic myself unless I absolutely had to (Office interop code). I just think it's ugly and unsafe.

Yes, it might remove the need to write a few small classes for complex nested JSON structures. But I never felt that it is worth the risk.

[dpbeaumont]

I'd go with option 1. I've never used dynamic myself unless I absolutely had to (Office interop code). I just think it's ugly and unsafe.

Yes, it might remove the need to write a few small classes for complex nested JSON structures. But I never felt that it is worth the risk.

I agree dynamic should be avoided at all times if possible. Typing classes for nesting should be standard as expected.

[gitfool]

Very interesting. After reading through the various issues quoted above I also concluded that dynamic should be avoided going forward. The price of this syntactic sugar is too high and there are, albeit more verbose and ugly, alternatives available.

Also, bearing in mind that you're bumping the major version to signal a breaking change by moving from Newtonsoft.Json to System.Text.Json, this is the best time to make a clean break.

[tmenier]

Ok, you asked for it, dynamic returning methods in Flurl are getting the 🪓! #699

[tmenier]

I bumped into another issue with STJ. If you attempt to deserialize an empty response stream (or empty string) to any nullable type, you get this exception:

System.Text.Json.JsonException : The input does not contain any JSON tokens. Expected the input to start with a valid JSON token, when isFinalBlock is true.

That's different than how Newtonsoft behaves. In my mind Newtonsoft has it right. In both cases the absence of a JSON property will deserialize to a null (or default) value in the C# object by default, so why should the absence of the entire document behave any differently?

I updated the serializer to return default(T) for empty streams and strings and everything's now working as I'd expect. Does everyone agree with this decision? If so, I believe this was final hurdle to releasing this.

[cremor]

I agree that returning default is better than throwing exceptions in this case.

[tmenier]

Here's a Newtonsoft-based serializer for those who prefer not to switch:

https://gist.github.com/tmenier/158ede5f33703036a0c26080adf2b8b9

[tmenier]

The first 4.0 prerelease is available! If you've been waiting for the switch to STJ, I HIGHLY encourage you to test it out. (Conversely, if you prefer to stick with Newtonsoft, there's not much in this initial prerelease for you 😁 )

[ZombiesWithCoffee]

So far, so good on the testing of 4.0!
I am now going thru and change all my DataMember attributes to JsonPropertyName.

When you get a chance, can you write up write up on how to add System.Text.Json converters to your global Flurl settings? For now, I'm returning a text string, then converting it.

            var text = await request
                .AppendPathSegments("test", value)
                .PutJsonAsync(new TPageModel())
                .ReceiveString();

            var serializeOptions = new JsonSerializerOptions();

            serializeOptions.Converters.Add(new Int32Converter());
            serializeOptions.Converters.Add(new Int16Converter());
            serializeOptions.Converters.Add(new Int16NullableConverter());
            serializeOptions.Converters.Add(new GuidConverter());

            return JsonSerializer.Deserialize<TPageModel>(text, serializeOptions);

[tmenier]

@ZombiesWithCoffee The pattern is very similar to how you would provide custom JsonSerializerSettings with Newtonsoft. That is, the new STJ-based serializer (just called DefaultJsonSerializer) optionally take a JsonSerializerOptions constructor arg. Build the options like you did and apply it globally like this:

FlurlHttp.Configure(settings => settings.JsonSerializer = new DefaultJsonSerializer(jsonSettings));

Or apply it to a client or individual request if you don't want to mess with the global context.

(I'll update the docs when 4.0 gets closer to full release, thanks for the reminder on that.)

[ZombiesWithCoffee]

That worked beautifully! Thanks @tmenier

[ZombiesWithCoffee]

Question @tmenier - I'm having to add a 'JsonPropertyName("FirstName")' to every model that I'm serializing thru flurl with this System.Text.Json change.

Is there a global property I'm not setting? Or is it now required to add?

[tmenier]

@ZombiesWithCoffee What Flurl is doing is pretty simple and all tests are passing without any custom options. I don't know why you're seeing what you're seeing but I don't think it's a Flurl problem. Maybe some of your custom settings are squashing some default behavior or something.

[ZombiesWithCoffee]

I agree, thanks for the confirmation. I've used your product since v1. I'm searching for why all my flurl calls are suddenly returning values without the property being set, but so far, every call is returning null for all properties.

Sadly, after several hours of attempting to find the issue, I'm going to have to revert back to Newtonsoft for now.

[Hawxy]

I'm searching for why all my flurl calls are suddenly returning values without the property being set, but so far, every call is returning null for all properties.

If you'd like STJ behavior a little closer to Newtonsoft, I'd recommend setting:

new JsonSerializerOptions(JsonSerializerDefaults.Web)

It makes property names case-insensitive, among a few other things.

[tmenier]

Quick note that prerelease 2 is now available. Nothing exciting or specific to this issue, but more breaking changes you might want to keep up with.

[adam-knights]

The pre release worked really well for us, will eagerly await 4.0's full release :).

Just a note for those with issues, one of the key settings will be

var options = new JsonSerializerOptions
{
    PropertyNameCaseInsensitive = true
};

(or use JsonSerializerDefaults.Web as Hawxy mentions above)

as detailed at https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-character-casing. Newtonsoft was case insensitive by default whereas STJ is sensitive by default (resulting in your properties not populating if you had differences previously and don't set this).

Another useful difference between NS and STJ to know is around being tolerant of integers as strings in the json - https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to?pivots=dotnet-6-0#allow-or-write-numbers-in-quotes