Skip to content

Conversation

@ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Oct 14, 2025

Do not prevent rendering for router that has OnNavigateAsync when navigation already started

Description

In #24225 we blocked rendering when Router has a pending OnNavigateAsync task to prevent exceptions during lazy loading (#24211). The reason was that rendering was throwing when triggered before with lazy loading finished: #24211. However, this blocking prevented the prerendering quiescence detection mechanism from working correctly, as the OnNavigateAsync task was not being tracked.

This PR fixes the issue by informing the renderer that the OnNavigateAsync task is part of the quiescence state, allowing prerendering to wait for it to complete before finalizing the response.

Changes

  • Added internal RenderHandle.AddPendingTask to allow components to register tasks that should be tracked for quiescence
  • Modified Router.RunOnNavigateAsync to register the OnNavigateAsync task with the render handle using _renderHandle.AddPendingTask
  • Improved variable naming in the task unwrapping code for clarity (whenAnyTask and firstCompletedTask)
  • Added test for the issue: BrowserNavigationToNotExistingPath_ReExecutesTo404.
  • Added comprehensive test for Don't render route component if OnNavigateAsync task in-progress #24225 that validates rendering waits for OnNavigateAsync completion during lazy loading: BrowserNavigationToLazyLoadedRoute_WaitsForOnNavigateAsyncGuard.

How it works

During prerendering, the renderer tracks pending tasks to determine when quiescence has been reached. Then, it flushes the http response and the rendering is finished. By registering the OnNavigateAsync task with the renderer, we ensure that the end of rendering won't happen before OnNavigateAsync is finished.

Fixes #63933

@ilonatommy ilonatommy self-assigned this Oct 14, 2025
@ilonatommy ilonatommy added the area-blazor Includes: Blazor, Razor Components label Oct 14, 2025
@ilonatommy ilonatommy marked this pull request as ready for review October 15, 2025 10:08
@ilonatommy ilonatommy requested a review from a team as a code owner October 15, 2025 10:08
@ilonatommy ilonatommy requested review from Copilot and removed request for a team October 15, 2025 10:08
Copy link
Contributor

Copilot AI left a 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 an issue where Blazor 404 re-execution was not working properly when the Router component had an OnNavigateAsync handler. The fix allows rendering to proceed during 404 re-execution scenarios while still preventing it during client-side navigation when lazy loading is in progress.

Key changes:

  • Modified the Router component to differentiate between server-side rendering (SSR) scenarios and client-side navigation
  • Updated the navigation blocking logic to check if route data is available from SSR before blocking rendering
  • Added comprehensive test coverage for the 404 re-execution scenario with OnNavigateAsync

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/Components/Components/src/Routing/Router.cs Updated navigation logic to allow rendering during SSR/re-execution while maintaining blocking for client navigation
src/Components/test/testassets/Components.TestServer/RazorComponents/App.razor Added test infrastructure with configurable OnNavigateAsync handler and query parameter support
src/Components/test/E2ETest/ServerRenderingTests/NoInteractivityTest.cs Added end-to-end test to verify 404 re-execution works with OnNavigateAsync

ilonatommy and others added 3 commits October 15, 2025 12:09
…onents/App.razor

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tyTest.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ilonatommy
Copy link
Member Author

Failures of UserStoreTest are not connected, they happen across different PRs and should be quarantined.

@ilonatommy ilonatommy added the * NO MERGE * Do not merge this PR as long as this label is present. label Oct 21, 2025
@ilonatommy ilonatommy removed the * NO MERGE * Do not merge this PR as long as this label is present. label Oct 22, 2025
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Oct 30, 2025
@ilonatommy ilonatommy requested a review from oroztocil November 10, 2025 11:52
Copy link
Member

@oroztocil oroztocil left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@ilonatommy
Copy link
Member Author

@javiercn's review: use the information if we are interactive or not instead of putting a flag into route data.

@ilonatommy ilonatommy marked this pull request as draft November 14, 2025 14:29
Comment on lines 672 to 679
protected virtual void AddPendingTask(ComponentState? componentState, Task task)
{
// The pendingTasks collection is only used during prerendering to track quiescence,
// so will be null at other times.
AddPendingTask(task);
}

internal void AddPendingTask(Task task)
Copy link
Member

Choose a reason for hiding this comment

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

This is adding public API.

We can achieve the same effect in a slightly different way without having to add an additional overload nor additional API to RenderHandle.

The issue is that the router doesn't properly notify that it is still doing work, so we reach quiescence without the router being done (and hence we miss on later updates).

Quiescence is tracked whenever we call SetParametersAsync. Instead of doing things this way, we can keep whatever task we need to pass in a field, and on the call to SetParametersAsync we do something like

if (!_renderHandle.RendererInfo.IsInteractive && _taskWeAddedCallingAddPendingTask != null) 
{
  await _taskWeAddedCallingAddPendingTask;
}

That will have the effect of having that task included in the quiescence computation (as SetParametersAsync won't complete until that task completes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blazor Router NotFoundPage incompatible with OnNavigateAsync

3 participants