-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Blazor] Support NavigateTo when enhanced nav is disabled (#52267) #64542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Makes the NavigateTo API work even if enhanced nav is disabled via config. By default, Blazor apps have either enhanced nav or an interactive router . In these default cases, the `NavigateTo` API works correctly. However there's also an obscure way to disable both of these via config. It's niche, but it's supported, so the rest of the system should work with that. Unfortunately `NavigateTo` assumes that either enhanced nav or an interactive router will be enabled and doesn't account for the case when neither is. Fixes #51636 Without this fix, anyone who uses the `ssr: { disableDomPreservation: true }` config option will be unable to use the `NavigateTo` API, as it will do nothing. This behavior isn't desirable. - [ ] Yes - [x] No No because existing code can't use `ssr: { disableDomPreservation: true }` as the option didn't exist prior to .NET 8. Someone else might argue that it's a regression in the sense that, if you're migrating existing code to use newer .NET 8 patterns (and are using `disableDomPreservation` for some reason, even though you wouldn't normally), your existing uses of `NavigateTo` could stop working. That's not how we normally define "regression" but I'm trying to give the fullest explanation. - [ ] High - [ ] Medium - [x] Low The fix explicitly retains the old code path if you're coming from .NET 7 or earlier (i.e., if you are using `blazor.webassembly/server/webview.js`. The fixed code path is only applied in `blazor.web.js`, so it should not affect existing apps that are simply moving to the `net8.0` TFM without other code changes. - [x] Manual (required) - [x] Automated - [ ] Yes - [ ] No - [x] N/A
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the NavigateTo API to work correctly when enhanced navigation is disabled via the ssr: { disableDomPreservation: true } configuration option. Previously, NavigateTo would do nothing in this scenario because it assumed either enhanced navigation or an interactive router would be present.
Key Changes:
- Introduced a
PageLoadMechanismtype system in TypeScript to explicitly handle three navigation modes: 'clientside-router', 'serverside-enhanced', and 'serverside-fullpageload' - Added
isBlazorWebflag to distinguish blazor.web.js behavior from legacy blazor.server.js/blazor.webassembly.js for backward compatibility - Refactored navigation logic to perform full page loads when neither enhanced navigation nor interactive routing is available in blazor.web.js
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Components/Web.JS/src/Services/NavigationManager.ts | Refactored navigateToCore to use explicit PageLoadMechanism type system; added exhaustiveness checking; updated onPopState condition to use new mechanism check |
| src/Components/Web.JS/src/GlobalExports.ts | Added optional isBlazorWeb flag to IBlazor._internal interface to distinguish blazor.web.js from other variants |
| src/Components/Web.JS/src/Boot.Web.ts | Set isBlazorWeb flag to true during boot to enable new navigation behavior in blazor.web.js |
| src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/Interactivity/InteractiveNavigateTo.razor | Added new test page component to verify NavigateTo works with/without enhanced navigation and with/without force load |
| src/Components/test/testassets/Components.TestServer/RazorComponents/Shared/EnhancedNavLayout.razor | Wrapped @Body in <main> tag for better semantic structure |
| src/Components/test/E2ETest/ServerRenderingTests/InteractivityTest.cs | Added parameterized test covering all combinations of enhanced navigation and forceLoad settings |
| src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTestUtil.cs | Added IsElementStale utility method to check if DOM elements have been replaced |
| src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTest.cs | Added private wrapper method delegating to EnhancedNavigationTestUtil.IsElementStale |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTest.cs
Outdated
Show resolved
Hide resolved
|
@copilot open a new pull request to apply changes based on the comments in this thread |
src/Components/test/E2ETest/ServerRenderingTests/EnhancedNavigationTestUtil.cs
Outdated
Show resolved
Hide resolved
|
@ilonatommy I've opened a new pull request, #64550, to work on those changes. Once the pull request is ready, I'll request review from you. |
…Stale (#64549) * Initial plan * Remove redundant IsElementStale method and use WebDriverExtensions.IsStale Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
Forward port of #52267 which never made it into main.
Makes the NavigateTo API work even if enhanced nav is disabled via config.
By default, Blazor apps have either enhanced nav or an interactive router . In these default cases, the
NavigateToAPI works correctly.However there's also an obscure way to disable both of these via config. It's niche, but it's supported, so the rest of the system should work with that. Unfortunately
NavigateToassumes that either enhanced nav or an interactive router will be enabled and doesn't account for the case when neither is.Fixes #51636
Without this fix, anyone who uses the
ssr: { disableDomPreservation: true }config option will be unable to use theNavigateToAPI, as it will do nothing. This behavior isn't desirable.No because existing code can't use
ssr: { disableDomPreservation: true }as the option didn't exist prior to .NET 8.Someone else might argue that it's a regression in the sense that, if you're migrating existing code to use newer .NET 8 patterns (and are using
disableDomPreservationfor some reason, even though you wouldn't normally), your existing uses ofNavigateTocould stop working. That's not how we normally define "regression" but I'm trying to give the fullest explanation.The fix explicitly retains the old code path if you're coming from .NET 7 or earlier (i.e., if you are using
blazor.webassembly/server/webview.js. The fixed code path is only applied inblazor.web.js, so it should not affect existing apps that are simply moving to thenet8.0TFM without other code changes.Manual (required)
Automated
Yes
No
N/A
{PR title}
Summary of the changes (Less than 80 chars)
Description
{Detail}
Fixes #{bug number} (in this specific format)