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.08k stars 9.9k forks source link

Blazor Virtualize: ItemsProvider called repeatedly after scrolling just far enough to load more items #28653

Open mrlife opened 3 years ago

mrlife commented 3 years ago

Describe the bug

The initial render with ItemsProvider works well with an EF Core data source with around 3800 items. Scrolling just enough to cause the ItemsProvider to be called again results in it being called repeatedly. After many calls, the page becomes unresponsive. Here are a couple VS console log samples of what's happening. You can see there's a bit of a pattern between the 2 samples.

{ItemsProvider call count}: {ItemsProviderRequest.StartIndex} {ItemsProviderRequest.Count}

Sample 1

1: 54 15 2: 3 15 3: 6 15 4: 0 15 5: 1 26 6: 1216 20 7: 10 15 8: 23 15 9: 10 15 10: 23 15 11: 10 15 12: 23 15 13: 10 15 14: 23 15 15: 10 15 16: 23 15 17: 10 15 18: 23 15 19: 10 15 20: 23 15 21: 10 15 22: 23 15 23: 9 15 24: 0 15 25: 25 15 26: 9 15 27: 23 15

Sample 2

1: 54 15 2: 3 15 3: 7 15 4: 0 15 5: 1 37 6: 1791 23 7: 81 58

To Reproduce

Here is the code involved:

<Virtualize Context="item" ItemsProvider="LoadItems" ItemSize="100">
    <ItemContent>
        <CascadingValue Value="item">
            <Item />
        </CascadingValue>
    </ItemContent>
    <Placeholder>
        <p>Loading</p>
    </Placeholder>
</Virtualize>

@code {
    [Parameter]
    public IQueryable<Item> Items { get; set; }

    [Parameter]
    public int ItemsCount { get; set; }

    private int _callCount = 0;

    private async ValueTask<ItemsProviderResult<Item>> LoadItems(ItemsProviderRequest request)
    {
        var numItems = Math.Min(request.Count, ItemsCount - request.StartIndex);
        var context = ItemsService.GetContext();
        var items = await context.Items.Skip(request.StartIndex).Take(numItems).ToListAsync();

        Console.WriteLine($"{_callCount}: {request.StartIndex} {request.Count}");
        _callCount++;

        return new ItemsProviderResult<Item>(items, ItemsCount);
    }
}

Exceptions (if any)

There may or may not be some of these:

Exception thrown: 'System.Threading.Tasks.TaskCanceledException' in System.Private.CoreLib.dll

Further technical details

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.

csharpfritz commented 3 years ago

I'm observing that the ItemsProvider isn't triggered for additional data until the next batch of data would be presented on screen.

Should the ItemsProvider be called before the Overscan items are presented? If Overscan is set to 3, then should ItemsProvider be called when the first of the 3 Overscan items are scrolled into view?

SteveSandersonMS commented 3 years ago

@csharpfritz Yes, that's the intended behavior for overscan. However note that the browser may consider the "visible" area to be bigger than the exact range of pixels that you can actually see, so it can choose to fire the event a bit earlier than you might have expected. It should be pretty close to your expectations through.

If the behavior you see is substantially different from that, then it's something we should investigate.

SQL-MisterMagoo commented 3 years ago

I suspect these problems occur when the Placeholder markup does not render the exact same height as the ItemContent markup.

This causes the list to temporarily display many more items than expected, causing more requests for data - then as each item is populated, the list changes size again and fires more requests.

Make sure your ItemContent and Placeholder heights are fixed and equal.

mfluehr commented 3 years ago

I'm having this same problem. I can't set fixed heights to my items because I need my content to be able to wrap.

Trapulo commented 3 years ago

I experienced same problem with a simple placeholder:

<Placeholder> <span>Wait...</span> </Placeholder>

This caused a lot of "strange" data requests to the provider (with nonsense starting index requests: for example it asked to get data at index 9 then 200 then 9, responding to a single scroll), and, after some scroll on a long list, some kind of loop and a full crash of the browser.

Removing the placeholder has solved.

csharpfritz commented 3 years ago

After thinking and investigating, the issue appears to be at https://github.com/dotnet/aspnetcore/blob/main/src/Components/Web/src/Virtualization/Virtualize.cs#L170

The observers in the TS file are being initialized with a reference to the spacer element, which is located outside of the list of elements that were drawn in the component.

Suggested fix: alter the observers to attach them to the first elements in the Overscan collection before and after the loaded elements. I'm going to attempt to put together a fix for this and send a PR

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.

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.

LaughingJohn commented 2 years ago

