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

Misleading wording for serializer behavior description. #22069

Closed
voroninp opened this issue Dec 17, 2020 · 5 comments · Fixed by #22461
Closed

Misleading wording for serializer behavior description. #22069

voroninp opened this issue Dec 17, 2020 · 5 comments · Fixed by #22461
Assignees
Labels
doc-enhancement Improve the current content [org][type][category] dotnet-fundamentals/svc Pri1 High priority, do before Pri2 and Pri3

Comments

@voroninp
Copy link
Contributor

Documentation says:

Non-public constructors, including parameterless constructors, are ignored by the serializer by default.

This creates an impression there exists a way to change the default behavior for non-public constructors. I'd completely remove by default.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@dotnet-bot dotnet-bot added the ⌚ Not Triaged Not triaged label Dec 17, 2020
@PRMerger20 PRMerger20 added dotnet-fundamentals/svc Pri1 High priority, do before Pri2 and Pri3 labels Dec 17, 2020
@gewarren
Copy link
Contributor

gewarren commented Dec 17, 2020

@layomia @tdykstra should "by default" be removed from the wording? I'm unsure if the use of a JsonConverter<T> allows the use of a non-public constructor.

Recommended action

- If you own the type and it's feasible, make the parameterless constructor public.
- Otherwise, implement a JsonConverter<T> for the type and control the deserialization behavior.

@gewarren gewarren added the doc-enhancement Improve the current content [org][type][category] label Dec 17, 2020
@dotnet-bot dotnet-bot removed the ⌚ Not Triaged Not triaged label Dec 17, 2020
@gewarren gewarren self-assigned this Dec 17, 2020
@tdykstra
Copy link
Contributor

I'm unsure if the use of a JsonConverter allows the use of a non-public constructor.

Same here.

@gewarren
Copy link
Contributor

Ping @layomia.

@layomia
Copy link
Contributor

layomia commented Jan 21, 2021

Yes @gewarren, a non-public constructor can be called from a JsonConverter<T> implementation if C# accessibility rules for that scenario allow it. We could explicitly state this in our recommendation.

For .NET 6.0, we are planning a way to support using non-public constructors as a first-class feature using JsonConstructorAttribute - dotnet/runtime#38327. Support for including non-public properties/fields with JsonIncludeAttribute is planned as well - dotnet/runtime#31511.

@douglasg14b
Copy link

@layomia The docs till says Public constructor annotated with JsonConstructorAttribute and makes no mention if a non-public constructor being able to be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-enhancement Improve the current content [org][type][category] dotnet-fundamentals/svc Pri1 High priority, do before Pri2 and Pri3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants