Rollup PRs #4677 and and #4678 - #4680
Conversation
|
✅ All tests passed successfully Details on your Workflow / Core Tests page. |
Summary - Unit Tests Code CoverageSummary
CoverageMicrosoft.FluentUI.AspNetCore.Components - 61.2%
|
There was a problem hiding this comment.
Pull request overview
Rolls up fixes for MessageBar and DataGrid to address a NullReferenceException in MessageService.ShowMessageBarAsync and reduce initial-load lag when FluentDataGrid is virtualized.
Changes:
- MessageService: return snapshot copies from
AllMessages/MessagesToShow, and guardOnMessageItemsUpdatedAsyncinvocation to avoid NRE. - DataGrid: skip the 100ms debounce delay on the first virtualization provider call to improve initial render time.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/Core/Components/MessageBar/Services/MessageService.cs |
Makes message enumeration safer via snapshots and avoids NRE when async update handlers aren’t subscribed yet. |
src/Core/Components/DataGrid/FluentDataGrid.razor.cs |
Improves perceived initial-load performance by skipping the debounce on the first virtualized provider call. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Debounce the requests. This eliminates a lot of redundant queries at the cost of slight lag after interactions. | ||
| // TODO: Consider making this configurable, or smarter (e.g., doesn't delay on first call in a batch, then the amount | ||
| // of delay increases if you rapidly issue repeated requests, such as when scrolling a long way) | ||
| await Task.Delay(100); | ||
| // Skip the delay on the first call to avoid unnecessary lag on initial load. | ||
| if (_isFirstVirtualizeProviderCall) | ||
| { | ||
| _isFirstVirtualizeProviderCall = false; | ||
| } |
There was a problem hiding this comment.
_isFirstVirtualizeProviderCall is flipped to false before checking request.CancellationToken. If the first provider call gets cancelled (common during refresh/rapid parameter changes), the next call will be delayed even though no items were ever delivered, reintroducing the initial-load lag. Consider only clearing the flag after passing the cancellation check (or after returning a non-default result), or resetting it when the first call is cancelled.
| } | ||
| else | ||
| { | ||
| await Task.Delay(100); |
There was a problem hiding this comment.
The debounce Task.Delay(100) doesn’t observe request.CancellationToken, so cancelled virtualization requests still wait the full 100ms before returning. Use a cancellable delay (and handle OperationCanceledException) so cancelled requests complete promptly and don’t contribute to perceived lag under rapid scrolling/refresh.
| await Task.Delay(100); | |
| try | |
| { | |
| await Task.Delay(100, request.CancellationToken); | |
| } | |
| catch (System.OperationCanceledException) | |
| { | |
| return default; | |
| } |
|
|
||
| private GridItemsProvider<TGridItem>? _lastAssignedItemsProvider; | ||
| private CancellationTokenSource? _pendingDataLoadCancellationTokenSource; | ||
| private bool _isFirstVirtualizeProviderCall = true; | ||
|
|
There was a problem hiding this comment.
_isFirstVirtualizeProviderCall is never reset when the data source or virtualization mode changes. If ItemsProvider/Items is replaced or Virtualize is toggled off/on, the first provider call after that change will be debounced even though it’s effectively a new initial load. Consider resetting this flag when dataSourceHasChanged is true and/or when _lastVirtualizationMode changes to true.
| } | ||
| _navigationManager?.LocationChanged -= NavigationManager_LocationChanged; | ||
|
|
||
| RemoveMessageItems(section: null); |
There was a problem hiding this comment.
Dispose() calls RemoveMessageItems(section: null) without taking MessageLock. Since this class otherwise uses ReaderWriterLockSlim around MessageList, clearing it during disposal without the write lock can race with concurrent reads/writes and reintroduce enumeration/modification exceptions. Consider acquiring the write lock in Dispose() (or calling Clear()/a locking helper) before mutating MessageList.
| RemoveMessageItems(section: null); | |
| MessageLock.EnterWriteLock(); | |
| try | |
| { | |
| RemoveMessageItems(section: null); | |
| } | |
| finally | |
| { | |
| MessageLock.ExitWriteLock(); | |
| } |
…AspNetCore.Components.Icons (#6497) Updated [Microsoft.FluentUI.AspNetCore.Components](https://github.com/microsoft/fluentui-blazor) from 4.14.0 to 4.14.2. <details> <summary>Release notes</summary> _Sourced from [Microsoft.FluentUI.AspNetCore.Components's releases](https://github.com/microsoft/fluentui-blazor/releases)._ ## 4.14.2 ## What's Changed * [Select] Fix selected items color by @MarvinKlein1508 in microsoft/fluentui-blazor#4760 * [DesignTheme] Tolerate blocked localStorage during startup by @Copilot in microsoft/fluentui-blazor#4857 * [Chore] Prep for release and update icons by @vnbaaij in microsoft/fluentui-blazor#4869 **Full Changelog**: microsoft/fluentui-blazor@v4.14.1...v4.14.2 ## 4.14.1 ## What's Changed * Merge changes from Main by @vnbaaij in microsoft/fluentui-blazor#4516 * [AppBar] Make OnClick close popover by @vnbaaij in microsoft/fluentui-blazor#4518 * [Docs] Update keycode docs to reflect latest interface by @MarvinKlein1508 in microsoft/fluentui-blazor#4522 * Fix index.ts section causing "Illegal constructor" on older iOS devices by @GusPietrasanta-Polygon in microsoft/fluentui-blazor#4531 * [Docs] Add note about render mode and required interactivity by @MarvinKlein1508 in microsoft/fluentui-blazor#4540 * [Tabs] Call HandleOnTabChange when changing to a tab through the menu by @vnbaaij in microsoft/fluentui-blazor#4564 * [Popover] Allow tabbing inside open anchored regions, support keyboard accessible navigation for popover by @adamint in microsoft/fluentui-blazor#4561 * [DataGrid] Add OnSortChanged event callback by @vnbaaij in microsoft/fluentui-blazor#4573 * [Templates] Fix 404 in WebAssembly temp by @vnbaaij in microsoft/fluentui-blazor#4578 * Bump minimatch from 3.1.2 to 3.1.5 in /src/Core.Assets by @dependabot[bot] in microsoft/fluentui-blazor#4586 * fix: rename `dateselected` custom event to `fluentdateselected` to resolve .NET 11 exception by @Copilot in microsoft/fluentui-blazor#4605 * fix: rename custom events matching browserEventName to use fluent prefix by @Copilot in microsoft/fluentui-blazor#4609 * [General] Add accent-color in reboot.css by @wherewhere in microsoft/fluentui-blazor#4614 * [Dialog] Update docs by @vnbaaij in microsoft/fluentui-blazor#4625 * [DataGrid] Fix SelectedItems getting unselected when using pagination (and virtualization) by @vnbaaij in microsoft/fluentui-blazor#4623 * Bump flatted from 3.3.1 to 3.4.2 in /src/Core.Assets by @dependabot[bot] in microsoft/fluentui-blazor#4638 * [Docs] Fix incorrect Icon in search after Navigation by @sebbogle in microsoft/fluentui-blazor#4641 * [SplashScreen] Mark sync methods as Obsolete by @MarvinKlein1508 in microsoft/fluentui-blazor#4644 * Bump picomatch from 2.3.1 to 2.3.2 in /src/Core.Assets by @dependabot[bot] in microsoft/fluentui-blazor#4646 * [NavGroup] Fix disabled state for top-level `FluentNavGroup` by @MarvinKlein1508 in microsoft/fluentui-blazor#4647 * [NavGroup] Apply font opacity instead of color by @MarvinKlein1508 in microsoft/fluentui-blazor#4648 * Rollup PRs #4677 and and #4678 by @vnbaaij in microsoft/fluentui-blazor#4680 ## New Contributors * @GusPietrasanta-Polygon made their first contribution in microsoft/fluentui-blazor#4531 * @sebbogle made their first contribution in microsoft/fluentui-blazor#4641 **Full Changelog**: microsoft/fluentui-blazor@v4.14.0...v4.14.1 Commits viewable in [compare view](microsoft/fluentui-blazor@v4.14.0...v4.14.2). </details> Updated [Microsoft.FluentUI.AspNetCore.Components.Icons](https://github.com/microsoft/fluentui-blazor) from 4.14.2 to 4.14.3. <details> <summary>Release notes</summary> _Sourced from [Microsoft.FluentUI.AspNetCore.Components.Icons's releases](https://github.com/microsoft/fluentui-blazor/releases)._ ## 4.14.3 ## What's Changed * [Chore] Publish symbols from CI builds to internal symbol server by @tlmii in microsoft/fluentui-blazor#4910 * [Chore] Update dependencies in package.json by @dvoituron in microsoft/fluentui-blazor#4946 * [Docs] Update link to v5 docs by @MarvinKlein1508 in microsoft/fluentui-blazor#4927 * Update icons and prep for release by @vnbaaij in microsoft/fluentui-blazor#4958 **Full Changelog**: microsoft/fluentui-blazor@v4.14.2...v4.14.3 Commits viewable in [compare view](microsoft/fluentui-blazor@v4.14.2...v4.14.3). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Djuradj Kurepa <dkurepa@microsoft.com>
Original PRs were created by @JamesNK. Thanks!
Summary
Fix #4674
NullReferenceException in
ShowMessageBarAsyncShowMessageBarAsynccalledawait OnMessageItemsUpdatedAsync!.Invoke()without a null check. IfShowMessageBarAsyncis called beforeFluentMessageBarProvider.OnInitializedsubscribes to the event, this throws aNullReferenceException. The delegate is now captured in a local variable before invoking for thread safety:Thread safety of
AllMessagesandMessagesToShowBoth properties/methods acquired a read lock but returned the live
MessageListreference. The lock was released before the caller could enumerate, so concurrent writes (e.g. adding a message) could modify the collection during enumeration, causingInvalidOperationException.Both now return a snapshot copy via
.ToList()while still holding the read lock.Fix FluentDataGrid initial load performance
Fixes #4676
Problem
ProvideVirtualizedItemsAsyncunconditionally callsawait Task.Delay(100)before every request, including the very first one on page load. This introduces an artificial 100ms delay before grid rows are displayed, even when theItemsProvidercallback is synchronous.Solution
Added a
_isFirstVirtualizeProviderCallboolean field that skips the debounce delay on the first call. Subsequent calls (e.g., during scrolling) continue to be debounced as before.Also fixed a pre-existing IDE0048 build error (missing parentheses for clarity) on the same file.
Changes
FluentDataGrid.razor.cs: SkipTask.Delay(100)on the firstProvideVirtualizedItemsAsynccallFluentDataGrid.razor.cs: Add parentheses toEffectiveLoadingValueexpression to fix IDE0048