Skip to content

MudDataGrid: Add CancellationToken support for ServerData#11841

Merged
danielchalmers merged 18 commits intoMudBlazor:devfrom
w3ori:feature/datagrid-server-side-paging-cancellationtoken
Nov 23, 2025
Merged

MudDataGrid: Add CancellationToken support for ServerData#11841
danielchalmers merged 18 commits intoMudBlazor:devfrom
w3ori:feature/datagrid-server-side-paging-cancellationtoken

Conversation

@w3ori
Copy link
Contributor

@w3ori w3ori commented Aug 30, 2025 •
edited by ScarletKuro
Loading

Description

Add CancellationToken support for ServerData with to prevent unnecessary data loading.
My main issue was with rapidly go over multiple pages and it loaded the data for every skipped page.

Try to resolve: #11185

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.
  • I've fixed/modified existing relevant tests.

gemini-code-assist[bot]
gemini-code-assist bot reviewed Aug 30, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces CancellationToken support to the ServerData delegate in MudDataGrid, which is a great improvement for preventing unnecessary data loading, especially when users navigate through pages quickly. The implementation looks solid. My feedback focuses on updating the XML documentation to reflect this new parameter and ensuring the example files correctly demonstrate its usage by passing the token to Task.Delay calls.

@w3ori
Copy link
Contributor Author

w3ori commented Aug 30, 2025

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Aug 30, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces CancellationToken support for ServerData in MudDataGrid, which is a valuable enhancement to prevent unnecessary data loading, particularly when navigating through pages quickly. The implementation correctly propagates the cancellation token to asynchronous operations in most of the updated examples and tests.

My review has identified a critical memory leak in the new cancellation logic within MudDataGrid.razor.cs. The CancellationTokenSource is not being disposed of correctly, which could lead to performance degradation over time. Additionally, there are several violations of the C# style guide regarding the naming of asynchronous methods across multiple files. Addressing these issues will improve the stability and maintainability of the code.

@w3ori w3ori changed the title MudDataGrid: Add CancellationToken support for ServerData with MudDataGrid: Add CancellationToken support for ServerData Aug 31, 2025
@ScarletKuro ScarletKuro added the breaking change This change will require consumer code updates label Sep 3, 2025
@w3ori
Copy link
Contributor Author

w3ori commented Sep 3, 2025

@ScarletKuro Can it have a default value to not be a breaking change?
For example CancellationToken.None ?
And remove that later when breaking change is acceptable?

@ScarletKuro
Copy link
Member

ScarletKuro commented Sep 3, 2025

@ScarletKuro Can it have a default value to not be a breaking change?

You can't have default value for a Func

MudBlazor/src/MudBlazor/Components/DataGrid/MudDataGrid.razor.cs

Line 960 in 59d0d8f

public Func<GridState<T>, CancellationToken, Task<GridData<T>>> ServerData { get; set; }

Anyone using the ServerData parameter will now be required to include a CancellationToken argument, regardless of whether they use it or not. Your changes to every single test case demonstrate this very well. Therefore, it's a breaking change.

@w3ori
Copy link
Contributor Author

w3ori commented Sep 3, 2025

Thanks for the explanation.
Breaking changes can be merged only when a new main version comes out, for example v9?

@ScarletKuro
Copy link
Member

ScarletKuro commented Sep 3, 2025

Breaking changes can be merged only when a new main version comes out, for example v9?

yeah

@w3ori
Copy link
Contributor Author

w3ori commented Sep 3, 2025

Okay, is there a separate branch for there to move the PR there?
Or I will just leave it here.

@versile2
Copy link
Contributor

versile2 commented Sep 25, 2025

Just leave here for now and since it's a breaking change, go ahead and change the name to append Async and add the ?.Dispose to CancelServerData. This will likely need to be updated to the latest dev occasionally.

V

@mudbot mudbot bot added enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library needs: changes A maintainer has asked for further modifications to be made to this pull request labels Sep 25, 2025
@mudbot

This comment was marked as duplicate.

Sign in to view
@mudbot mudbot bot changed the title MudDataGrid: Add CancellationToken support for ServerData MudDataGrid: Add CancellationToken support for ServerDataAsync Sep 25, 2025
@w3ori
Copy link
Contributor Author

w3ori commented Sep 26, 2025

Just leave here for now and since it's a breaking change, go ahead and change the name to append Async and add the ?.Dispose to CancelServerData. This will likely need to be updated to the latest dev occasionally.

V

I renamed ServerData to ServerDataAsync.

Is Dispose on the correct place? b43c399

Should I rename test razor files also from ServerData to ServerDataAsync in filenames?

I also updated to recent dev branch.

@ScarletKuro
Copy link
Member

ScarletKuro commented Sep 26, 2025

go ahead and change the name to append Async

I renamed ServerData to ServerDataAsync.

I don't think that's a good idea. Yes, normal properties or methods should have an Async prefix, but we're talking about a component parameter here. I haven't seen anyone use this in practice for a [Parameter]. Moreover, this now deviates from MudTable:

MudBlazor/src/MudBlazor/Components/Table/MudTable.razor.cs

Line 658 in bc08c88

public Func<TableState, CancellationToken, Task<TableData<T>>>? ServerData { get; set; }

And I don't think having inconsistency is a good thing.

@mudbot

This comment was marked as off-topic.

Sign in to view
@mudbot mudbot bot changed the title MudDataGrid: Add CancellationToken support for ServerDataAsync MudDataGrid: Add CancellationToken support for ServerData Sep 26, 2025
This was referenced Feb 23, 2026
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Closed
Merged
Closed
Open
Closed
Closed
Closed
Merged
Closed
Closed
Merged
Closed
Open
Open
Closed
Open
Closed
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@versile2 versile2 versile2 approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking change This change will require consumer code updates enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add cancellation token support for paged server data in MudDataGrid

4 participants

@w3ori @ScarletKuro @versile2 @danielchalmers