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.47k stars 10.03k forks source link

All component parameters setters invoked when any component event is triggered. #18279

Closed HTD closed 4 years ago

HTD commented 4 years ago

Describe the bug

I have a component. The component has a parameter. The component has any event bound.

When the event is triggered the event callback should be invoked, nothing else. The element should not be redrawn, reinitialized, its state should not be changed. This is desired behavior.

Actually something else happens. When the event is triggered, the element is initialized and all of its setters are called with default values provided in .razor file. When the element contains user input - it is destroyed / overwritten.

See the demo: BlazorComponentEvents

To Reproduce

  1. Create new Blazor Server App (.NET Core 3.1).
  2. Create new folder Components in main project directory.
  3. Create new component (Components\Component1.razor) like this:
    <h3 @attributes="AdditionalAttributes" >Component1</h3>
    @code {
    [Parameter] public object Tag { get; set; }
    [Parameter(CaptureUnmatchedValues = true)] public IReadOnlyDictionary<string, object> AdditionalAttributes { get; set; }
    }
  4. Add the namespace to the _Imports.razor file:
    @using System.Net.Http
    @using Microsoft.AspNetCore.Authorization
    @using Microsoft.AspNetCore.Components.Authorization
    @using Microsoft.AspNetCore.Components.Forms
    @using Microsoft.AspNetCore.Components.Routing
    @using Microsoft.AspNetCore.Components.Web
    @using Microsoft.JSInterop
    @using BlazorComponentEvents.Shared
    @using BlazorComponentEvents.Components
  5. Place the component on the Index.razor page:
    @page "/"
    <Component1 Tag="0" @onclick="Void" />
    @code  {
    void Void() {
        ;
    }
    }
  6. Set the break point on Tag parameter setter in Component1.razor file.
  7. Run the project.
  8. Observe the Tag setter is called TWICE (why? it should be called once!)
  9. Click on the Component1 component in the browser.
  10. Observe Tag setter is called again without any reason.

Further technical details

PS C:\Users\Adam> dotnet --info .NET Core SDK (reflecting any global.json): Version: 3.1.100 Commit: cd82f021f4

Runtime Environment: OS Name: Windows OS Version: 10.0.18363 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\3.1.100\

Host (useful for support): Version: 3.1.0 Commit: 65f04fb6db

.NET Core SDKs installed: 2.1.802 [C:\Program Files\dotnet\sdk] 2.2.402 [C:\Program Files\dotnet\sdk] 3.0.100 [C:\Program Files\dotnet\sdk] 3.1.100 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed: Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Andrzej-W commented 4 years ago

@HTD Parameters are used to pass some data from parent to child components. EventCallbacks are used to pass some data from child to parent. In this section https://docs.microsoft.com/en-us/aspnet/core/blazor/components?view=aspnetcore-3.1#eventcallback you can read:

A call to StateHasChanged isn't required in the callback's method (ShowMessage). StateHasChanged is called automatically to rerender the ParentComponent

Each time component is rendered it must render all of its children and to do this correctly it has to pass all parameters again because they can have new values after callback.

HTD commented 4 years ago

@Andrzej-W I understand, for me it also looks like the components are re-rendered and this is the issue itself. I make component that doesn't change its state, then I add for example onclick EventCallback handler. The handler is empty (a function that does nothing and returns immediately). When I click the element, causing the event to trigger - it start to behave like it needs to re-render. It occurs with existing HTML/DOM events and custom events triggered like in my example by JS or any other code.

The point is - the event handler is empty, the event itself has nothing with changing state of the element. The exception is a HTML input element involved - the element is changed when user writes text in it. The text is important and the last thing we want is to redraw the element after the text is entered by user (loosing the text inside). And this is exactly what happens when I attach ANY events to the input element (like, for example focusin, focusout).

In my app I enter the text into my component, trigger the focusout event. Debugger shows that the input text is what it should be in the exact moment the focusout event is triggered. Then all setters of my element are invoked with DEFAULT values (Value is set to String.Empty).

My real life code which fails due this bug is an autocomplete component, so basically an input element with a table. As such it needs multiple event handlers. The main one: changed event used by validation, but also focusout to close the autocomplete list. And then of course my custom RowSelected when the user selects autocomplete item either with mouse click or keyboard selection.

In provided demo project I however isolated the problem to the elements that don't change the state at all. The most basic and simple ones. Nothing happens to them, and displaying the counter of the setter calls have nothing to do with the issue. You can remove all dynamic content from them and still observe the setters being called using the debugger.

In the demo there is a h3 element with a custom event.

HTD commented 4 years ago

I've changed the problem description and example in the main issue. I isolated the problem to the most basic case, absolute minimum. I described all steps to reproduce the issue from creating new project to the issue occurring.

Here's the quick workaround causing the problem to disappear:

<h3 @attributes="AdditionalAttributes" >Component1</h3>
@code {
    [Parameter]
    public object Tag { get; set; }
    [Parameter(CaptureUnmatchedValues = true)] public IReadOnlyDictionary<string, object> AdditionalAttributes { get; set; }

    #region Workaround

    public override async Task SetParametersAsync(ParameterView parameters) {
        if (IsSetParametersEnabled) {
            await base.SetParametersAsync(parameters);
            IsSetParametersEnabled = false;
        }
    }
    bool IsSetParametersEnabled = true;

    #endregion

}

The workaround allows the parameters to be set only once. However, this is a very special case which won't work properly, if we actually want to change a parameter.

I used the workaround in my big application, surprisingly - it works! However I have to set my IsSetParametersEnabled whenever I want to set a parameter in code. This produces dirty and unnecessarily complicated code. The result is however - amazing. The whole application became like 10x more responsive and now it feels all user actions are handled immediately, in real time. The application makes database queries via Entity Framework, so I assume every time the mouse cursor was moved on the screen, the Blazor engine reinitialized all elements and triggered completely redundant database queries. The current Blazor state is unacceptable - SetParametersAsync should be called only when a parameter is set, not when ANY event triggers.

enetstudio commented 4 years ago

bserve the Tag setter is called TWICE (why? it should be called once!)

This behavior is because your app is using pre-rendering. You may obviate this feature by setting the render-mode="Server" instead of render-mode="ServerPrerendered" for the component tag helper in the _Host.cshtml file

Observe Tag setter is called again without any reason

This behavior is by design. I guess it's too late to change it. A UI event such as 'click', automatically triggers re-rendering of the component which issues the event; that is, the Index component. It should also be clear that re-rendering does not occur blindly. If the component state does not change, no re-rendering occur.

Just for the record, initialization of a component takes place only once in the component's life cycle, and re-rendering occurs multiple times.

Your workaround seems to me problematic, but then I'm not the expert here. Only a learner...

Andrzej-W commented 4 years ago

You can try to use Action instead of EventCallback. This will not call StateHasChanged automatically in the parent component.

HTD commented 4 years ago

My example clearly shows the state of my component haven't changed at all. So - re-rendering happens blindly and is clearly a bug. It does not make any sense to re-render element because it's been clicked or mouse cursor moved over the element.

Please note that it would be desired, if the HTML / CSS of the element CHANGED in the event callback (handler function). It doesn't in the example, yet StateHasChanged() seems to be called, SetParametersAsync(ParameterView parameters) is called for sure.

@Andrzej-W I tested using Action instead of EventCallback and it works perfectly. Is it equivalent to EventCallback + StateHasChanged()? To test that I used following code:

<h3 @attributes="AdditionalAttributes" @onmouseover="OnMouseOverHandler" >Component1</h3>
@code {

    [Parameter] public object Tag { get; set; }

    [Parameter(CaptureUnmatchedValues = true)] public IReadOnlyDictionary<string, object> AdditionalAttributes { get; set; }

    [Parameter] public Action<object> TestEvent { get; set; }

    bool IsSetParametersEnabled = true;

    async Task OnMouseOverHandler() {
        await InvokeAsync(() => TestEvent.Invoke(this));
        StateHasChanged();
    }

}

Guess what: that works despite StateHasChanged() being called. The setter IS NOT called with the StateHasChanged(). This is normal and desired behavior. So now why EventCallback behave so counter-intuitive?

javiercn commented 4 years ago

@HTD thanks for contacting us.

Looking at the the code (didn't read through all the comments) I believe this issue is caused by the fact that the int (0) is being boxed into an object.

Does this problem happen if you change the parameter from Object to int?

This can also be the eventcallback triggering a re-render which happens automatically after each event for the component and for the callback owner I believe (I'm not 100% sure on the details, @rynowak can correct me if I'm wrong) which is by design as we want to avoid users having to explicitly call state has changed.

Andrzej-W commented 4 years ago

@HTD Read this https://github.com/dotnet/aspnetcore/issues/6351 to understand the difference between Action and EventCallback.

Then read this https://github.com/dotnet/aspnetcore/issues/7898 and especially this answer https://github.com/dotnet/aspnetcore/issues/7898#issuecomment-479863699 to understand what is going on when component is rendered.

So far I was thinking that child components are always rerendered with initial parameter values when parent component is rendered but it looks that it is not always the case, as described in this issue: https://github.com/dotnet/aspnetcore/issues/17034

HTD commented 4 years ago

Does this problem happen if you change the parameter from Object to int? Yes.

HTD commented 4 years ago

@Andrzej-W What I read made me want to rewrite some of my components to use Action instead of EventCallback feature (at least until it's fixed), however it's pointless since default HTML component's events are EventCallback type. So one little "onclick" and half of my application gets re-rendered and reset. All user input lost and so on. So for now I stick to a very ugly workaround with IsSetParametersEnabled set whenever I want to set a parameter in code. It works reliable, as long I remember to write those extra sets.

What we have here is a Heisenberg's component! It changes state by the sole fact of being observed. If some people would find an esoteric use of such feature, it should definitely be optional. For normal usage, like any UI since UIs were invented - Heisenberg's behavior is not desired. Observing the component should not change its state. The programmer might want to change the component state based on observation. I mean it's like getter triggered the setter! Getters were always used to read data, and setters were used to write data. Event handlers were used to notify about an object state, not to modify said state. And this is what happens when EventCallbacks are used in server-side Blazor.

BTW, shouldn't I change the description of the bug to "Component re-renders when any EventCallback triggers"?

Andrzej-W commented 4 years ago

@HTD please take into account that we have to have an option to rerender components with new parameter values. This is required in almost every application.

Maybe it is difficult for you to understand how Blazor works because it is different than Windows Forms or WPF. I suppose you are confusing the component parameters with the internal state of the component. If you want to initialize internal state of the component only once (and preserve it when the component is rerendered) don't use the parameter as the internal state. Declare private field in the component and initialize it only once in OnInitialized lifecycle method (https://docs.microsoft.com/en-us/aspnet/core/blazor/lifecycle?view=aspnetcore-3.1).

If I understand your problem correctly there is no bug and the behaviour is "as designed". You have to take into account that components can be rendered multiple times and don't use parameters to keep internal state of component or if you want to (or have to) do this you should implement in your component two-way binding. See here: https://docs.microsoft.com/en-us/aspnet/core/blazor/components?view=aspnetcore-3.1#data-binding and scroll down to Component parameters.

HTD commented 4 years ago

I know the component can be rendered multiple time, but I completely do not understand why the component should be re-render just because received a click or mouse cursor movement. ANY UI, not just web or Blazor should render elements on demand. Or when it's necessary to update how they look. Constantly rendering everything with full initialization doesn't make any sense. And it's pretty obvious, that unchanging parameters should keep their state.

When there is a form containing multiple input elements, they are not redrawn, reinitialized, their content clear each time a user input occurs. It would be insane. And this is how Blazor works now.

It's simple: user enters text into an input element. Are you saying that user pressing keys requires the element be re-rendered on each key press? Checking if the element was pointed or clicked requires it to be re-drawn?

In my real life case I have an input box with autocomplete list read from database. It should be re-rendered when I decide, not spuriously in random moments. User typing triggers autocomplete event, my completion list changes, I want only my autocomplete list to be redrawn (and only when the list is changed), not the input box, because it would cause the text entered by user to be destroyed. This is what actually happens if I don't use crazy workarounds for this insane behavior.

Andrzej-W commented 4 years ago

@HTD Blazor is different than WPF or some other UI frameworks but it doesn't mean that it is better or worse. It renders components in response to events. I have absolutely no experience in Angular, Vue or other JavaScript UI framework, but I think they are similar to Blazor. Because they render components in response to events you don't have to create data models and implement property change events for every property. Blazor just works and it is beautiful !!!

I completely do not understand why the component should be re-render just because received a click or mouse cursor movement.

Component is usually something more than simple HTML input element. You create components because you often want to have internal state, you want to respond to events, and in those event handlers almost always want to change some state inside or outside component.

Constantly rendering everything with full initialization doesn't make any sense.

You are wrong. Rendering with the same or different parameter values is no the same as full initialization. You should probably read this https://docs.microsoft.com/en-us/aspnet/core/blazor/lifecycle?view=aspnetcore-3.1

When there is a form containing multiple input elements, they are not redrawn, reinitialized, their content clear each time a user input occurs. It would be insane. And this is how Blazor works now.

You are wrong again. I have created a simple example which can be directly mapped to your input box with autocomplete list. Of course to make it simple and easy to understand I only display current input box value - I don't use any database nor filter any collection

Create a new Blazor application (or use existing one) and in Pages folder create new file MyInputComponent.razor:

<div>
    <div>
        InitialText: @InitialText
    </div>
    <div>
        <input @bind-value="CurrentText" @bind-value:event="oninput" />
        <button @onclick="@(() => Callback.InvokeAsync(CurrentText))">Send current value to parent</button>
    </div>
    <div>
        CurrentText: @CurrentText Last rendering: @DateTime.UtcNow
    </div>
</div>
@code {
    private string CurrentText { get; set; }
    [Parameter]
    public string InitialText { get; set; }
    [Parameter]
    public EventCallback<string> Callback { get; set; }

    protected override void OnInitialized()
    {
        base.OnInitialized();
        CurrentText = InitialText;
    }
}

In Index.razor file paste this code:

@page "/"

<div>
    <h3>First component</h3>
    <MyInputComponent InitialText="abc" Callback="OnChild1Event"></MyInputComponent>
    <h3>Second component</h3>
    <MyInputComponent InitialText="123" Callback="OnChild2Event"></MyInputComponent>
    <h3>Values sent from childreen</h3>
    <div>
        Value1: @Value1<br />
        Value2: @Value2
    </div>
    <div>
        Last parent rendering: @DateTime.UtcNow
    </div>
</div>
@code {
    public string Value1 { get; set; }
    public string Value2 { get; set; }
    private void OnChild1Event(string value)
    {
        Value1 = value;
    }
    private void OnChild2Event(string value)
    {
        Value2 = value;
    }
}

This example is simple and works perfectly. There is no "crazy workarounds for this insane behaviour." Here is a screenshot after a few experiments. Notice there is a time stamp in every component which shows when component was last rendered. Write something in input box, click on buttons and learn how everything works. obraz

enetstudio commented 4 years ago

Isn't this code buggy:

@page "/"
<Component1 Tag="0" @onclick="Void" />
@code  {
    void Void() {
        ;
    }
}

@onclick is a compiler attribute directive, as far as I know, but here it is used as though it is a component's parameter, thus each time you click on the Html element, the Index component is re-rendered( UI events force automatic re-rendering), and consequently its child component re-renders as well.

To prevent either components from re-rendering, you can define your component like this:

<h3 @attributes="AdditionalAttributes" @onclick="@(() => 
   CallMe("Hello"))">Component1</h3>

 @code {
    [Parameter]
    public object Tag   {  get;  set;   }

    [Parameter(CaptureUnmatchedValues = true)]
    public IReadOnlyDictionary<string, object> AdditionalAttributes { get; set; 
  }

    [Parameter]
    public Action<string> CallMe { get; set; }
  }
HTD commented 4 years ago

I know using Action instead of EventCallback works well. The one particular bug report concerns EventCallback trigger re-rendering of the component, which is incorrect and undesired behavior.

This is nothing right about re-rendering just because mouse event was bound to an empty user code. I already found an ugly hack to workaround this issue, however it would be so infinitely better if we just could use Blazor without resort to ugly hacks and going through unexpected behaviors.

I know doing browser based UI work seamlessly with .NET Core is a really hard problem, but something tells me that handling the events correctly is not impossible. Triggering the event does not redraw elements in web browser, it should not redraw elements in Blazor. As simple as that. Triggering one-way binding should at least consider redrawing, but not triggering just the event handler.

BTW, the incorrect behavior is not limited to HTML element based events. It's related to the EventCallback usage. The problem doesn't occur when Action is used instead of EventCallback. But AFAIR, EventCallback was made to replace Action usage for event handling in Blazor. As such, it should behave properly, without highly undesired side effects.

Let me be absolutely clear about what happens here, I will use a car example. I have a car with an alarm system. I open my car with a remote and this triggers the alarm. This is a bug, not a feature. I want just to open my car without triggering the alarm. Please don't tell me triggering the alarm each time I enter the car is necessary. It isn't. Also, don't instruct me to use mechanical lock instead the remote control, because the role of the car remote is not to trigger the alarm, but to open the doors. The role of the event handler is to notify user code that something happen, without drawing anything in the UI. Drawing, redrawing, re-rendering is like setting the car alarm when opening the doors. The doors are open, but the alarm should not be triggered.

HTD commented 4 years ago

@Andrzej-W I tested your example, it works, however it doesn't test the issue. To test it, in the file MyInputComponent.razor change the line 6 to:

<input @bind-value="CurrentText" @bind-value:event="oninput" @onmouseover="Void" />

Then add:

private void Void() { }

anywhere in the @code block.

Run and see what happens when you move your mouse cursor over the first input. It gets re-rendered, however nothing has changed in the component.

Your example is better than mine, because it shows even more issues, like the component being re-rendered even if an Action is used instead of EventCallback, however, the setters are triggered only by EventCallback.

I investigated the sources of EventCallback and ComponentBase and found nothing that could trigger this behavior, however, ComponentBase calls ParameterView methods. I didn't have time to investigate that further, but I think the engine checks which parameters changed to re-render the component if necessary. When it comes to EventCallbacks - it "thinks" they are "changed" just because they are triggered. That's, IMHO the core error. Triggered state of the event should be ignored during the checking if parameters changed.

enetstudio commented 4 years ago

It gets re-rendered, however nothing has changed in the component

You can prevent re-rendering by overriding the ShouldRender method like this:

 protected override bool ShouldRender()
 {
        return false;
 }

Note that first rendering always takes place.

HTD commented 4 years ago

The bug I described causes another bug, this time more obvious and easier to observe. In previous example please replace Component1.razor with:

<div>
    <input @bind="DateTime" type="datetime-local" />
</div>
@code {
    [Parameter] public DateTime DateTime { get; set; }
}

and Index.razor with

@page "/"
<Component1/>

Now try to set date. It doesn't work, the component resets itself on any change. It clearly looks like no feature.

@enetstudio Yes, I included this in my workaround code, it works, however it doesn't solve the problem. Sometimes the component should be re-rendered, my workaround class has a special methods for that case. When re-rendering and resetting parameters is disabled - one way bindings don't work anymore, so I need to set values manually and then enable resetting, then I call OnStateChanged().

I can paste my workaround class code here, but I doubt it would help to resolve the problem. There are workarounds, we can make components work properly, but it should not require workarounds.

Andrzej-W commented 4 years ago

@HTD In my humble opinion there is no bug in Blazor - it is simply designed to rerender component after each event. It doesn't matter what you do in your event handler code. Your example with empty Void() method is not a real life example. As I said in my previous posts people handle events because they want to do something. You can handle mouse movements to display pointer coordinates or highlight elements under pointer. You will be more than happy when you notice that it simply just works and you don't have to call StateHasChanged in your every method and you don't have to implement data models with a lot of boilerplate code to implement notification events for every property.

Your example:

<input @bind="DateTime" type="datetime-local" />

doesn't work because there is probably no support in Blazor for datetime-local. It is also not supported in at least a few browsers. You can find this in the doc:

When a user provides an unparsable value to a databound element, the unparsable value is automatically reverted to its previous value when the bind event is triggered.

Maybe Blazor doc is not perfect yet but it is a valuable source of information. Read it two times from the beginning to the and and you will see that Blazor is fantastic!

enetstudio commented 4 years ago

It doesn't work, the component resets itself on any change

Sorry, nothing to do with Blazor. I guess it is related to browser's partial and patchy support of "datetime-local". I don't think you've discovered a bug in Blazor. You may believe that the design of Blazor is rather bad, but you can't point out any existing bug . I've already said that this behavior is by design... what is bug for you is a limitation of the system for me. I'm happy with Blazor.

HTD commented 4 years ago

I reported an existing bug of resetting components when using EventCallback.

You seem to be personally offended by me reporting a bug arguing that a totally unexpected behavior is a feature, you tell me Blazor is fantastic, like instead of providing detailed report with some research and examples I just wrote Blazor is bad. It's totally immature and unprofessional.

I reported the bug TO HELP, make Blazor better. I use it to pay my bills. If I hated Blazor, I would not use it, I wouldn't take my precious time to do research and report bugs.

Please, be mature. I don't question your competence, I do not challenge you. Why do YOU question my competence and programming skills? (It's a rhetorical question.) It's nothing personal, I wanted to help and of course, I wanted help, because I just USE Blazor, I'm not Blazor developer.

