Skip to content

Blazor diffing behaviour inside a loop #24079

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
mayur-ekbote opened this issue Jul 17, 2020 · 14 comments
Closed

Blazor diffing behaviour inside a loop #24079

mayur-ekbote opened this issue Jul 17, 2020 · 14 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Docs This issue tracks updating documentation ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. Status: Resolved
Milestone

Comments

@mayur-ekbote
Copy link

mayur-ekbote commented Jul 17, 2020

Intuitively, programmers have been trained to mentally 'scope' the behavior inside a loop. e.g. if a variable is created inside a loop, and if that loop is re-run, the previously created instances are destroyed and new are created.

However, it doesn't intuitively work that way in blazor.

Consider an immutable blazor element:

Immutable.razor

< h3 > @value < /h3 >
@code {
[Parameter]
public string Value { get; set; }
protected override bool ShouldRender() => false;
}

Create an 'immutable' razor component by setting "protected override bool ShouldRender() => false"

Now, create a set of component instances inside a loop:

@for (int i = 0; i < count; i++)
{
<Immutable Value="@("value"+DateTime.Now.Ticks)" />
}

Set the value of 'count' to a number say 5

protected override void OnInitialized()
{
count = 5;
}

This will render 5 values with some timestamp.

Change 'count' dynamically to 2. This will render only 2 values (as expected). However, these two will have the old timestamp.

This is as expected: Blazor had 5 elements to start with. Later it was asked to keep only 2 which were not allowed to re-render. This will also happen if the parameter values are processed and captured in OnInitialized() only and not in OnParametersSet() even if ShouldRender() is not overridden.

I feel that this should be explicitly documented somewhere. I am 100% sure if it is.

However, this is an asymmetric behavior. Suppose you increase the count from 2 to 4. Blazor has already discarded the 3 components in the first loop, so it creates 2 new components.

For example: Suppose the element shows (total count multiplied by its index)^2.
Count = 5 => (5i)^2 : For all elements
Count = 2 => (5
i)^2 : For all elements
Count = 4 => (5i)^2 : For first 2 elements, and (4i)^2 for next 2 elements.

This might create unpredictable situations, especially if the developer is using component libraries and he is not 100% sure what is getting set at which stage inside the components.

Note that setting everything inside OnParameterSet may not be desirable all the time, especially if one wants to use immutable components.

Solution

One of the following:

  1. A flag/attribute to describe that a component created inside a loop is always reinitialized when the loop is re-run.
  2. A way to know which parameters are never changed. Possibly through code analyzer/attribute/compiler warning.
@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Jul 19, 2020
@mayur-ekbote
Copy link
Author

If you only want it to check for render changes only when parameter values change then

  • override SetParametersAsync
  • check if any values submitted are different
  • if so, set a flag
  • ShouldRender should return the value of that flag
  • override OnAfterRender and set the flag back to false

The default behavior is not a problem if the author of the component and the consumer are the same. There is a potential to cause 'unintended' side effects and overall confusion if the component is made available as a third-party library or even when the code gets into the maintenance mode.

The crux of the problem (IMO) is the semantic overlapping of OnInitialized and OnParameterSet. OnInitialized is similar to a constructor and OnParameterSet is like a property setter. However, in Razor syntax, both are clubbed into one. When attributes are set, they are both arguments for the 'constructor' as well as the property setter. But as a consumer, I am not aware of the parameter setting is guaranteed to happen.

@mayur-ekbote
Copy link
Author

@key preserves the elements. The use case is for the exact opposite effect. I don't want Blazor to preserve the state.

@mayur-ekbote
Copy link
Author

Yes. But I believe that the use case I have described was not the purpose of introducing @key. Having said that, the blazor documentation does mention that you can use it for discarding everything in case the parameter has changed.

"_You can also use @key to prevent Blazor from preserving an element or component subtree when an object changes:

... content that depends on currentPerson ...
If @currentperson changes, the @key attribute directive forces Blazor to discard the entire
and its descendants and rebuild the subtree within the UI with new elements and components. This can be useful if you need to guarantee that no UI state is preserved when @currentperson changes._"

