Skip to content

NavigationLock not working with enhanced navigation in an interactive server page component #51436

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

Closed
1 task done
marinasundstrom opened this issue Oct 17, 2023 · 8 comments
Closed
1 task done
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Docs This issue tracks updating documentation

Comments

@marinasundstrom
Copy link

marinasundstrom commented Oct 17, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

NavigationLock doesn't activate when navigation away using link that triggers enhanced navigation in an interactive server page component. (For instance a link in the navbar)

Only works on page refresh.

Expected Behavior

The OnBeforeInternalNavigation should be triggered on enhanced navigation to another page.

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

8.0.100-rc.2.23502.2

Anything else?

No response

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Oct 17, 2023
@MackinnonBuck MackinnonBuck added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Oct 17, 2023
@ghost
Copy link

ghost commented Oct 17, 2023

Hi @marinasundstrom. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@MackinnonBuck
Copy link
Member

Thanks for reporting this, @marinasundstrom. To clarify, is the scenario that you have an interactive component on the page trying to block enhanced navigation? Or does your scenario only involve statically-rendered components?

@marinasundstrom
Copy link
Author

marinasundstrom commented Oct 17, 2023

@MackinnonBuck Ah. Forgot. It's when I'm on a page which is an interactive server component.

It doesn't block enhanced navigations. The only time the handler is being called is when doing a hard navigation, like a refresh.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Oct 17, 2023
@mkArtakMSFT mkArtakMSFT added Docs This issue tracks updating documentation and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Oct 18, 2023
@mkArtakMSFT mkArtakMSFT added this to the .NET 8: Documentation milestone Oct 18, 2023
@marinasundstrom marinasundstrom changed the title NavigationLock not working with enhanced navigation NavigationLock not working with enhanced navigation in an interactive server page component Oct 18, 2023
@MackinnonBuck
Copy link
Member

It doesn't block enhanced navigations.

This is expected and by-design (since there is no interactive router), but I can see how this might not be intuitive.

We did just make a change recently that makes enhanced navigations invoke the LocationChanged event, though. I wonder if it would be worth extending this to also invoke location changing handlers and allow for the cancellation of enhanced navigations. This would be theoretically possible, but opens up some other questions too. For example, if an interactive runtime initiates a programmatic navigation using NavigationManager.NavigateTo(), should location changing handlers be invoked in all other interactive runtimes? If so, this would require updating the implementation to perform multiple extra .NET<-->JS interop calls to coordinate the cancellation of the navigation.

The only time the handler is being called is when doing a hard navigation, like a refresh.

I'm surprised that the location changing handler runs at all in this case. Or are you referring to the ConfirmExternalNavigation parameter on NavigationLock? That actually uses the beforeunload event to prompt the user and doesn't rely on registering a location changing handler.

Here's the behavior of navigation events when using server-side routing today (assuming internal navigations if possible):

Navigation type Location changed Location changing Explanation/justification
NavigationManager.NavigateTo() (interactive) 1 Navigation is completely controlled by the interactive runtime
NavigationManager.Refresh() (interactive) Causes an enhanced navigation, so is consistent with other enhanced navigation triggers
Browser link click Causes an enhanced navigation, so is consistent with other enhanced navigation triggers
NavigationManager.NavigateTo() (SSR) Navigations during SSR are treated as redirects and don't go through the normal Blazor navigation code path
NavigationManager.Refresh(forceReload: true) (interactive) Consistent with clicking the "refresh" button on the browser

And here's the behavior when using client-side routing:

Navigation type Location changed Location changing Explanation/justification
NavigationManager.NavigateTo() (interactive) Navigation is completely controlled by the interactive runtime
NavigationManager.Refresh() (interactive) Enhanced navigation cannot be performed, so this is treated the same as NavigationManager.Refresh(forceReload: true)
Browser link click Navigation is handled by the interactive router
NavigationManager.NavigateTo() (SSR) Navigations during SSR are treated as redirects and don't go through the normal Blazor navigation code path
NavigationManager.Refresh(forceReload: true) (interactive) Consistent with clicking the "refresh" button on the browser

