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

Support for record types in MVC #23465

Closed
2 tasks
javiercn opened this issue Jun 29, 2020 · 2 comments
Closed
2 tasks

Support for record types in MVC #23465

javiercn opened this issue Jun 29, 2020 · 2 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Milestone

Comments

@javiercn
Copy link
Member

javiercn commented Jun 29, 2020

The main area of focus is modelbinding/deserialization. For the most part it works, you can define record types and add properties with init only and so on and we should have no problem deserializing or binding to them, since the properties will be public and will have a setter.

We need to examine our support for record types with a "primary" constructor. For these types, I believe we don't have support out of the box. We need to evaluate the work needed to properly integrate with these types. The main issue arises from the fact that these types don't have a parameterless constructor and that can be used to create an instance of the object, and it is not possible to add one in code (as of the latest preview).

I believe the work required here is:

  • Support records with primary constructors in all formatters.
  • Support records with primary constructors in modelbinding.

I believe it is important that we support this syntax, since it significantly reduces the cognitive load of declaring a type. Consider a type hierarchy like a todo list:

public class TodoList
{
    public string Title { get; set; }
    public List<TodoItem> Items { get; init; } = new ();
}

public class TodoItem
{
    public int Id { get; init; }
    public string Description { get; init; }
    public bool Done { get; init; }
}

Compare that to

public record TodoList(string title, List<TodoItem> Items);
public record TodoItem(int Id, string Description, bool Done);

Not only the second definition is more succinct an clear, but it is also more correct and less verbose.

  • You can't create records with missing data.
  • You don't have to specify all property names when creating new instances.

For example:

  • First case:
new (){ Title ="Review pending issues", TodoItem = new List<TodoItem>() }
  • Second case
new("Review pending issues", new List<TodoItem>())
@javiercn javiercn added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jun 29, 2020
@javiercn javiercn changed the title Support for record types with ASP.NET Core Support for record types in MVC Jun 29, 2020
@javiercn
Copy link
Member Author

Model metadata is also an area where we will likely have to do work. Mostly because with primary constructors the attributes will appear in the constructor parameters, so we need to grab them from there.

@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Jun 29, 2020
@mkArtakMSFT mkArtakMSFT added this to the 5.0.0-preview8 milestone Jun 29, 2020
@mkArtakMSFT mkArtakMSFT added this to 5.0 - Preview 8 in ASP.NET Core Blazor & MVC 5.0.x Jun 29, 2020
@pranavkm pranavkm self-assigned this Jul 15, 2020
@pranavkm pranavkm moved this from 5.0 - Preview 8 (CC: 27 July) to In progress in ASP.NET Core Blazor & MVC 5.0.x Jul 15, 2020
@mkArtakMSFT mkArtakMSFT modified the milestones: 5.0.0-preview8, 5.0.0-rc1 Jul 29, 2020
@mkArtakMSFT
Copy link
Member

This is now done.

ASP.NET Core Blazor & MVC 5.0.x automation moved this from In progress to Done Jul 31, 2020
@ghost ghost added Done This issue has been fixed and removed Working labels Jul 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Aug 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Done This issue has been fixed enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
No open projects
Development

No branches or pull requests

3 participants