-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Added opt-in flag to enable non-public members #2246
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,7 +138,10 @@ public JsonClassInfo(Type type, JsonSerializerOptions options) | |
| { | ||
| CreateObject = options.MemberAccessorStrategy.CreateConstructor(type); | ||
|
|
||
| PropertyInfo[] properties = type.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the only fix here is to remove the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Responded in the PR that's removing it: #2278 (comment) |
||
| BindingFlags bindingFlags = options.AllowPrivateProperties | ||
| ? BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | ||
| : BindingFlags.Instance | BindingFlags.Public; | ||
| PropertyInfo[] properties = type.GetProperties(bindingFlags); | ||
|
|
||
| Dictionary<string, JsonPropertyInfo> cache = CreatePropertyCache(properties.Length); | ||
|
|
||
|
|
||
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.
Since this is an API change, it would need to go through API review. Also, I don't think we want to support non-public properties within the JsonSerializer. I haven't seen evidence that a lot of folks want this feature.
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.
An issue with supporting non-public is that it won't work with code-gen \ AOT scenarios where we can't use reflection.