[DataGrid ] Refactor DataGrid virtualization and async handling; add tests#4885
Conversation
- Always render Virtualize component; remove empty content logic for virtualization. - Remove `_renderEmptyContent` field and initialization. - Improve data loading: avoid loading if Virtualize is not ready, set `Loading` to false. - Respect cancellation tokens in async delays and query execution. - Fix pagination count calculation to prevent negative values. - Only call `ToArrayAsync` if count > 0; check for cancellation. - Update `EntityFrameworkAsyncQueryExecutor` and `ODataAsyncQueryExecutor` to handle cancellation and exceptions more robustly.
Added test projects for EntityFramework and OData adapters, updated solution file, and exposed internals for testing. Improved ODataAsyncQueryExecutor to handle cancellation and nulls. Added unit tests covering normal, cancellation, disposal, and exception scenarios.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds test coverage for DataGrid async query executors (Entity Framework + OData) and adjusts cancellation/paging/virtualization behavior to avoid unnecessary work and better handle canceled operations.
Changes:
- Added new test projects and test suites for EF and OData async query executors.
- Updated EF + OData query executors to better handle cancellation and exceptions.
- Refactored
FluentDataGridrefresh/virtualize flow and adjusted paging/item loading logic.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Extensions/DataGrid.ODataAdapter/ODataAsyncQueryExecutorTests.cs | Adds unit tests for OData async executor support detection and cancellation behavior. |
| tests/Extensions/DataGrid.ODataAdapter/Microsoft.FluentUI.AspNetCore.Components.DataGrid.ODataAdapter.Tests.csproj | Introduces a dedicated OData adapter test project with required packages. |
| tests/Extensions/DataGrid.EntityFrameworkAdapter/Microsoft.FluentUI.AspNetCore.Components.DataGrid.EntityFrameworkAdapter.Tests.csproj | Introduces a dedicated EF adapter test project with required packages. |
| tests/Extensions/DataGrid.EntityFrameworkAdapter/EntityFrameworkAsyncQueryExecutorTests.cs | Adds EF executor tests, including cancellation/dispose and ignore-exception paths. |
| src/Extensions/DataGrid.ODataAdapter/ODataAsyncQueryExecutor.cs | Improves cancellation handling and makes count conversion checked. |
| src/Extensions/DataGrid.ODataAdapter/Microsoft.FluentUI.AspNetCore.Components.DataGrid.ODataAdapter.csproj | Adds InternalsVisibleTo for the new OData adapter test assembly. |
| src/Extensions/DataGrid.EntityFrameworkAdapter/Microsoft.FluentUI.AspNetCore.Components.DataGrid.EntityFrameworkAdapter.csproj | Adds InternalsVisibleTo for the new EF adapter test assembly. |
| src/Extensions/DataGrid.EntityFrameworkAdapter/EntityFrameworkAsyncQueryExecutor.cs | Passes cancellation into the lock wait and swallows pre-cancelled operations. |
| src/Core/Components/DataGrid/FluentDataGrid.razor.cs | Changes refresh flow for virtualization and modifies delay/paging and async-query execution logic. |
| src/Core/Components/DataGrid/FluentDataGrid.razor | Always renders Virtualize when Virtualize is enabled (removes “empty content” branch). |
| Microsoft.FluentUI.sln | Adds the new test projects and expands solution configurations/platforms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| startIndex += Pagination.CurrentPageIndex * Pagination.ItemsPerPage; | ||
| count = Math.Min(request.Count, Pagination.ItemsPerPage - request.StartIndex); | ||
| count = Math.Max(request.Count, Math.Min(0, Pagination.ItemsPerPage - request.StartIndex)); |
| @if (Virtualize) | ||
| { | ||
| if (_internalGridContext.TotalItemCount == 0) | ||
| { | ||
| @_renderEmptyContent | ||
| } | ||
| else | ||
| { | ||
| <Virtualize @ref="@_virtualizeComponent" | ||
| TItem="(int RowIndex, TGridItem Data)" | ||
| ItemSize="@ItemSize" | ||
| OverscanCount="@OverscanCount" | ||
| ItemsProvider="@ProvideVirtualizedItemsAsync" | ||
| ItemContent="@(item => builder => RenderRow(builder, item.RowIndex, item.Data))" | ||
| Placeholder="@(placeholderContext => builder => RenderPlaceholderRow(builder, placeholderContext))" | ||
| SpacerElement="tr"/> | ||
| } | ||
| <Virtualize @ref="@_virtualizeComponent" | ||
| TItem="(int RowIndex, TGridItem Data)" | ||
| ItemSize="@ItemSize" | ||
| OverscanCount="@OverscanCount" | ||
| ItemsProvider="@ProvideVirtualizedItemsAsync" | ||
| ItemContent="@(item => builder => RenderRow(builder, item.RowIndex, item.Data))" | ||
| Placeholder="@(placeholderContext => builder => RenderPlaceholderRow(builder, placeholderContext))" | ||
| SpacerElement="tr"/> |
There was a problem hiding this comment.
Yeah, this changes the current behavior of the grid significantly and cannot be done this way
There was a problem hiding this comment.
Yeah completely right. I'll do it differently in a way that works for all scenarios.
| catch (DataServiceQueryException ex) when (ex.InnerException is OperationCanceledException oce) | ||
| { | ||
| ExceptionDispatchInfo.Capture(oce).Throw(); | ||
| throw; // unreachable; satisfies compiler | ||
| } |
| => [.. await ExecuteAsync(queryable, cancellationToken)]; | ||
| { | ||
| var response = await ExecuteAsync(queryable, cancellationToken); | ||
| return response is null ? default! : [.. response]; |
There was a problem hiding this comment.
Good suggestion.
| => queryable.Provider is IAsyncQueryProvider; | ||
|
|
||
| /// <inheritdoc /> | ||
| /// |
|
Hi Miguel, Valid changes I presume, but we will not merge this in it's current form. Don't want to be blunt but actually have a couple of reasons for it:
Let's work together to see if and how we can do this for v5. |
Totally understandable, and I would be happy to work on this with you for v5. Some of the changes here are hardening enhancements, but the changes involving Virtualize mode are fixes to broken behavior when we switch back and forth between virtual and non-virtual when items are provided by IAsyncQueryProvider. I do have one question for you... what's plan on moving IAsyncQueryProvider extension implementations to v5? |
- Render empty content when no items in virtualized DataGrid - Fix pagination count calculation logic - Refine exception handling in EF async executor - Simplify and annotate OData async query methods - Remove redundant OperationCanceledException catch blocks
Tests now expect OperationCanceledException for pre-cancelled tokens instead of default values. Added tests for disposal and exception predicate logic in EntityFrameworkAsyncQueryExecutor. Enhanced ThrowingQueryable<T> to implement IAsyncEnumerable<T> for better exception path coverage.
Pull Request
📖 Description
Solution and Test Project Enhancements:
EntityFrameworkAdapterandODataAdapterto the solution, improving test coverage and reliability.x64,x86) for all projects, enhancing compatibility across platforms.DataGrid Virtualization and Loading Logic:
FluentDataGrid.razorand its code-behind to ensure that data is only loaded when theVirtualizecomponent is ready, and to prevent unnecessary or duplicate data loads during the initial render. [1] [2] [3]_renderEmptyContentdelegate and related code, simplifying the component. [1] [2]Async Operation and Cancellation Improvements:
countvariable is calculated when pagination is enabled.Entity Framework Async Executor:
EntityFrameworkAsyncQueryExecutorto properly propagate and handle cancellation tokens for bothCountAsyncandToArrayAsyncmethods, and to ensure resources are released correctly even when operations are cancelled. [1] [2]These changes collectively improve the maintainability, testability, and reliability of the DataGrid component and its supporting infrastructure.
🎫 Issues
👩💻 Reviewer Notes
The following examples can be used to test behavior enhancements:
AdventureWorks
TripPin
✅ Checklist
General
Component-specific