Skip to content

Extend API for component attributes with render-comparer different from its value #21915

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

Closed
Liander opened this issue May 16, 2020 · 7 comments
Closed
Assignees
Labels
area-blazor Includes: Blazor, Razor Components
Milestone

Comments

@Liander
Copy link

Liander commented May 16, 2020

To get more control over re-rendering of components it would be good to be able to specify another object for comparisons to determine if an attribute has changed or will lead to another rendering result.

An initial approach could be to just support an extra method in the API for manually building a component for advanced scenarios and leave any tooling changes for later to support it from razor files.

Is your feature request related to a problem? Please describe.

It provides a manual way to get encapsulated component behavior, for performance reasons, or for any other reason (like reliable logging etc.). It is a more built-in way of controlling the re-render condition over the alternative provided now with ShouldRender which is less efficient, leads to different code execution depending on the arguments set and may thus also be sensitive in how you test it.

Describe the solution you'd like

I can see two ways of doing this. The most straightforward is just to recognize that there are cases when the object for testing for equal rendering may not be the same as the attribute value and extend the API to reflect that.

AddAttribute<T, TCompare>(int seq, string name, T value, TCompare attributeRenderComparer) 

UPDATE NOTE: The name render-comparer in the text should not be confused with an IComparer-implementation, it is instead the object that you call Equals on to compare for equal rendering.

Current behavior is achieved by just letting the render-comparer to be the same as the value for the supported types by default, and null otherwise. The internal testing for re-render can then be done by only considering all render-comparers. When manually set one can have the flexibility to completely control the condition for re-render. For example, a RenderFragment type can have a ValueTuple of the dependent values that will produce a different render result, and for event-handlers one might choose just any constant to get them to be static.

Internally it would require the RenderTreeFrame to hold an additional value. Without having studied it in detail it feels like one can use the slot that is now used for AttributeEventUpdatesAttributeName and handle that special situation differently. Perhaps include it in a two-way-binding-handler type instead?

An alternative way can be to have ability to set a flag on an attribute to indicate that it should not participate in the comparison for testing change and have ability set additional 'RenderGuard'-values as another type of render-tree-frames that participate in the test but are not included as attributes to be set as parameters.

The first alternative feels easier and it looks like it can be done by just moving around existing code.

Further described here: #13610 (comment)

@mkArtakMSFT mkArtakMSFT added area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one labels May 18, 2020
@pranavkm
Copy link
Contributor

@Liander, a comparer API doesn't work very well for this. When Blazor is comparing two values, in a lot of cases, there aren't two instances - one old and one new - of a value. Instead, there's a single instance with properties that might have changed since the previous render. If instead parameter values were immutable, Blazor could apply the same change detection behavior it applies to strings and value types. We plan on addressing this as part of #13610.

@pranavkm pranavkm removed the enhancement This issue represents an ask for new feature or an enhancement to an existing one label May 18, 2020
@pranavkm pranavkm added this to the Discussions milestone May 18, 2020
@Liander
Copy link
Author

Liander commented May 18, 2020

@pranavkm

a comparer API doesn't work very well for this.

Can you explain that? The suggestion is an extension to what is present. The comparer here is just any object that can reflect a change in the rendering that will result from that attribute. Also for parameters like RenderFragments.

When Blazor is comparing two values, in a lot of cases, there aren't two instances - one old and one new - of a value.

Yes, sometimes it is the same reference, and it might still represent another state in a subsequent render. The suggestion is universial in that it separate the value from the value you test on. You would not have them as the same object in the case you mention if you want to test deep structure as it sounds like you referring to. But it goes beyond that. For 'RenderFragment' you would likely test on some value representing the dependencies. For event-handler you would probably have a value who doesn't change.

Immutable values is just a subset, so it sounds like there are some misunderstanding of the core of the proposal. Let me know if I should make something more clear. The important part is the ability to separate the attribute value and the object you test on for that attribute and that test object can be anything that should capture different rendering.

@pranavkm
Copy link
Contributor

Can you show us an example of what a comparer instance would look like in the ordinary case? Let's use WeatherForecast instance from the Blazor template as the value to compare.

@Liander
Copy link
Author

Liander commented May 18, 2020

If you have a parameter of type WeatherForecast and you build it with this API, then a ValueTuple could be a good proxy to capture render change, so:

AddAttribute(seq, "Forecast", forecast, (forecast.Date, forecast.TemperatureC, forecast.TemperatureF, forecast.Summary))

When it comes to capture structures from references one could easily build some reflection-based type as a helper (but it would be up to the user for what is reasonable in the situation used) so you could have something like:

AddAttribute(seq, "Forcast", forecast, ValueStructure.Create(forecast))

But other interesting examples can be that you have a RenderFragment parameter

<MyComponent>
   <Header>
      <p>Hello: @User.FirstName @User.LastName</p>
   </Header>
</MyComponent>

then you would build it with

AddAttribute(seq, "Header", render-fragment-func, (User.FirstName, User.LastName))

which will give you an encapsulated component that will only render when the names changes.

And if you have a component that have primitive parameters but you want to add a callback to it without getting the side-effect of destroying its encapsulation then you would add the handler with a constant:

AddAttribute(seq, "OnClick", OnClick, true)

which will not trigger any new re-render condition (except from OnClick itseflf).

UPDATE NOTE: The name render-comparer in the text should not be confused with an IComparer-implementation, it is instead the object that you call Equals on to compare for equal rendering.

@pranavkm
Copy link
Contributor

pranavkm commented May 19, 2020

Thanks @Liander the examples help. The design sounds nice in theory, but for the the average developer, it's likely going to be hard to use and easy to get incorrect. In particular for complex or collection types, being able to author an expression that accurately represents the change is going to be non-trivial. Immutability is what we've seen other frameworks have some degree of success with, and we think it'd be much easier for use it.

That said, we aren't really done designing a solution for #13610 . I'll make sure we consider your suggestions here as a way to resolve it. Feel free to chime in the linked issue with your suggestion.

@Liander
Copy link
Author

Liander commented Jul 7, 2020

@pranavkm @SteveSandersonMS This proposal was referred to #13610 which is now closed so, is this now handled through #22432? Or does it stand on its own now?

For me this proposal is more about ability to get component encapsulation with predicted behavior (and predicted performance) rather than performance in itself, but I can see it being treated as a performance task also and it is easy to see it can be used for completely other render-conditions than mentioned here.

@ghost
Copy link

ghost commented Dec 17, 2020

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Dec 17, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 16, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

No branches or pull requests

3 participants