However, it also goes on to mention:
"Ensure that values used for @key don't clash. If clashing values are detected within the same parent element, Blazor throws an exception because it can't deterministically map old elements or components to new elements or components. Only use distinct values, such as object instances or primary key values."

Which begs the question, what do you do if 1) there are multiple parameters 2) the values are not unique?

@mkArtakMSFT
Copy link
Contributor

@SteveSandersonMS FYI

@mkArtakMSFT mkArtakMSFT added the Docs This issue tracks updating documentation label Jul 20, 2020
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Jul 20, 2020
@ghost
Copy link

ghost commented Jul 20, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@mkArtakMSFT
Copy link
Contributor

Also related #10476

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jul 22, 2020

Considering how @key provides control over retention of child components, I'm not sure what else there is need for us to do here.

Which begs the question, what do you do if 1) there are multiple parameters 2) the values are not unique?

@key and parameters are independent. Keep the key the same if you want to retain this child instance; make it different if you want to force recreation of a new child instance. So it doesn't matter how many parameters you have, and it's up to you to control whether the @key values are unique.

If there's something missing in the functionality, could you please clarify with a simple example?

@mayur-ekbote
Copy link
Author

@SteveSandersonMS The original post was not about @key. The discussion went in that direction based on the replies.

The crux is that blazor preserves the elements without making it apparent. It is vaguely mentioned in the docs:

Even if @key isn't used, Blazor preserves child element and component instances as much as possible.

What are the conditions for preserving elements? It is not clear.

@SteveSandersonMS
Copy link
Member

@mayur-ekbote If you're not using @key, then the conditions are undefined, because it's an implementation detail that may change. The docs imply that the diffing system will make its best effort to preserve as much as it can, but it's not making any promises about what actually gets preserved because that's a tradeoff against performance, and we may change the details of that over time.

If you want to be in control over whether elements and components are preserved, then use @key, because that's explicitly the feature that gives you control and guarantees about things being preserved or not preserved.

@mayur-ekbote
Copy link
Author

mayur-ekbote commented Jul 24, 2020

Sure. But the key has to be unique. In many UI situations, it would involve using a surrogate key likey a Guid. That is an overhead (Also overdesigning?). A simple flag based solution would be a more elegant approach IMO. It would make the intention of the designer clear and would still let you abstract away all the diffing details.

Anyway, even if it is not implemented, a dedicated, in-depth description in the official documentation would be great.

@SteveSandersonMS
Copy link
Member

A flag alone doesn't tell us which old record should correspond to which new record.

@mayur-ekbote
Copy link
Author

Something like this:
enum PreserveState{
Never, //Never preserve components declared inside the loop
Default, // Blazor behavior.
Always // Always preserve the elements, which is also the current implementation.
}

The objective is not tracking the state of the component (like, say, EfCore does). Rather, declaratively state the intention.

@mrpmorris
Copy link

If the component should be rendered differently then you need to pass it some different state. If it should be rendered the same then it should be passed the same state and it will render the same output.

I don't actually understand your scenario. You want immutable components that render different output whilst accepting the original values as parameters? I'm confused. Can you elaborate?

@SteveSandersonMS SteveSandersonMS self-assigned this Sep 21, 2020
@SteveSandersonMS
Copy link
Member

We marked this as a docs issue, but I think it's already covered in docs. In the docs about @key, we already talk about how, in the absence of @key, Blazor's diffing system attempts to preserve component/element identity, and that if you actually want guarantees about things definitely being preserved or discarded, you can control it using @key.

If there is any further customer confusion about this then we can consider rephrasing or emphasising different parts of the docs, but right now since it's already covered and there isn't widespread confusion, I don't think there's any doc changes that would be justified.

@SteveSandersonMS SteveSandersonMS added the ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. label Oct 2, 2020
@ghost ghost added the Status: Resolved label Oct 2, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Nov 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Docs This issue tracks updating documentation ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. Status: Resolved
Projects
None yet
Development

No branches or pull requests

4 participants