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

Add DateOnly and TimeOnly support to System.Text.Json #69160

Merged
merged 4 commits into from
May 13, 2022

Conversation

eiriktsarpalis
Copy link
Member

Fix #53539.

@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone May 10, 2022
@eiriktsarpalis eiriktsarpalis self-assigned this May 10, 2022
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

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.

@ghost
Copy link

ghost commented May 10, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #53539.

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json, new-api-needs-documentation

Milestone: 7.0.0

@piekstra
Copy link

@eiriktsarpalis will this address serialization failures when DateOnly is used as a key in a dictionary on an object? I've added a custom DateOnly JsonConverter that works well when there are DateOnly properties on an object, but fails for some reason when one of the properties is Dictionary<DateOnly, decimal>

@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented May 11, 2022

@eiriktsarpalis will this address serialization failures when DateOnly is used as a key in a dictionary on an object? I've added a custom DateOnly JsonConverter that works well when there are DateOnly properties on an object, but fails for some reason when one of the properties is Dictionary<DateOnly, decimal>

Yes, the DateOnly converter comes with support for dictionary keys. In the meantime, if you are running .NET 6 it should be possible to add support for key serialization in your custom converter by overriding the WriteAsPropertyName/ReadAsPropertyName methods.

@piekstra
Copy link

Yes, the DateOnly converter comes with support for dictionary keys. In the meantime, if you are running .NET 6 it should be possible to add support for key serialization in your custom converter by overriding the WriteAsPropertyName/ReadAsPropertyName methods.

Thank you! That was the piece I was missing. Works great now 😄

@@ -13,6 +13,7 @@ private struct DateTimeParseData
public int Year;
public int Month;
public int Day;
public bool IsCalendarDateOnly;
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if it would help with #69447 (comment), but defining this field here will increase the size of this struct from 40 bytes to 48 bytes on 64-bit. If you instead move this field to the end, the size should stay at 40.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not seeing a difference in benchmarks when I rearrange the fields, but I could try pushing the change and see if it registers in the performance infrastructure.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support DateOnly and TimeOnly in JsonSerializer
9 participants