-
Notifications
You must be signed in to change notification settings - Fork 56
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
Change Newtonsoft.Json to System.Text.Json #112
Comments
Yes but there are different side-effects:
|
Thanks for the response. I agree it's a change that requires thought and good communication. There are many differences in behaviour however, they are not, IMO, a reason to keep the dependency given that most of them can be controlled with configuration and extension points such as ReferenceLoopHandling would be my only concern for this library and even then, to differ from the System.Text.Json implementation it's a setting which has to be explicitly set to Ignore (which this library does not do by default and I don't believe can be set via LogglyClient - please correct me if i'm wrong). |
The topic says change (replace) one with the other. I'm saying that you probably want to keep NewtonSoft around. And System.Text.Json is not a good dependency for net45. But if new net50 target is added, then one could have both with a setting (or maybe a rewrite is better)
|
Agreed - for net45 (and netstandard 1.6) I would keep Newtonsoft and have a multi target build which uses System.Text.Json for anything netstandard2.0+. My previous post was more about backward compatibility and reducing behavioural changes as much as possible to reduce the impact on consumers. |
Would it be possible to change Newtonsoft.Json to System.Text.Json? There's performance benefits, it's lighter weight and it keeps the code native to Microsoft.
The text was updated successfully, but these errors were encountered: