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.5k stars 10.04k forks source link

API for explicitly putting multiple renders into a single batch #42686

Open Liero opened 2 years ago

Liero commented 2 years ago

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

In my custom component (e.g. blazor wrapper for existing js library), I need to invoke EventCallback from JS.

Currently there is no way to properly invoke EventCallback on custom component from JS in such manner, so that it would behave the same way like blazor directive (@onclick).

Consider this MyComponent example:

    [Parameter] public EventCallback OnCallback1 {get; set;}
    [Parameter] public EventCallback OnCallback2 {get; set;}

    async Task Invoke() {
        await OnCallback1.InvokeAsync();
        await OnCallback2.InvokeAsync();
    }

    [JSInvokable]
    public async Task InvokeFromJS() => await Invoke(); //when calling from JS, consuming component is rerended 2x

    public async Task InvokeFromNet() => await Invoke(); //when calling from @onclick, consuming component is rerended 1x

Calling InvokeFromNet() and InvokeFromJs() causes different rerendering of the consuming component:

<MyComponent 
    OnCallback1="Callback1" 
    OnCallback2="Callback2" />

In the latter case, the consuming component is rerendered twice.

Describe the solution you'd like

Add optional parameter to [JsInvokableAttribute(EventCallback=true)]. If EventCallback=true, then blazor would do standard event loop (resp event batch, render batch whatever you call it)

Additional context

Demo: https://blazorrepl.telerik.com/cQuLbmYX188NSRxn14

SteveSandersonMS commented 2 years ago

One option is to create a .NET class that holds the EventCallback and has a method for invoking it. Then you can pass this down to JavaScript as a DotNetObjectReference and then JS can call your method.

This would give you what you're looking for today without waiting for any new framework feature, and without there being a whole different calling convention to pick from.

Would that work for you?

Liero commented 2 years ago

@SteveSandersonMS : Not sure I understand.

.NET class that holds the EventCallback and has a method for invoking it. Then you can pass this down to JavaScript as a DotNetObjectReference and then JS can call your method.

Isn't that what posted in the code sample?

If I invoke the EventCallback from [JSInvokable] method, it behaves differently, see the REPL, please.

ghost commented 2 years ago

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

SteveSandersonMS commented 2 years ago

Thanks for clarifying, @Liero. You're right that there isn't currently a way of forcing multiple renders to be in the same batch. It's currently a decision the framework makes, and in common situations makes a good choice, but there are some more advanced cases where you could want to control it more directly. This is somewhat related to (but not identical to) https://github.com/dotnet/aspnetcore/issues/22159.

It is technically possible to make this do what you want today, using a rather obscure technique to move arbitrary logic inside the async render flow by rendering a child component that does what you want inside its OnParametersSetAsync. Here's your example, with both buttons behaving as you want: https://blazorrepl.telerik.com/wwkBvMbF40J70uhY33

For now, if you want to control the batch grouping, this is the sort of thing you'd have to do. I'll update the issue title to reflect better what I think the framework could do to help.

javiercn commented 2 years ago

@SteveSandersonMS wouldn't that be the same as doing what ComponentBase does inside HandleEvent?

    public async Task InvokeFromJS()
    {
        var task = Invoke();
        var shouldAwaitTask = task.Status != TaskStatus.RanToCompletion &&
            task.Status != TaskStatus.Canceled;

        StateHasChanged();

        return shouldAwaitTask ?
            CallStateHasChangedOnAsyncCompletion(task) :
            Task.CompletedTask;

       Task CallStateHasChangedOnAsyncCompletion(Task task)
        {
            try
            {
                await task;
            }
            catch // avoiding exception filters for AOT runtime support
            {
                // Ignore exceptions from task cancellations, but don't bother issuing a state change.
                if (task.IsCanceled)
                {
                    return;
                }

                throw;
            }

            StateHasChanged();
        }
    }
SteveSandersonMS commented 2 years ago

Not quite:

  1. StateHasChanged only enqueues the render. The render can happen later.
  2. The thing that causes event-triggered renders to go into the same batch is actually inside the renderer, not inside ComponentBase: https://github.com/dotnet/aspnetcore/blob/main/src/Components/Components/src/RenderTree/Renderer.cs#L390
javiercn commented 2 years ago

@SteveSandersonMS I am still not sure I see how that matters. From my understanding the issue here is that you can't make it work like an event triggered from an EventCallback when the method is invoked from JS, which is what I believe the snippet I provided was doing.

As far as I understand, the renderer only cares about the "chunks of sync work" that happen at a given time and doesn't make any distinction on where the source of the work comes from (whether is a .NET event or a JS interop call). If your component directly implemented IHandleEvent, the result would be the same in JS and .NET, since this behavior is specific to the ComponentBase implementation, isn't it?

I am failing to understand what feature is missing on the renderer.

SteveSandersonMS commented 2 years ago

In the sample given, they are triggering two different EventCallback instances which each enqueue a render. Since there's nothing to force the renderer to see them as being in the same batch, as soon as you trigger the first one, it will complete a batch synchronously. Then you call the second one and it will complete a batch synchronously again. There isn't a natural way (besides the trick I showed in the code sample) to say "open a new batch but don't complete it until I've enqueued multiple top-level renders".

As far as I understand, the renderer only cares about the "chunks of sync work" that happen at a given time and doesn't make any distinction on where the source of the work comes from (whether is a .NET event or a JS interop call)

It does distinguish (which in this specific case is unfortunate). That's what the line of code at https://github.com/dotnet/aspnetcore/blob/main/src/Components/Components/src/RenderTree/Renderer.cs#L390 is doing.

ghost commented 2 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.

vflame commented 1 year ago

This is also useful in cases where there's a parent component with multiple child components and needs to do batch operations on the child components + control child component rendering.

For example a parent component with a List<Children> with a method that batch toggles the visibility for all of the children can result in the "single batch" operation to be split across multiple renders, depending on timing.

This causes flickering/partial updates to the browser if the all the children component updates aren't batched together in a single render. A better experience would be the ability for the parent component to suppress all children renders, and then initiate/schedule a "batched" render to the browser.

ghost commented 11 months 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.