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

System.Text.Json can't deserialize ISO dateTime to DateOnly #102594

Open
marcinjahn opened this issue May 23, 2024 · 6 comments
Open

System.Text.Json can't deserialize ISO dateTime to DateOnly #102594

marcinjahn opened this issue May 23, 2024 · 6 comments
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@marcinjahn
Copy link

marcinjahn commented May 23, 2024

Description

It looks like System.Text.Json cannot deserialize from ISO 8601 datetime string to DateOnly, which I find odd, since DateOnly.Parse("2024-05-01T00:00:00") works perfectly fine.

Reproduction Steps

Here's the code:

record MyRecord(DateOnly Date);

var serialized = "{ \"Date\": \"2025-05-01T00:00:00\" }";

System.Text.Json.JsonSerializer.Deserialize<MyRecord>(serialized); // throws

Expected behavior

I'd expect no exception to be thrown and an isntance of MyRecord to be created.

Actual behavior

An exception is thrown:

The JSON value could not be converted to Submission#1+MyRecord. Path: $.Date | LineNumber: 0 | BytePositionInLine: 31.

Regression?

No response

Known Workarounds

Use DateTime instead of DateOnly.

Configuration

dotnet: 8.0.204
OS: Fedora 40
arch: x64

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 23, 2024
Copy link
Contributor

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

@gregsdennis
Copy link
Contributor

gregsdennis commented May 23, 2024

Possible duplicate of #85545 (maybe just related)

@eiriktsarpalis
Copy link
Member

I believe this was a deliberate decision for the built-in to avoid loss of information (same is true for TimeOnly). I wouldn't be opposed to a PR that extends this to full ISO dates, if you're willing to put up a PR.

cc @jozkee

@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels May 23, 2024
@eiriktsarpalis eiriktsarpalis added this to the Future milestone May 23, 2024
@marcinjahn
Copy link
Author

I think it's expected that the time portion of the datetime string will be lost when you're deserializing to a DateOnly. It's kind of similar to how you might have a type:

record (int PropOne, int PropB);

and then you try to deserialize:

{
  "PropOne": 1,
  "PropTwo": 2,
  "PropThree": 3
}

The PropThree field gets lots, and it's expected.

I will try to prepare a PR for it. I will see if I manage to build it locally. If not, I guess I could rely on PR tests.

@JaimeStill
Copy link

I ran into this issue just yesterday and want to share my experience in hopes of being able to accommodate native deserialization of ISO strings to DateOnly properties. I concur with @marcinjahn that the expectation was that the time portion of the ISO string would be lost in the deserialization and that this behavior should be supported.

Our web apps are written in Angular and make use of the Angular Material library. Our forms use the Datepicker component with the native date adapter. All of this to say that the native date adapter does not support the ability to set the parse format, which would allow us to adjust selected dates to a format that could deserialize to DateOnly on the API side. The format that it provides by default is the ISO datetime string. I could use a different date adapter, but that would require adding a whole new unnecessary npm dependency.

See the note below the tables in Customizing the parse and display formats.

What I ultimately ended up doing is creating my own DateOnly converter as follows:

public class DateOnlyJsonConverter : JsonConverter<DateOnly>
{
    public override DateOnly Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return DateOnly.FromDateTime(
            DateTime.Parse(
                reader.GetString()
                ?? DateTime.MinValue.ToString()
            )
        );
    }

    public override void Write(Utf8JsonWriter writer, DateOnly value, JsonSerializerOptions options)
    {
        writer.WriteStringValue(value.ToString());
    }
}

@julealgon
Copy link

@eiriktsarpalis

I believe this was a deliberate decision for the built-in to avoid loss of information

What about introducing a setting in the serializer options to allow or prevent loss of information then?

It seems to me that both scenarios could be valid: either someone actively wants for it to "just work" and trim the data, or they might actively want to block the behavior and make their API more strict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

5 participants