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

Per-page opt out from interactive routing #55157

Merged

Conversation

SteveSandersonMS
Copy link
Member

@SteveSandersonMS SteveSandersonMS commented Apr 16, 2024

This allows page components to exclude themselves from interactive routing.

This is not a feature about rendermodes, but it's all we need to add into the framework for #51046, since the rest is achievable in a few lines of application code (as validated in the E2E tests in this PR).

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Apr 16, 2024
@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/static-pages-amid-global-interactivity-v2 branch from 6a4a350 to 7f78bc7 Compare April 16, 2024 09:40
@SteveSandersonMS SteveSandersonMS changed the title Static pages amid global interactivity (v2) Per-page opt out from interactive routing Apr 16, 2024
@SteveSandersonMS SteveSandersonMS marked this pull request as ready for review April 16, 2024 13:10
@SteveSandersonMS SteveSandersonMS requested a review from a team as a code owner April 16, 2024 13:10
if (typeof(IComponent).IsAssignableFrom(type) && type.IsDefined(typeof(RouteAttribute)))
if (typeof(IComponent).IsAssignableFrom(type)
&& type.IsDefined(typeof(RouteAttribute))
&& type.GetCustomAttribute<AllowInteractiveRoutingAttribute>() is not { Allow: false })
Copy link
Member

@halter73 halter73 Apr 16, 2024

Choose a reason for hiding this comment

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

This could end up giving different results than the HttpContext.AllowsInteractiveRouting() extension method if someone does something like MapRazorComponents<App>()...WithMetadata(new AllowInteractiveRoutingAttribute(false)). This could lead people to think things are working at first only to realize it's broken later when they attempt an interactive navigation. Is there a way we can look at the final metadata from the potentially routable component's Endpoint?

I know this is a problem for other metadata like [AllowAnonymous]. Blazor not being able to "see" all the endpoint metadata can be helpful for working around the issue many people are facing in .NET 8 where the authorization middleware rejects requests before Blazor's AuthorizeRouteView can handle it. That's because the authorization middleware will "see" the [AllowAnonymous] metadata added by MapRazorComponents<App>()...AllowAnonymous() and let the request through while AuthorizeRouteView cannot. However, I think we should fix that. And I certainly don't think we should be adding new attributes that can be interpreted one way by Blazor's core components and another way by code checking the Endpoint.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a way we can look at the final metadata from the potentially routable component's Endpoint?

No, this code runs client-side (e.g., on WebAssembly), and can't see the server-side endpoint table.

If we later create some bigger mechanism for supplying extra info from the server-side endpoint table to the client, then we could use that here. But a new mechanism like that would be well out of scope for this issue. Until something like that exists, it's up to the developer not to cause themselves any extra problems here by thinking they can change the server-side metadata and the client would somehow know about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we wanted to absolutely ensure consistency in this case, the only reasonable option I can think of is for the server not to read the attribute from endpoint metadata at all, but instead to read it from the component type directly, so you can't customize it using WithMetadata.

Copy link
Member

Choose a reason for hiding this comment

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

If we wanted to absolutely ensure consistency in this case, the only reasonable option I can think of is for the server not to read the attribute from endpoint metadata at all, but instead to read it from the component type directly, so you can't customize it using WithMetadata.

I think that's how it should work for now. I like the cleanliness of just reading the metadata directly on the server side, but not if it's inconsistent with what the route table sees. If we figure out how to easily read the final metadata in both places later, we can make that change then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fine with me. It's how I implemented it originally 😄

@michelkommers
Copy link

michelkommers commented Apr 18, 2024

@SteveSandersonMS I left a comment (last one) in #51046 giving more feedback from a blazor dev pov, but i saw a few discussions that got me wandering if its going to be the way people expect, so im going to ask here directly to clarify.

With the new atrribute [StaticPage] and using a global render mode InteractiveWebassembly, will the wasm routes need to hit the server after the wasm is downloaded? This is the most annoying issue that devs are currently trying to solve, because this way we can have areas with SSR dumb pages and areas with smart Wasm pages, but this is beneficial compared to the current options only if the wasm routes don't need to hit the server after the wasm is downloaded. The exceptions would be the ones marked with [StaticPage], as in, a route that dont exist in wasm, will hit the server. Can you confirm this? and also, when can we expect this feature? Waiting for .net 9 for something like this is a long wait.

@javiercn
Copy link
Member

@michelkommers You'll get to choose the render mode you use based on the attribute within your app (so its clear what mode and why its rendering) on a per endpoint (page) basis.

The logic is something like:

<Routes @rendermode="PageRenderMode">

public IRenderMode PageRenderMode => HttpContext.AcceptsInteractiveRouting() ? InteractiveWebAssembly : null;

Any page that has [ExcludeFromInteractiveRouting] will be excluded from the interactive router and will always render SSR via an enhanced nav.

If you start a webassembly session and navigate between pages without [ExcludeFromInteractiveRouting] the navigations will be client-side navigations.

If you navigate to a page with [ExcludeFromInteractiveRouting] we will perform an enhanced nav.

If you navigate within pages with [ExcludeFromInteractiveRouting] we will perform an enhanced nav.

@michelkommers
Copy link

Thanks @javiercn for taking the time to reply, this will work perfectly, this is exactly the behavior that people are waiting to be able to do. Thanks again and now I'm anxiously waiting for its release :)

Keep up the good work

@SteveSandersonMS
Copy link
Member Author

SteveSandersonMS commented Apr 19, 2024

If you navigate to a page with [ExcludeFromInteractiveRouting] we will perform an enhanced nav.

Just a clarification on that, navigations from interactive to static will actually be full page loads, not enhanced nav. This is because we don't support detaching an interactive router once it's been attached.

On the other hand, navigations from static to interactive will be enhanced navs, because we do support attaching interactive routers at any time.

If you navigate within pages with [ExcludeFromInteractiveRouting] we will perform an enhanced nav.

This is true. I'm just confirming that in case my comment above makes it seem unclear!

Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Looks great!

@SteveSandersonMS SteveSandersonMS enabled auto-merge (squash) April 19, 2024 13:54
@SteveSandersonMS SteveSandersonMS merged commit 23afddf into main Apr 19, 2024
26 checks passed
@SteveSandersonMS SteveSandersonMS deleted the stevesa/static-pages-amid-global-interactivity-v2 branch April 19, 2024 15:54
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview4 milestone Apr 19, 2024
@michelkommers
Copy link

@guardrex I think would be good to add this feature to the official documentation and add an example of use of this feature under 'advanced scenarios'.

@guardrex
Copy link
Contributor

@michelkommers ... No worries. All new features are tracked for docs 😉.

@SteveSandersonMS
Copy link
Member Author

@guardrex @michelkommers As a starting point, I added some content at dotnet/AspNetCore.Docs#32361 (comment). Please feel free to use or not use this!

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

Successfully merging this pull request may close these issues.

5 participants