Skip to content

Conversation

@taooceros
Copy link
Member

close #252

@jjw24
Copy link
Member

jjw24 commented Jan 6, 2021

hey @taooceros have you tested this new serializer working with settings and our default plugins?

@jjw24 jjw24 added enhancement New feature or request review in progress Indicates that a review is in progress for this PR labels Jan 6, 2021
@taooceros
Copy link
Member Author

Yes, I have tested with default plugin and the setting, and found one issue. Originally, Json.Net will use ignore null value to load setting.json, while System.Text.Json doesn't successfully load that with the setting, but successfully load it without ignore null value.
Also, it doesn't support manually property ordering, so I put the plugins setting to the later part of the Settings.cs

@taooceros
Copy link
Member Author

It seems work fine.

Comment on lines 25 to +29
// use property initialization instead of DefaultValueAttribute
// easier and flexible for default value of object
_serializerSettings = new JsonSerializerSettings
_serializerSettings = new JsonSerializerOptions
{
ObjectCreationHandling = ObjectCreationHandling.Replace,
NullValueHandling = NullValueHandling.Ignore
IgnoreNullValues = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am assuming these options and comment are no longer applicable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the default behavior of System.Text.Json is replace, and ignore doesn't work well as my comment before.

@jjw24 jjw24 merged commit b348f1d into Flow-Launcher:dev Jan 7, 2021
@taooceros taooceros deleted the UpdateJson branch January 9, 2021 02:59
@jjw24 jjw24 removed the review in progress Indicates that a review is in progress for this PR label Jan 19, 2021
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 this pull request may close these issues.

Replace Newtonsoft.Json with System.Text.Json

2 participants