It seems like I have to fix the bug myself. I hope someone reasonable will merge the pull request.

BTW: Void function doing nothing - seriously, you don't get a minimal example? Can't you imagine real world scenario when magical if appears, we first check if a condition is met, then (depending on the condition) change the component state or not? Don't you get, that the USER (a programmer) should decide whether to do something with a component or not? Don't you get it's about CHOICE, that is better than NO CHOICE? BTW, something within the if block can access databases, remote services and do other expensive things? Well, it won't show in "hello world" example. Redrawing is cheap and insignificant when used on a single component, or 5. But in the real world we can have like 50 components in the same form. Complex components made of other components. When said behavior scales, accumulates - we have a disaster.

I already found workarounds. It's possible to fix, fixing this will break nothing, but come on, change my mind the UI will break if it won't reset the elements on calling callbacks. I mean - when Actions were used - it didn't work, did it?

One more thing, I've just tested input type="datetime-local" browser support in the latest Edge (Chromium engine). It doesn't show the behavior present in Blazor, I can read and write this element with JavaScript without any problems. The only bug in the browser is ignoring system language and date format setting and using browser display language instead. I'm not sure and frankly speaking I completely don't know if the bugs are related. Maybe they aren't. Maybe they are. It's just a helpful hint. I mean reporting that the `input type="datetime-local" behaves like it's read-only. It probably should be reported as completely different issue.

Andrzej-W commented 4 years ago

This is my last message in this thread, because I think it is time to close it.

@HTD I think your reaction is too emotional. Nobody has questioned your competency. People take their free time to explain how Blazor works. You have got a few helpful tips with links to documentation and other issues. Issues on Github are primarily used to report bugs. You already know that this is not a bug. What else can we do for you?

Void function doing nothing - seriously, you don't get a minimal example?

I get it, but please read your other posts where you have written:

It gets re-rendered, however nothing has changed in the component.

Maybe after 30 years of programming I don't have enough experience yet, but as far as I know there are 3 primary UI component concepts:

  1. Components without any automatic updates. Each time you want to update information on the screen you have to call Update() function on behalf of every component.
  2. Components similar WPF where MVVM pattern is implemented and you have to create data models with property change notifications.
  3. Components which render itself after every event.

Blazor is number 3. Blazor is in production and nobody will change this fundamental behaviour.

Take into account that in a complex component you can use parameter values and data from injected services during rendering. Don't forget about CascadingParameter. Framework authors don't know in advance how your component works. The only universal way to check if something has changed is to keep a copy and compare all values after each event. It is not efficient.

In real application usually you want to render components automatically and that is default behaviour in Blazor. If you think it is a performance problem you can always override ShouldRender method as mentioned by @enetstudio above or/and use Action instead of EventCallback where it makes sense.

You already know that you should not use parameters to keep component's state - use private variables/properties or keep state in injected service. This was demonstrated in my answer here https://github.com/dotnet/aspnetcore/issues/18279#issuecomment-574776872.

If you want to keep state in parameters you can always implement two way data binding as described here: https://docs.microsoft.com/en-us/aspnet/core/blazor/components?view=aspnetcore-3.1#data-binding

You have plenty of options - choice is yours.

HTD commented 4 years ago

The only universal way to check if something has changed is to keep a copy and compare all values after each event. It is not efficient.

I'm not exactly sure if this more efficient than re-rendering the component after each event. I try to wrap my head around it and I can't. I can't understand why. One of the way to optimize many programs that draw on screen is to better detect what changed to exclude non-changed parts from updating.

The other strange thing I've noticed is when I call StateHasChanged - the setters aren't called. Why? I choose to re-render element, framework says "no". I guess obvious reasons - nothing in fact has changed. But I provide onclick handler, and it suddenly starts re-rendering on click. It's very counter intuitive to me. I haven't realized that it could look completely different for others, and maybe I'm just minority.

If you are completely positive that's desired behavior and changing it would break any existing code or its performance, sure, close the issue, what can I do? Let's say I've made a new version ComponentBase where the updates after each events are optional. The best thing about ComponentBase is when virtual methods are overridden and base methods not called - all default and automatic behavior is blocked, the components are "static" so all the dynamic behavior can be rewritten from scratch. Even the first rendering can be blocked. Very good feature, I hope it will stay in future versions.

enetstudio commented 4 years ago

One of the way to optimize many programs that draw on screen is to better detect what changed to exclude non-changed parts from updating.

But that's exactly how Blazor works... Please read about the virtual DOM created by Blazor, the Html algorithm diffs used, etc.

If you are completely positive that's desired behavior and changing it would break any existing code or its performance, sure, close the issue, what can I do?

That is not a desired behavior or a feature of Blazor... that is the heart of Blazor, actually that is Blazor itself.

Andrzej-W commented 4 years ago

The other strange thing I've noticed is when I call StateHasChanged - the setters aren't called. Why?

Where do you call this method? In child component or parent component? StateHasChanged forces rerendering of the component. If you call it in child component there is no reason to call setters for its parameters.

It's very counter intuitive to me. I haven't realized that it could look completely different for others, and maybe I'm just minority.

I suppose you simply have not enough experience with Blazor yet and you try to apply experience from other frameworks. I will repeat my suggestion from previous post:

Maybe Blazor doc is not perfect yet but it is a valuable source of information. Read it two times from the beginning to the and and you will see that Blazor is fantastic!

Liander commented 4 years ago

I feel your pain @HTD. If one neglect that you receive the tag-property as an object and that one always gets two initializations when having pre-rendering and instead concentrate on the re-render conditions that surprises you, then I can say it surprises me too. One can even see it as passing callbacks and delegates are not fully supported (yet) as event handlers so they are seen as references to dynamic state instead.

Bug? No. So by design?... well, I see it more as a workaround in the awaiting of ability to say that those function references do not represent dynamic state, although I haven't seen any backlog item to mitigate it (like having ability to specify one-time-bind or something similar), but I have seen the working of the binding syntax which has been prepared to take arguments like that.

To work around the workaround by overriding ShouldRender can be a good fit if it works for you, but it has the drawback with having you to deal with different code for checking state-change whether you set your callback or not... puh.

As you say, to observe a child component has nothing to do with the condition to render that child component. It is just that when the child component notifies its parent it most often also wants to render the child again with different parameters, so the net result is the same for most cases and it therefor might not be as bad as it sounds. But it is very confusing in other cases as you have seen.

javiercn commented 4 years ago

The behavior here is the expected.

The component is rendered twice, once during prerendering and then again after the client starts the app from the browser.

The re-render on click happens because the Onclick parameter being passed down is a delegate and Blazor doesn't have a way to know that it is the same delegate being passed as before. The rendering engine will avoid calling SetParametersAsync when it can determine all parameters are the same, but the algorithm that it uses to do so is very conservative, so if it can't really be sure the parameters all the same, it will pass on the new instances.

As some others have pointed out, re-rendering a component should not be an issue as long as you are not using the parameters to hold state (which we warn against). So you can initialize your state within OnInitializedAsync and ignore any parameters set after the initial render or as an alternative, implement your own logic in SetParametersAsync to determine if you need to update something in your component and if you need to call StateHasChanged (to trigger a re-render).

You can also control when the component should render by overriding ShouldRender and providing your own logic there.

Finally all these things are conventions in ComponentBase, if you don't like them you can create your own component base class and override SetParametersAsync to customize the default behavior in a single place that all your components can take advantage of.

I'm going to close this issue as the behavior exhibited here is by design and is not something we plan to change as this would involve a massive breaking change and we prefer the current behavior where components updates are triggered automatically as a result of events or parameters changing.