-
-
Notifications
You must be signed in to change notification settings - Fork 142
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
Use System.Text.Json #251
base: main
Are you sure you want to change the base?
Use System.Text.Json #251
Conversation
Nice! I'm definitely interested in making this change. We would need to make it a major version because it would definitely be breaking. We would also need to make it easy for users to still use JSON.NET to serializer their own models. |
Windows error seems to be around not being able to load the a dll for 462 |
Bumping to net472 fixes it; but at that point is it just netstandard2.0? |
https://www.nuget.org/packages/System.Text.Json Looks like it targets 4.6.1. I’m definitely not against targeting just netstandard2.0 though. With the limited resources we have, I will prioritize ease of development over really old platforms that aren’t used by that many people. |
Dropping to 4.6.1 looks to work. I assume targeting 4.6.2 means it picks up the netstandard2.0 project instead (as that's a later version than the 4.6.1); and then its in the grey area of netstd2.0 not quite working between 4.6.1-4.7.1 (for things like valuetuples etc) |
Hmm.. CI doesn't agree with my assessment |
My biggest concern about this pr is taking a dependency on physical assemblies again. In the past a long long time ago... we ran into many runtime issue due to having no control over what version is loaded from say a VS Addon which might have multiple versions of JSON.NET (causing a crash reporting library to potentially crash the process). Also, there are some pretty crazy data types that could potentially get thrown at this. I hope Microsoft has solved both of these concerns. Another thought, for full framework we'd be forced to include any dependencies in our WIX installer at least for generator and this goes for our other customers as well (it's not the end of the world). |
Yeah, we had a lot of diamond dependency issues with users apps when we had a direct dependency on JSON.NET. I think that is still somewhat of a concern, but it seems like things have gotten a lot better in .NET since then. Worst case, we could do the same source import for STJ. As far as crazy data types, ideally the things people attach to their events are pretty simple objects, but it has to be a requirement that they can serializer in a different way and honestly, I'd be fine with that other way being that they have to serialize themselves and add it as raw JSON. |
An exploration; still needs some work