dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.2k stars 9.94k forks source link

Handling bounce with two-way bound values in components inside cascading values #27440

Open simonziegler opened 3 years ago

simonziegler commented 3 years ago

### Background

In issue #24599, @SteveSandersonMS gave a full and very helpful description of how Blazor renders and updates values through two-way binding. This was further touched upon in another issue (I forget which - sorry), where Steve asked if we could raise an issue with a limited example - this is our response to that request.

@MarkStega and I have modified Material.Blazor to adjust to the situation as described by Steve, and are fully comfortable that our needs are met. That said, we believe that the "bounciness" that Blazor can exhibit, in particular when components are surrounded by a cascading value with IsFixed="false" will lead other developers to experience issues. To demonstrate how this can come about we have prepared a repo at https://github.com/Material-Blazor/BlazorBounce.

Examining Bounce with our repo

Fork the repo and you will have a Blazor WebAssembly project that copies a limited amount of Material.Blazor's code for select input components. It does this twice, once with our debouncing mechanism in place and another with that mechanism disabled. We debounce by comparing an inbound change to the Value parameter via two way binding with a cached value. Two way binding for Material.Blazor has complexity because we need to allow Google's Material Components Web ("MCW") to take control of component behaviour by our returning false from ShouldRender(). See our documentation site's two way binding article for more.

We present a 3x2 matrix of examples, with some guidance on how to view bounce (or lack thereof) on the demo index page itself:

You can see how bounce is exhibited by opening developer tools and looking at logging messages that our code prints with Console.WriteLine(). You will notice that the final example entitled "Bouncy / Fixed-False Cascading Value" exhibits the bounce that Steve describes.

Why we think bounce is a problem stored up for the future

To be clear: as it stands this bounce wouldn't cause issues for Material.Blazor. We believe that this is luck rather than anything else (along with our explicit debouncing in production). To understand why consider the following set of events that are occuring in the bouncy case:

So why do we think this is a problem? The answer is that we are using a third party web component framework, and each time Value is updated, we make a JSInterop call to that framework's library. It turns out that MCW is well built (no surprise there) and does not emit a change event when you use their JS functions to change a component's value. If they did emit a change, you would see the selects oscillating forever - we used to see this before we worked out how to use MCW's JS library properly.

So the issue here is a complex one of the interaction between two foundations that don't know of each other's existence: Blazor and (in our case) MCW. Blazor exhibits a certain amount of bounce and fortunately MCW subdues this effectively. Other component libraries in the future may not, and indeed there is no guarantee that MCW, presently in version 7.0.0 will continue to do so in the future.

We therefore strongly believe that Blazor would be substantially more robust if this bounce could be eliminated at source, preferably from .NET 6 onwards. We think it would be even better if Blazor were able to completely avoid attempting to render a component if its record of a Value matched that of a component's record, thus eliminating the potential for future RCLs such as Material.Blazor experiencing our growing pains and needing to resort to your issues and the Blazor community for assistance.

ghost commented 3 years ago

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.

simonziegler commented 3 years ago

Thanks @mkArtakMSFT. @SteveSandersonMS we built this reminder your consideration so please review when appropriate.

SteveSandersonMS commented 3 years ago

Thanks for submitting this, @simonziegler. We will have a look into this when we can.

If you wanted to get a quicker resolution, it might still be worth creating the absolutely smallest possible self-contained repro case, without using any code from Material.Blazor. If you're able to produce the unwanted effect in (say) 10-20 lines of code, then there's a good chance we can assess it directly by reading the code and without having to go through the lengthy process of setting up a sandbox environment in which we can run your repo's code and make sense of all its dependencies.

simonziegler commented 3 years ago

Thanks Steve, I'll try to cut it down as much as I can. May take a little while.

MarkStega commented 3 years ago

@SteveSandersonMS Simon & I cut the repository down so it has no dependency on Material.Blazor.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

Konstantin-tr commented 2 years ago

Has this issue ever been resolved? I believe I'm currently facing this exact issue and I'm not quite sure how to work around it.

MarkStega commented 2 years ago

@Konstantin-tr No, this was never resolved.