The Wayback Machine - https://web.archive.org/web/20220301212055/https://github.com/dotnet/aspnetcore/issues/34591
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 model binding & routing #34591

Open
3 tasks
pranavkm opened this issue Jul 21, 2021 · 10 comments
Open
3 tasks

Add DateOnly and TimeOnly support to model binding & routing #34591

pranavkm opened this issue Jul 21, 2021 · 10 comments

Comments

@pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Jul 21, 2021

  • Required for Blazor/MVC
    • Model Binding
    • Blazor Routing
    • Regular Routing
@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Jul 21, 2021

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@davidfowl
Copy link
Member

@davidfowl davidfowl commented Jul 23, 2021

I want to mention that the TryParse heuristic that we added in minimal API means we support these out of the box 😄

@pranavkm
Copy link
Contributor Author

@pranavkm pranavkm commented Jul 23, 2021

Hmm, your comment reminded me that we want to special case DateTime. The default DateTime.Parse (and I imagine DateTime.TryParse), formats the date to a server local time. We had to author a model binder in MVC to parse it as UTC. @DamianEdwards did some research on this: #11584 (comment)

@mkArtakMSFT mkArtakMSFT removed this from the Next sprint planning milestone Aug 12, 2021
@mkArtakMSFT mkArtakMSFT added this to the Backlog milestone Aug 12, 2021
@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Aug 12, 2021

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@TanayParikh TanayParikh changed the title Add DateOnly and TimeOnly support to model binding (Mvc) Add DateOnly and TimeOnly support to model binding & routing Aug 23, 2021
@rafikiassumani-msft rafikiassumani-msft removed their assignment Aug 30, 2021
@srpeirce
Copy link

@srpeirce srpeirce commented Sep 17, 2021

Is this the reason I get the following error when returning DateOnly when returning via controller? Or should I raise a separate issue.

System.NotSupportedException: Serialization and deserialization of 'System.DateOnly' instances are not supported and should be avoided since they can lead to security issues. Path: $.Date.
 ---> System.NotSupportedException: Serialization and deserialization of 'System.DateOnly' instances are not supported and should be avoided since they can lead to security issues.

🤔
image

@DamianEdwards
Copy link
Contributor

@DamianEdwards DamianEdwards commented Sep 17, 2021

Seems that's from the underlying serializer in System.Text.Json, see this issue dotnet/runtime#53539

@maxkoshevoi
Copy link

@maxkoshevoi maxkoshevoi commented Sep 18, 2021

@srajkovic Here's a workaround if you're interested: https://kevsoft.net/2021/05/22/formatting-dateonly-types-as-iso-8601-in-asp-net-core-responses.html (NuGet)

@DamianEdwards DateOnly/TimeOnly promise to be popular types, and currently (as of RC1) Asp.Net doesn't support them as both body (dotnet/runtime#53539) and query (dotnet/runtime#59253) parameters. Both can be worked around with something like this, but shouldn't Asp.Net support it out of the box until runtime catches up in .Net 7? I feel like this will be a common issue after the release of .Net 6.

@srajkovic
Copy link
Contributor

@srajkovic srajkovic commented Sep 19, 2021

@maxkoshevoi, I think you meant to tag @srpeirce. DateOnly and TimeOnly look great though!

@mkArtakMSFT
Copy link
Contributor

@mkArtakMSFT mkArtakMSFT commented Jan 19, 2022

@rafikiassumani-msft some of this work will be an ask from your team for .NET 7.

@simonziegler
Copy link

@simonziegler simonziegler commented Feb 10, 2022

Are you going to review change detection for component parameters too in https://github.com/dotnet/aspnetcore/blob/main/src/Components/Components/src/ChangeDetection.cs#L37? Suggested change as below.

    // The contents of this list need to trade off false negatives against computation
    // time. So we don't want a huge list of types to check (or would have to move to
    // a hashtable lookup, which is differently expensive). It's better not to include
    // uncommon types here even if they are known to be immutable.
    private static bool IsKnownImmutableType(Type type)
        => type.IsPrimitive
        || type == typeof(string)
        || type == typeof(DateTime)
        || type == typeof(DateOnly)    // to be added
        || type == typeof(TimeOnly)    // to be added
        || type == typeof(Type)
        || type == typeof(decimal)
        || type == typeof(Guid);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants