Fix NavigationManager.Refresh ignoring forceReload parameter - #68185
Open
ManuelEnzo wants to merge 1 commit into
Open
Fix NavigationManager.Refresh ignoring forceReload parameter#68185ManuelEnzo wants to merge 1 commit into
ManuelEnzo wants to merge 1 commit into
Conversation
Refresh(forceReload) always called NavigateTo with forceLoad: true, regardless of the forceReload argument, so callers could never opt in to the enhanced-navigation merge behavior described in the method's own doc comment. Pass the forceReload parameter through to NavigateTo's forceLoad option instead of hardcoding true. Fixes dotnet#59854
Contributor
|
Thanks for your PR, @ManuelEnzo. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Author
|
@dotnet-policy-service agree |
1 similar comment
Author
|
@dotnet-policy-service agree |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
NavigationManager.Refresh(bool forceReload = false)always performed a fullforced page reload, regardless of the
forceReloadargument passed in:The
forceReloadparameter was accepted but never used —forceLoad: truewas hardcoded. This contradicts the method's own doc comment, which states
that when
forceReloadisfalsethe response may be merged with theexisting document to preserve client-side state (enhanced navigation),
falling back to a full reload only if necessary.
This affects any
NavigationManagerthat relies on the base class's defaultRefreshimplementation (e.g.HttpNavigationManager, used fornon-interactive/static server-side rendering) rather than overriding it.
RemoteNavigationManager(Blazor Server) andWebAssemblyNavigationManageralready forward
forceReloadcorrectly through their own overrides, so thisdoes not affect interactive Blazor Server/WebAssembly circuits.
Fix
Pass
forceReloadthrough toNavigateTo'sforceLoadoption instead ofhardcoding
true:Testing
Added three regression tests in
NavigationManagerTest.cscoveringRefresh(false),Refresh(true), and the default parameter value, assertingthe
ForceLoadoption passed toNavigateTomatches the argument.Ran the full
Microsoft.AspNetCore.Components.Testssuite(
eng\build.cmd -test -projects .\src\Components\Components\test\Microsoft.AspNetCore.Components.Tests.csproj):1278 tests, 1265 passed, 5 failed, 8 skipped. The 5 failures are pre-existing
on a clean checkout of
main(unrelatedEventCallbackFactoryBinderExtensionsTestdate-formatting tests and one
PersistentServicesRegistryTesttest, likelyculture-dependent) and are unaffected by this change — confirmed by running
the same suite before applying the fix.
Fixes #59854