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

Serialize MiniProfiler\Timing class with System.Text.Json #630

Closed
KLuuKer opened this issue Dec 14, 2022 · 4 comments
Closed

Serialize MiniProfiler\Timing class with System.Text.Json #630

KLuuKer opened this issue Dec 14, 2022 · 4 comments

Comments

@KLuuKer
Copy link

KLuuKer commented Dec 14, 2022

I currently json serialize the MiniProfiler instance with Newtonsoft Json.
So i can transport the profiling data to some other service layer (yes bad design i need shared storage i know).

But i would like to completely switch to System.Text.Json instead,
and also use it's source generator to remove reflection where possible
https://devblogs.microsoft.com/dotnet/try-the-new-system-text-json-source-generator/

but it doesn't support data contract attributes....
could you add attributes that are supported by system.text.json?
https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/ignore-properties

@NickCraver
Copy link
Member

To do that, we'd have to take a dependency on System.Text.Json for everyone, which is not something I want to do (referencing 2 serializers). The way to do this currently is by implementing a resolver as I understand it - examples here: dotnet/runtime#63686. I'm not an expert on the state though, the decision to leave this out-of-box is something I disagree with as do many in the community - IMO, this should be far easier.

@KLuuKer
Copy link
Author

KLuuKer commented Dec 26, 2022

maybe it could be "solved" by resolving a IMiniProfilerSerializer with 2 implementations in different packages
would have some configuration issues, and most likely be a breaking change

or maybe a IFDEF version with a MiniProfiler.Shared-TextJson package.....
but that would most likely end up needing more -TextJson suffix version for a bunch of other packages

biggest reason i'm complaining about this is because i want to remove newtonsoft completely from my project

@NickCraver
Copy link
Member

@KLuuKer I think the only path here I can imagine (for compatibility) is to see if we can make System.Text.Json compatible 1:1 with the Newtonsoft output (so that it could read existing stored profiles). If we can do that, potentially a net6.0+ build wouldn't have any dependencies on Newtonsoft.

@NickCraver
Copy link
Member

A net6.0 build without a Newtonsoft.Json dependency will be in the next version, via #641 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants