search

MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.87k stars 845 forks source link

System.Text.Json migration #983

Open rubo opened 3 years ago

rubo commented 3 years ago

Are there any plans to replace Json.NET with System.Text.Json?

ArtjomP commented 3 years ago

Sorry, could you explain what are the benefits of doing this? Thank you in advance.

nopara73 commented 3 years ago

It's faster, but more importantly it enables us to remove the Newtonsoft dependency, and keeping dependencies low is important for a Bitcoin project.

knocte commented 3 years ago

Not remove Newtonsoft dependency: replace it with another one. Are you claiming that the one to be replaced with brings more legitimacy because it's from Microsoft?

nopara73 commented 3 years ago

System.Text.Json is already a dependency shipped to the users' computer regardless if we use Newtonsoft or not.

knocte commented 3 years ago

I thought the point of .NET Core was to break the BCL in many different components so that you didn't need to depend on a monolith. Did they regress on this point with .NET5? If yes, this would only be the case for this target anyway.

NicolasDorier commented 3 years ago

Not in the short term. Last time I tried playing wiht System.Text.Json, it was shit and I came back asap to newtonsoft. Waiting 1 or 2 years or somebody making all the converters.

ArtjomP commented 3 years ago

Me too. The main issue with the System.Text.Json that it's not enough just to replace the namespace and/or class name.

augustoproiete commented 3 years ago

Waiting 1 or 2 years or somebody making all the converters (@NicolasDorier)

I can take a stab at this one if there's interest in reviewing/merging. I'd suggest doing so over multiple PRs that gradually replace parts of the codebase using JSON .NET with System.Text.Json adding the necessary converters.

If necessary, we can have both side-by-side for the time being, controlled by a compile-time constant/feature flag e.g. SYSTEMTEXTJSON introduced in the first PR.

sgraphics commented 1 year ago

Hei any plans to revise this? I'm working on a Blazor/MAUI PWA app (Client side) and for me adding 2MB of Newtonsoft is bloat I'd rather avoid.