Blazor form model expression formatting#47754
Conversation
ghost
added
the
| CachedResult = result, | ||
| }; | ||
| } | ||
| // TODO: Top-level caching. |
There was a problem hiding this comment.
I'm thinking top-level caching might not be a great optimization right now. Comparing this to MVC for a moment, the ExpressionHelper class maps cached LambdaExpression instances to strings. That strategy won't be useful in Blazor because we'll be getting a new Expression instance on each render. It will likely be good enough to make the assumption that the ValueExpression parameter won't change throughout a form input component's lifetime. That way, we only have to generate the string once per component instance.
If we did want to somehow cache on the top level, it would require us to implement our own sort of value equality checking for Expressions. And in order to check if a Expression has an entry in that cache, it would require evaluating each sub expression, meaning we would have to explore the full expression twice per string generation (once to check the cache and once to generate the string if the expression isn't in the cache). In the vast majority of cases, all that work would be to save a single allocation (the final string), which just doesn't seem worth it IMO. The existing caching mechanisms are still valid and really help optimize the case where we've seen an identical expression before.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| if (!string.IsNullOrEmpty(Name)) | ||
| { | ||
| // Prefer the explicitly-specified group name over anything else. | ||
| _context.GroupName = Name; | ||
| } | ||
| else if (!string.IsNullOrEmpty(NameAttributeValue)) | ||
| { | ||
| // If the user specifies a "name" attribute, or we're using "name" as a form field identifier, use that. | ||
| _context.GroupName = NameAttributeValue; | ||
| } | ||
| else | ||
| { | ||
| // Otherwise, just use a GUID to disambiguate this group's radio inputs from any others on the page. | ||
| _context.GroupName = _defaultGroupName; | ||
| } |
There was a problem hiding this comment.
Let's put a pin on this one, we don't have to deal with it in this PR, but it might be challenging if providing your own name breaks binding.
There was a problem hiding this comment.
Let's go with this for now, and we'll revisit in preview5
javiercn
left a comment
javiercn
left a comment
There was a problem hiding this comment.
Looks great! Only a minor piece of feedback
2 participants
Blazor form model expression formatting
With Blazor server-side rendering, we need to have the ability to map form request entries to their corresponding properties on the related model. This PR enables
Input*components to render anameattribute that makes this mapping possible.Fixes #47780