-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Add OnSerialize callbacks to POCOs #54709
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Tagging subscribers to this area: @eiriktsarpalis, @layomia Issue DetailsResolves #54528 Does not measurable affect performance; the
|
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IJsonOnDeserialized.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IJsonOnDeserialized.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/IJsonOnDeserializing.cs
Outdated
Show resolved
Hide resolved
} | ||
|
||
[Fact] | ||
public static void NonPocosIgnored() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What prevents us from supporting callbacks in collection types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing (same for value types) although there are very limited scenarios for both since custom converters would normally be used which implicitly have support for before\after.
Also, a richer interaction model would be better for collections - in order to provide support for "Add()" as well as holding onto any temporary collections needed for things like Arrays, Stacks and immutable collections.
/// </summary> | ||
/// <remarks> | ||
/// This behavior is only supported on types representing JSON objects. | ||
/// Types that have a custom converter or represent collections or values do not support this behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is this more clear?
/// Types that have a custom converter or represent collections or values do not support this behavior. | |
/// Types that have a custom converter or represent collections or primitive values do not support this behavior. |
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/OnSerializeTests.cs
Outdated
Show resolved
Hide resolved
d3674c1
to
a035411
Compare
Test failures in mono arm64 Debug appear unrelated:
|
Resolves #54528
Does not measurable affect performance; the
is IJsonSerializable
etc C# calls appear to be the same performance as a standard "if (bool)" check after a few runs.This does not include the work necessary for source-gen.