Skip to content
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

HttpNavigationManager no longer uses NavigationException #61306

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Apr 3, 2025

Description

  • Guard throwing NavigationException with AppContext switch. Naming updates are welcome.
  • Keep catches it in the places where they are, without AppContext.
  • Add an event listener that triggers httpContext redirection.
  • Test for no interactivity SSR (HttpNavigationManager) and interactivity SSR (RemoteNavigationManager) scenarios.
  • Removal of "routing/not-found-ssr" etc, they don't exist and should not be on Routing test cases page.

Fixes #59451

@ilonatommy ilonatommy added area-blazor Includes: Blazor, Razor Components feature-blazor-navigation labels Apr 3, 2025
@ilonatommy ilonatommy added this to the 10.0-preview4 milestone Apr 3, 2025
@ilonatommy ilonatommy requested a review from javiercn April 3, 2025 13:34
@ilonatommy ilonatommy self-assigned this Apr 3, 2025
@ilonatommy ilonatommy requested a review from a team as a code owner April 3, 2025 13:34
@ilonatommy ilonatommy requested a review from a team as a code owner April 7, 2025 12:04
…onents/Pages/Routing/SSRRedirectionStreaming.razor

Co-authored-by: Javier Calvarro Nelson <jacalvar@microsoft.com>
@ilonatommy
Copy link
Member Author

ilonatommy commented Apr 11, 2025

Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests.PrerenderingTest.CanRedirectDuringPrerendering failures are connected with the fact that we are not stopping the renderer effectively:
if the stopping occurred in prerendering, it will be valid for prerendering only and in the main rendering, _rendererIsStopped will already be false. We have to find an effective way to communicate the pause between the calls of BeginRenderingComponent.
wrong diagnosis.
We are not stopping effectively because we planned for the whole ongoing batch to finish rendering and the pause was planned to be effective only for future UI updates. We are in the middle of processing the render queue in ProcessRenderQueue

var nextToRender = _batchBuilder.ComponentRenderQueue.Dequeue();

while new elements are still getting enqueued into the _batchBuilder.ComponentRenderQueue in
_batchBuilder.ComponentRenderQueue.Enqueue(

We're operating on the shared object in the loop, so we cannot stop PrerenderedRedirection render. PrerenderedRedirection Deque is triggered after OnInitializedAsync but started in a process triggered before OnInitializedAsync, with _rendererIsStopped=false.

My proposal:
this test has to be changed, in the current situation we are not able to maintain the expectations of not finishing the ongoing render batch. This does not affect the correctness of navigation process.

An alternative (I need your opinion on this, @javiercn):
prevent editing _batchBuilder.ComponentRenderQueue when _isBatchInProgress=true. We can gather the items in a separate, "waiting" queue and move them to the main one in the end of ProcessPendingRender, when we are clearing _isBatchInProgress = false;. This way we will make sure that stopping was done properly.

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 feature-blazor-navigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reconcile NavigationManager behavior to no longer use NavigationException
2 participants