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

Migrate from Newtonsoft.Json to System.Text.Json #52

Closed
sshquack opened this issue Jan 12, 2022 · 4 comments · Fixed by #56
Closed

Migrate from Newtonsoft.Json to System.Text.Json #52

sshquack opened this issue Jan 12, 2022 · 4 comments · Fixed by #56
Labels
enhancement New feature or request

Comments

@sshquack
Copy link
Contributor

Problem I am trying to solve

  • Improve default performance, security and standards compliance of Json ser/deser
  • Currently the default scaffold builders use Newtonsoft.Json here
  • The .NET team has written a detailed article on System.Text.Json here
  • The migration guide lists the problems it solves and the gaps in comparison to Newtonsoft.Json here
  • The MassTransit project is migrating to System.Text.Json and saw a 2x increase in performance while using less memory here and here.

Solution I'd like

  • Make System.Text.Json the default. Since a lot of web APIs rely on JSON ser/de-ser, this allows newcomers to realize the performance benefits while eliminating security risk and improving JSON standards compliance.

I'd love to contribute this change, if this is something that sounds interesting to you!

@sshquack sshquack added the enhancement New feature or request label Jan 12, 2022
@pdevito3
Copy link
Owner

Hey! Yeah, this definitely sounds reasonable for HttpClientExtensionsBuilder.

I want to say that I remember there being a weird gap on the json attributes being used in entity builder that required both options when working with relationships and the Startup config also plays into that to a degree. Of course I didn't document the issue though so I might not be able to repo it though...

With that said, I'm open to giving it a try and if it does break I can document it and move it back until there is a fix since we're still pre-v1.

I'd love it if you were able to contribute! I haven't had enough interest yet to put together a Contributing.md and the project is definitely still messy in some areas, but hopefully it's straight forward enough. Essentially, you'd go into the builder files that you linked and update the scaffolding for each to whatever is appropriate. When you're ready to try the scaffolding, you can run it locally, most easily with a dotnet run new:example command. It will prompt you for a directory to toss the example in when running it locally like this and then it operates as normal. You don't need to worry about tests yet as I don't want to dump time into them yet until the project is more stable.

@Jogai
Copy link

Jogai commented Jan 12, 2022

Maybe use something like this: https://github.com/Open-NET-Libraries/Open.Serialization so projects still can make the choice.

@sshquack
Copy link
Contributor Author

@pdevito3 Great -- I'll take a crack at it and get a PR for this too shortly. No worries on the contributing doc. Thanks for the pointers on where to look. I'll mention you when it's ready.

@pdevito3
Copy link
Owner

Awesome, appreciate ya! I've got a really packed week with work, but I want to sink some time into craftsman this weekend! Feel free to ping me here or in discord if you have any questions

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

Successfully merging a pull request may close this issue.

3 participants