Blazor change detection too liberal

[harleydk]

The change detection seems to me a bit liberal (for lack of a better word). Given a list of N instances of some POCO model, each handed to an individual component, changing just one of them causes the entire set of components to re-render.

I made a fiddle to demonstrate the issue: https://blazorfiddle.com/s/fla3gm2q

200 instances in the list, provided (individually) to 200 sub-components. Changing just the first one causes 200 calls to OnParametersSet - I was hoping for just one.

(Ideally I'd like to be able to update a specific sub-component, but #13358. Pardon me pressing that issue, I mean no offence).

Thank you in advance for your consideration.

[RemiBou]

Maybe this can help https://docs.microsoft.com/en-us/aspnet/core/blazor/components?view=aspnetcore-3.0#use-key-to-control-the-preservation-of-elements-and-components

[harleydk]

@RemiBou Thanks, but not quite what I was looking for.

Further to the point, the discussion in regards to my issue came up in a user-group of which I'm a member, I'm trying to promote Blazor. The question was raised, "why are CPU-cycles being wasted in touching 199 sub-components , when quite obviously no changes are made to them?!" I hadn't the answer to that, and it even seemed like a very reasonable concern.

I can appreciate it might be a complicated issue, after all it's possible to attach quite complex objects as parameters. But it shouldn't be impossible to detect those changes and, besides, my audience certainly expected that to be the case. So maybe it's more of a communication-issue. The expected behavior seemed to be something along these paraphrasing lines, "I hand the data to the component and expect it to react when the data changes, I do not expect it to react when the data does not change."

[mkArtakMSFT]

Thanks for contacting us, @harleydk.
We will look into this to see how we can improve this area to make it simpler to use in the near future.

[Andrzej-W]

@harleydk I think it is not a trivial task to optimize this at the framework level. As you have written we can use complex objects as parameters. Those objects can have references to other objects and so on. To be able to detect changes framework should make a deep copy of every parameter and then compare current values to old values. Take into account that you can inject services to your components and render some values returned from these services. In my opinion only component author can make responsible decision and you can override ShouldRender method to do this.

[harleydk]

@Andrzej-W I respectfully disagree - even with complex objects, with N levels of sub-references, it should be a - relatively - simple (in implementation terms) algorithm, and a fast comparison. ShouldRender wouldn't do much good in my current situation, it brings the change-detection responsibility to the child-component. And maybe I'm the one who implemented the parent-component, but not the child-component - so I need not necessarily be the one who can make the responsible decision, you mention, on the other component author's behalf. ShouldRender, I'm not at all a fan of that construction, by the way. It feels like something - my personal opinion only - that was bolted on to solve a problem the framework should take care of.

Like I said, maybe it's more of a communication issue. So I presented Blazor to this group I'm in, and they see the OnParametersSet being touched for apparently no reason, and it suddenly gets much harder to promote Blazor with these folks.

[Madhust]

+1 for this issue. I have faced similar issue that OnParamterSet is invoked so many times and also i find it hard to get the reason for the re-render. So i had to manually check/compare the values in the OnParameterSet.

[Liander]

@harleydk The calls to OnParametersSet is a bit confusing in this situation because if you set a break-point on the parameter you have you will see that there are no parameters set and no calls to DataPiece-set in the BidAskComponent when you click your button. Right now, any existence of a parameter (which are not-null..) of reference type is making all decisions about the render-condition being passed on to the component to decide, which is not obvious, and I agree that it is not a great developer experience. The consequence is that even if you set up your own change detection for the reference type you will miss the change detection for the parameters of primitive types the framework has done and you will have to do the same work for those also.

It can also give surprising side effects. You get different behavior depending what parameters you set. Let say you transform your component to use primitive parameter types of Bid and Ask, then you can see in this fiddle that it will only render the changed item: https://blazorfiddle.com/s/gsxywn2w
But then you would expect that if you pass a delegate OnChange to that component you would only get one changed item… you don’t (because it is a non-null reference type). You can see the side effect by uncommenting the call in the same fiddle.

Treating what has changed really depends on context, like saying my delegate will never change, etc. I think what is missing is the ability to specify (and handle) additional options of the change condition that make sense in a particular situation. I will explain in the next post as soon as I get time to simulate it in fiddle.

[Liander]

Today you can test with speed render-trees of primitives, but it gives up on all references (when set), which I think should be improved. See my previous post above.

I suggest introducing a setting on the Parameter attribute to specify choice of change detection with the options: [Parameter(ChangeDetection.Default | OnInitialization | ByReferenceValue | ChildPropertyChange | TrackChangeProperties | UsingCustomEquals)]

With the following meaning:
ChangeDetection.Default – The current behavior. For reference types (that are not of a common set of immutable types such as date, string, etc) it means always true.
ChangeDetection.OnInitialization – To trigger only on initialization. It is likely the wanted setting for delegates.
ChangeDetection.ByReferenceValue – It would be used for immutable objects or objects you want to see as immutable in the context used. (I used it differently from Equals because we know this is fast and it doesn’t potentially hide what is done).
ChangeDetection.ChildPropertyChange – This would mean either the object reference of the parameter has changed or any of its child properties have changed. To be used when having a view-model-like structure to be rendered as immutable children. (Like the initial example provided in this issue).
ChangeDetection.TrackChangeProperties – This is the same as previous but while the previous compares all child properties this would use the properties you have specified in a TrackChange attribute.
ChangeDetection.UsingCustomEquals – When there is an overridden Equals you want to use. Could be used when having an entity version (from rowversion in db) you want to use.

Implementation discussions/speculation: 😊
Perhaps one could introduce another frame type for these new change detection options and test accordingly. One way to achieve tests on child properties of a type is to flatten them (first level) into attribute-like properties in the render-tree. They will then be tested for change and could be hidden when setting properties to the component. Simulated in this fiddle: https://blazorfiddle.com/s/7tfa2wh1
I think the most reasonable default on a child property of reference type is to do a reference-test and see the children as immutable. (The always render option wouldn’t make sense obviously). In case you want another setting one could have another attribute to specify it, like:
[ChangeDetectionOnChild(nameof(..property..), ChangeDetection.OnInitialization)

About the TrackChangeProperties option: Sometimes you only want to display some part of an entity, and instead of mapping everything one could have additional attributes to specify them. Simulated in this fiddle: https://blazorfiddle.com/s/eq9qwspc

About delegates and OnInitialization option: I believe that the builder could have knowledge that it is in the first render so the compiler could generate a guard for delegate construction also, like builder.AddAttribute(seq, builder.IsInitialRender ? () => MyMethod() : null), which should give some performance boost and less memory foot-print.

About template parameters: template context parameters are usually also of reference type so it feels like it would make sense to have the ability setting this change-detection-option on the template parameter tag also (perhaps in combination of ability to specify a selector).

We will look into this to see how we can improve this area to make it simpler to use in the near future.

@mkArtakMSFT I am glad to hear. Is it something along this line? Any info/plan you can share?

[harleydk]

If only we could reference individual components and call Render on them by our own discretion, that's really all it would take. Overload the ShouldRender to return false, take control of the render for any given scenario where we feel the out-of-the-box wouldn't work to our advantage. That's the kind of flexibility and control I want from a framework such as this.