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

[Breaking change]: System.Text.Json default serialization format for TimeSpan #27317

Closed
2 tasks
eiriktsarpalis opened this issue Nov 30, 2021 · 1 comment · Fixed by #27655
Closed
2 tasks
Assignees
Labels
breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 6 Issues and PRs for the .NET 6 release doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3

Comments

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Nov 30, 2021

Description

System.Text.Json shipped support for TimeSpan in .NET 6 via dotnet/runtime#54186, however this change left out support for source generators. We submitted a fix via dotnet/runtime#62130 and are planning on servicing .NET 6 via dotnet/runtime#62191. This is technically a breaking change since it changes the default serialization format for TimeSpan in source generators.

Version

.NET 6 servicing (exact version TBD)

Previous behavior

In .NET 6 GA, source generators will by default serialize TimeSpan values as

{"days":0,"hours":0,"milliseconds":0,"minutes":0,"seconds":1,"ticks":10000000,"totalDays":1.1574074074074073E-05,"totalHours":0.0002777777777777778,"totalMilliseconds":1000,"totalMinutes":0.016666666666666666,"totalSeconds":1}

New behavior

The serialization format becomes consistent with that used by the reflection-based serializer:

"00:00:01"

Type of breaking change

  • Binary incompatible: Existing binaries may encounter a breaking change in behavior, such as failure to load/execute or different run-time behavior.
  • Source incompatible: Source code may encounter a breaking change in behavior when targeting the new runtime/component/SDK, such as compile errors or different run-time behavior.

Reason for change

System.Text.Json source generation is a new feature, our goal is to have serialization behavior that is as consistent as possible with the existing reflection-based serializer. This is to simplify our users' migration to source generators.

Recommended action

We believe that it is unlikely for users to depend on the current TimeSpan serialization format, as it redundantly outputs all public properties of the type --the default serialization behavior for objects-- and it is non roundtrip-able.

If users do depend on the existing behavior, the recommended course of action is to author a custom converter that outputs the needed properties from TimeSpan:

public class TimeSpanConverter : JsonConverter<TimeSpan>
{
    public void WriteValue(Utf8JsonWriter writer, TimeSpan value, JsonSerializerOptions options)
    {
        writer.WriteStartObject();
        writer.WriteNumber("days", value.Days);
        writer.WriteNumber("hours", value.Hours);
        /* insert any needed properties here */
        writer.WriteEndObject();    
    }
}

Feature area

Core .NET libraries

Affected APIs

No response

@eiriktsarpalis eiriktsarpalis added doc-idea Indicates issues that are suggestions for new topics [org][type][category] breaking-change Indicates a .NET Core breaking change Pri1 High priority, do before Pri2 and Pri3 labels Nov 30, 2021
@dotnet-bot dotnet-bot added ⌚ Not Triaged Not triaged 🏁 Release: .NET 6 Issues and PRs for the .NET 6 release labels Nov 30, 2021
@eiriktsarpalis
Copy link
Member Author

cc @ericstj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 6 Issues and PRs for the .NET 6 release doc-idea Indicates issues that are suggestions for new topics [org][type][category] Pri1 High priority, do before Pri2 and Pri3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants