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

Migration to System.Text.Json #96

Open
heinsenberg82 opened this issue Aug 23, 2023 · 4 comments
Open

Migration to System.Text.Json #96

heinsenberg82 opened this issue Aug 23, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@heinsenberg82
Copy link

I think it would be really great to migrate from Newtonsoft.Json to System.Text.Json. The latter is more performant than the former, in addition to being the library officially maintained by Microsoft.

If the creators are interested, I can try to help by working on this change and making a pull request.

@heinsenberg82 heinsenberg82 added the enhancement New feature or request label Aug 23, 2023
@acupofjose
Copy link
Member

I'm open to it! I don't believe it's a trivial migration however, especially in the realtime-csharp and postgrest-csharp repos, there are some Newtonsoft specific implementations that would need to be converted. But if you're willing to open a PR, please do!

@wiverson
Copy link
Collaborator

FWIW Unity also has a (slightly different) hacked up version of Newtonsoft (because of course) https://docs.unity3d.com/Packages/com.unity.nuget.newtonsoft-json@3.2/manual/index.html

I was able to get postgrest etc working using the Unity Newtonsoft instead and it seems they are binary compat. Out of curiosity do you happen to recall what was Newtonsoft specific? Guessing it's more than a call to serialize/deserialize?

@acupofjose
Copy link
Member

@wiverson sure!

For Postgrest: its the PostgrestContractResolver.cs and the Converters.

For Realtime: its the CustomContractResolver and the Converters.

Outside of those, everything should transfer pretty easily. But they are (obviously) all tied to core functionality.

@PATRICKdallat
Copy link

Would love to see this migration.

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

No branches or pull requests

4 participants