The Wayback Machine - https://web.archive.org/web/20201117191907/https://github.com/eventflow/EventFlow/issues/678
Skip to content
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

Replace JSON.NET with System.Text.Json #678

Open
DavidRouyer opened this issue Sep 4, 2019 · 5 comments
Open

Replace JSON.NET with System.Text.Json #678

DavidRouyer opened this issue Sep 4, 2019 · 5 comments
Labels

Comments

@DavidRouyer
Copy link
Contributor

@DavidRouyer DavidRouyer commented Sep 4, 2019

.NET Core 3.0 is coming with the new System.Text.Json API with much better performance than JSON.NET, what do you think of replacing it with the new API?

@rasmus rasmus added the enhancement label Sep 5, 2019
@rasmus
Copy link
Member

@rasmus rasmus commented Sep 5, 2019

You can use it today already, simple provide an implementation that implements IJsonSerializer and EventFlow will use that instead.

Right now EventFlow targets .NET Framework 4.5.2 and System.Text.Json requires 4.6.1. I'm not sure that increasing the framework requirement for improved performance is a good idea, especially when you can change the implementation.

Might consider using different JSON implementations for different frameworks, i.e., System.Text.Json for .NET Standard and then JSON.NET for .NET Framework.

@DavidRouyer
Copy link
Contributor Author

@DavidRouyer DavidRouyer commented Sep 5, 2019

Might consider using different JSON implementations for different frameworks, i.e., System.Text.Json for .NET Standard and then JSON.NET for .NET Framework.

Looks good to me 👍

@bartelink
Copy link

@bartelink bartelink commented Sep 5, 2019

One thing to bear in mind is that STJ doesnt necessary have 100% compatibility with json.net conventions and doesn't honor json.net Converters that one might be reliant on. The choice of serializer and settings is definitely not something one'd want to happen without an explicit change somewhere - i.e. I'd expect to at least need to change a line of code to opt in.

@rasmus
Copy link
Member

@rasmus rasmus commented Sep 6, 2019

@bartelink agree

@tedvanderveen
Copy link

@tedvanderveen tedvanderveen commented Jun 2, 2020

I have submitted PR #760 that allows for more flexible use of serializers, basically not just lock in one string based serialization.
I'm very much interested in your feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.