I think the things here that are candidates for reconsideration are:

  • Enhanced navigations invoke location changing handlers in all interactive runtimes
    • If we did this, we would need to think about whether NavigationManager.NavigateTo() should also allow other interactive runtimes to cancel the navigation
  • To be consistent with location changing handlers not being invoked for enhanced navigations, NavigationManager.NavigateTo() also does not invoke location changing handlers if there is no client-side router

I think if we were going to make one of the two above changes, we should make the second one. This would turn ✅1 into an ❌.
That way, the rules are super clear and predictable:

  • When interactive routing is used, location changing handlers always get invoked for internal navigations
  • When server-side routing is used, location changing handlers are never invoked

What you think? @marinasundstrom @danroth27 @javiercn @SteveSandersonMS

@SteveSandersonMS
Copy link
Member

@MackinnonBuck Your proposal sounds good to me. I agree it makes the rules simpler. However I've no idea if it would meet the bar for something we'd do before GA, or even if we could plan to bring this as an obscure breaking change into an 8.0.x servicing release based on the idea that we're denoting it as a bug. @mkArtakMSFT

@marinasundstrom
Copy link
Author

marinasundstrom commented Oct 19, 2023

@MackinnonBuck Yes, it was the ConfirmExternalNavigation that was executed. But the in app navigating didn't trigger. Which is an odd behavior for an interactive page, at least.

@MackinnonBuck
Copy link
Member

Thanks for the feedback!

We currently have this issue labeled as "docs". If we were to make the change I proposed in my last comment, the docs would be fairly straightforward (a slightly expanded version of the last two bullets in my last comment).

However, if we don't make this change, the docs will need to elaborate that "location changing" handlers sometimes get invoked when there's no client-side router. That could definitely seem a bit weird for people using <NavigationLock> with the OnBeforeInternalNavigation callback when server side routing is enabled.

I see us having two options here.

Option 1: Make the proposed change

This would technically a breaking change, even for previous versions of .NET. However, since the vast majority of existing Blazor apps enable navigation interception (client-side routing), I doubt many customers will be relying on the fact that location changing handlers get invoked for programmatic navigations when navigation interception is disabled.

There is also the concern that it's too late to make this change in .NET 8, and it's too breaking to include in a patch release (because at that point, there will be many customers using server-side routing, which would invalidate my previous point about customers not being affected by the change in behavior). However, as mentioned previously, this behavior is much easier to explain.

Option 2: Strategically document the existing behavior

Alternatively, we could keep the existing behavior and document it such that we could make enhancements without it being considered breaking. We could say something like this:

"When server-side routing is enabled, location changing handlers may or may not intercept internal navigations. It's important that app logic not rely on location changing handlers being called consistently. For example, the user might switch to another page before an interactive runtime becomes available."

This statement would still be true if we enable navigation cancellation for other types of navigations in the future, so it's probably good advice no matter how many different scenarios are supported.

If we want to be more precise, we could amend this with something like:

"In .NET 8, if server-side routing is enabled, location changing handlers will only get invoked for programmatic navigations initiated from an interactive runtime. In future .NET versions, more types of navigations (e.g., link clicks) may also invoke location changing handlers".

Thoughts? @mkArtakMSFT @SteveSandersonMS

@MackinnonBuck
Copy link
Member

@guardrex could we add some content to the "Blazor routing and navigation" docs about how the LocationChanged event and "location changing" handlers interact with enhanced navigation?

The table above shows the cases where each event gets called, but we probably don't need a table like that in the docs. We could summarize the behavior as:

"When an enhanced navigation occurs, LocationChanged event handlers registered in interactive runtimes (i.e., Blazor Server/WebAssembly) get invoked. However, location changing handlers may or may not intercept enhanced navigations, so it's important that app logic not rely on location changing handlers being invoked reliably. For example, the user might switch to another page before an interactive runtime becomes available. In .NET 8, if server-side routing and enhanced navigation are enabled, location changing handlers will only get invoked for programmatic navigations initiated from an interactive runtime. In future .NET versions, more types of navigations (e.g., link clicks) may also invoke location changing handlers".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Docs This issue tracks updating documentation
Projects
None yet
Development

No branches or pull requests

4 participants