I've just come across this one myself. It's odd because for me it only happens sometimes, maybe to do with the number of records or something. It's quite a bad one because it causes repeated calls to the ItemsProvider which in turn does repeated calls to my API and stresses the service, potentially causing Production issues. You can "scroll past" the issue, but if a user was to leave it in this loop state it could cause problems. This should be addressed ASAP in my humble opinion.

The pattern for me looks like this: StartIndex: 0, Count: 8 StartIndex: 1, Count: 13 StartIndex: 35, Count: 8 StartIndex: 0, Count: 8 StartIndex: 1, Count: 13 StartIndex: 35, Count: 8 StartIndex: 0, Count: 8 StartIndex: 1, Count: 13 StartIndex: 35, Count: 8 and on forever...

I also have ItemSize="100".

mwasson74 commented 2 years ago

Is there a workaround (or at least an alternative to Virtualize) for this? I am also experiencing this issue.

I'm going to attempt to put together a fix for this and send a PR

@csharpfritz did you get a fix put together for this?

@TanayParikh @mkArtakMSFT It would be neat if this could be a higher priority than it is...

andersson09 commented 2 years ago

Also experiencing this.

thirstyape commented 1 year ago

In my case this was caused by dynamically resizing the area the <Virtualize /> exists in from 100vh to 50vh. I had a collapsible filters panel that was open by default and closing it triggered this issue. Guess I will use a hover effect or something.

Anyhow, hope this helps someone.

DDR12 commented 1 year ago

It's happening for us without any placeholder templates, and without using any collapsible filters panel, just

<Virtualize TItem="ConsumableProduct" Context="item" ItemsProvider="@LoadItemsAsync" OverscanCount="5">
        <ItemContent>
            <ItemComponent DataContext="@item" />
        </ItemContent>
        @*      <Placeholder>
    <MudProgressCircular Color="Color.Default" Indeterminate="true" />
    </Placeholder>
    *@
    </Virtualize>
I can't believe the issue is 2 years old and still not yet fixed!
ghost commented 1 year ago

To learn more about what this message means, what to expect next, and how this issue will be handled you can read our Triage Process document. We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious what is causing this behavior, we would like to keep this around to collect more feedback, which can later help us determine how to handle this. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact work.

binnerup commented 11 months ago

I had this same issue, and traced it down to having a "transition: 0.4s linear;" in the CSS of the site. Removing the transition resolved the issue.

ghost commented 10 months ago

To learn more about what this message means, what to expect next, and how this issue will be handled you can read our Triage Process document. We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. Because it's not immediately obvious what is causing this behavior, we would like to keep this around to collect more feedback, which can later help us determine how to handle this. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact work.

thirstyape commented 10 months ago

Taking all bets, which version of .NET will really address this issue? I say lucky number 13.

DenisSikic commented 9 months ago

Guys this is crap. Provide a working demo that shows the data being requested before the overscan comes close to running out, or admit that Virtualize is a POS and prioritize it please.

ghost commented 8 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

quiian commented 7 months ago

I've had this myself, but after looking at the source code of QuickGrid - this does not seem to happen there. Take a look in the "Network" tab of the developer console when scrolling. https://aspnet.github.io/quickgridsamples/virtualizing

I've had problems with "all" off the shelf datagrids and virtualize. I've ended up creating my own component based on QuickGrid - no issues with Virtualize whatsover now..

Virtualize might not be perfect, and it might have bugs, but it is possible to make it work also. Would be great if it's taken a look at and improved however. :)

DenisSikic commented 7 months ago

Guys this is crap. Provide a working demo that shows the data being requested before the overscan comes close to running out, or admit that Virtualize is a POS and prioritize it please.

I ended up forking the Virtualize component and changing the typescript file, to make it work better.

EmilAlipiev commented 6 months ago

https://aspnet.github.io/quickgridsamples/virtualizing

Of course quickGrid has the same problem as well. Example for quick grid uses Grid style,once you make it with Flex, problem occurs.

umairtkxel commented 6 months ago

The issue still persists in February 2024.

oldominion commented 1 month ago

Still experience this too.

DenisSikic commented 1 month ago

I ended up having to fork microsoft's code, in the typescript file that runs the virtualizer that they're pulling in I defaulted the rootMargin parameter to 3000 and it works fine now.

function initCustomVirtualizer(dotNetHelper, spacerBefore, spacerAfter, rootMargin = 3000) { .... }

thirstyape commented 1 month ago

@DenisSikic did you find a way to load your transpiled JavaScript in place of the default library or did you need to fork the Microsoft.AspNetCore.Components.Web.Virtualization components as well?

I see the VirtualizeJsInterop class has a JsFunctionsPrefix constant that could be used to call a forked version...

DenisSikic commented 1 month ago

I took their entire ts file, converted it to js, and then took all of this: image