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

Components router refactoring. Fixes #10493 #10445 #12800

Merged
merged 36 commits into from
Aug 5, 2019

Conversation

SteveSandersonMS
Copy link
Member

This factors out some of the behaviors that were previously hardcoded into Router and makes them independent primitives. Benefits:

  • More flexibility to use different subsets of functionality, or to run things like authorization and routing in a different order if you want
  • Implicitly fixes Router displays NotFoundContent without any layout #10445, which would otherwise be quite difficult to fix while also keeping a nice API shape
  • Means we can later split off the entire concept of authentication/authorization into a separate optional assembly. It no longer needs to be in Microsoft.AspNetCore.Components.

Drawbacks:

  • There's now more markup in App.razor in the templates. But you could also argue that making the workings more explicit is actually beneficial.

The overall design here is to simplify <Router> so it no longer knows about auth or layouts, and instead is only responsible for matching the URL to a route and rendering either a Found block (passing the matched route data) or a NotFound block.

We then introduce three new built-in components:

  • <LayoutView>, which renders a specified content block inside a specified layout (and any ancestor layouts)
    • This will stay in M.A.Components
  • <RouteView>, which wraps <LayoutView> and renders whichever page component corresponds to some given route data. The layout it nominates is the one on that page, if any, or falls back on a specified DefaultLayout (or failing that, no layout).
    • This will stay in M.A.Components
  • <AuthorizeRouteView> which combines <AuthorizeView> with <RouteView>. It also takes a route data param, and tells the <AuthorizeView> to use the authorizedata on the matched page. The Authorized content is a <RouteView> that renders the page inside its layout. Optionally you can also specify NotAuthorized and Authorizing content.
    • It also implicitly adds a <CascadingAuthenticationState> to the output if you don't already have one
    • This will later move to the new M.A.Components.Authorization assembly

Then:

  • <PageDisplay> is now obsolete and has been removed. It is superseded by whichever one of the above three matches your actual scenario.
  • Project templates no longer need to include any explicit <CascadingAuthenticationState>, as that's provided by <AuthorizeRouteView> if you're using that. But we still have <CascadingAuthenticationState> as a public API, because that's relevant if you're using authorization without routing (common in native apps).
  • Project templates no longer need a Pages/_Imports.razor file because it's more natural to put a DefaultLayout on the <RouteView>. Technically then you're no longer forced to put the layout file in a separate folder from the pages, though I'm not proposing to change the folder structure in the templates.

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Aug 1, 2019
@SteveSandersonMS SteveSandersonMS added this to the 3.0.0-preview9 milestone Aug 1, 2019
[Parameter]
public Type DefaultLayout { get; set; }

public RouteView()
Copy link
Member

Choose a reason for hiding this comment

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

missing docs.

src/Components/Components/src/Auth/AuthorizeRouteView.cs Outdated Show resolved Hide resolved
src/Components/Components/src/Auth/AuthorizeRouteView.cs Outdated Show resolved Hide resolved
src/Components/Components/src/Auth/AuthorizeRouteView.cs Outdated Show resolved Hide resolved
<p>Sorry, there's nothing at this address.</p>
<LayoutView Layout="@typeof(MainLayout)">
<p>Sorry, there's nothing at this address.</p>
</LayoutView>
Copy link
Member

Choose a reason for hiding this comment

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

The no-auth case for this seems really simple and cool.

Something we could do (that would trade one kind of complexity for another) would be to add a <DefaultLayout> as a cascading value provider component.

I know you tend to prefer using Context="..." but I tend to not. Ultimately we could end up with something like:

<DefaultLayout Layout="typeof(MainLayout)">
<Router AppAssembly="@typeof(Program).Assembly">
    <Found>
        <RouteView RouteData="@context">
    </Found>
    <NotFound>
        <LayoutView>
            <p>Sorry, there's nothing at this address.</p>
        </LayoutView>
    </NotFound>
</Router>
</DefaultLayout>

In these case there's very little duplication and noise. These are nitpicks though, I'm happy with the result.

I'm not sure it's the right thing to use a cascasing value just to DRY-up the code, but it can make it less verbose.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I was definitely aware of this option but sided against it. I'm concerned with trying to make <LayoutView> generally useful in more scenarios than only the top-level App.razor file. It would be totally reasonable to use <LayoutView> at multiple levels, especially if we have nested routers in the future.

In cases where <LayoutView> is used outside App.razor, you really want to make it use the right layout for its own level, and not randomly inherit a choice from further up the tree (from some other file you can't necessarily find because there's no F12 way of getting to it). It seems greatly preferable for it to be a required explicit parameter.

Of course if someone wants, they can make their own <CascadedLayoutView> that consumed a cascading parameter and outputs a <LayoutView>. The feature is achievable in user code, but I don't fancy it as the built-in default, even if we need slightly more explicitness in App.razor.

src/Components/Components/src/LayoutView.cs Show resolved Hide resolved
src/Components/Components/src/ComponentRouteData.cs Outdated Show resolved Hide resolved
@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/router-refactoring branch from 04e7b61 to 2591f0f Compare August 2, 2019 15:49
Copy link
Member

@rynowak rynowak left a comment

Choose a reason for hiding this comment

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

Awesome!

@SteveSandersonMS SteveSandersonMS force-pushed the stevesa/router-refactoring branch from 2591f0f to 150376b Compare August 5, 2019 08:22
@SteveSandersonMS SteveSandersonMS merged commit 2ff6a5c into release/3.0 Aug 5, 2019
@ghost ghost deleted the stevesa/router-refactoring branch August 5, 2019 12:52
@SteveSandersonMS SteveSandersonMS added the tell-mode Indicates a PR which is being merged during tell-mode label Aug 6, 2019
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 tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants