Skip to content

Add complex form binding support to RDF #48790

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.

Sign up for GitHub

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

Jump to bottom
Merged
captainsafia merged 2 commits into from
Jun 16, 2023
Merged

Add complex form binding support to RDF #48790

captainsafia merged 2 commits into from
Jun 16, 2023

Conversation

@captainsafia
Copy link
Member

@captainsafia captainsafia commented Jun 14, 2023 •
edited
Loading

  • Add trimming annotations to shared form binding code
  • Add basic support for form-binding for complex types to RDF
var app = WebApplication.Create();

app.MapPost("/", ([FromForm] Todo todo) => Results.Ok(todo));

app.Run();

record Todo(int Name, bool IsCompleted);

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jun 14, 2023
@captainsafia captainsafia force-pushed the safia/rdf-minimal-forms branch 5 times, most recently from 8562327 to 9cd1006 Compare June 14, 2023 21:45
@captainsafia captainsafia added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-rdf and removed area-blazor Includes: Blazor, Razor Components labels Jun 14, 2023
@captainsafia captainsafia force-pushed the safia/rdf-minimal-forms branch from 9cd1006 to 723e2aa Compare June 14, 2023 22:15
@captainsafia captainsafia marked this pull request as ready for review June 14, 2023 23:52
@mitchdenny
Copy link
Member

mitchdenny commented Jun 15, 2023

Is RDG support coming later?

adityamandaleeka
adityamandaleeka reviewed Jun 15, 2023
View reviewed changes
src/Http/Http.Extensions/src/RequestDelegateFactory.cs
}

return BindParameterFromFormItem(parameter, formAttribute.Name ?? parameter.Name, factoryContext);
var useSimpleBinding = parameter.ParameterType == typeof(string) ||
Copy link
Member

@adityamandaleeka adityamandaleeka Jun 15, 2023

Choose a reason for hiding this comment

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

Nit: comment on why this is the criteria used for simple vs complex binding?

@captainsafia
Copy link
Member Author

captainsafia commented Jun 15, 2023

Is RDG support coming later?

Yep. The meta issue for this is available at #48759. "Converter source generation" outlines the requirement to codegen implementations of the converters for the serializer that are reflection-free so that we can use them in RDG.

I've done some prototyping in this area but wanted to get this MVP out for RDF to make some movement in the space.

adityamandaleeka
adityamandaleeka reviewed Jun 15, 2023
View reviewed changes
@mitchdenny
Copy link
Member

mitchdenny commented Jun 15, 2023

Gotta start somewhere. I'm a little concerned that we'll have diverging support for these kinds of features in RDF and RDG which is why I picked it up.

adityamandaleeka
adityamandaleeka reviewed Jun 15, 2023
View reviewed changes
adityamandaleeka
adityamandaleeka approved these changes Jun 15, 2023
View reviewed changes
Copy link
Member

@adityamandaleeka adityamandaleeka left a comment

Choose a reason for hiding this comment

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

Left some minor nit comments but LGTM.

MackinnonBuck
MackinnonBuck approved these changes Jun 15, 2023
View reviewed changes
Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

Looks great!

@captainsafia captainsafia merged commit c53f18a into main Jun 16, 2023
@captainsafia captainsafia deleted the safia/rdf-minimal-forms branch June 16, 2023 16:01
@ghost ghost added this to the 8.0-preview6 milestone Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adityamandaleeka adityamandaleeka adityamandaleeka approved these changes

@MackinnonBuck MackinnonBuck MackinnonBuck approved these changes

@Tratcher Tratcher Awaiting requested review from Tratcher

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy BrennanConroy is a code owner

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

Assignees

No one assigned

Labels

area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-rdf

Projects

None yet

Milestone

8.0-preview6

Development

Successfully merging this pull request may close these issues.

5 participants

@captainsafia @mitchdenny @adityamandaleeka @MackinnonBuck