-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Components router refactoring. Fixes #10493 #10445 #12800
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
Merged
Merged
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
d0ea8bf
Add LayoutView component
SteveSandersonMS edba2f8
Add RouteView component
SteveSandersonMS 29687d8
Update Router to new factoring
SteveSandersonMS 5bc856d
Mark route parameter dictionaries as readonly
SteveSandersonMS f686840
Update ComponentsApp to use new APIs
SteveSandersonMS 4c896a5
Update client-side StandaloneApp to use new APIs
SteveSandersonMS 5a6a896
Renames for clarity
SteveSandersonMS d095e99
Convert router E2E test to use new APIs
SteveSandersonMS 59de8be
Simplistic implementation of AuthorizeViewCore which I don't like. Up…
SteveSandersonMS ac012eb
Improve AuthorizeRouteView by subclassing RouteView
SteveSandersonMS 2310a25
Inline AuthorizeRouteViewCore
SteveSandersonMS aa29a6f
Cache some delegates
SteveSandersonMS 571f963
Merge the behavior of CascadingAuthenticationState into AuthorizeRout…
SteveSandersonMS cddcc71
Revert "Merge the behavior of CascadingAuthenticationState into Autho…
SteveSandersonMS 9539832
Some clarifications/simplifications
SteveSandersonMS dfb75ce
Begin unit tests for AuthorizeRouteView
SteveSandersonMS 4a04b3c
Finish unit tests and implementation for AuthorizeRouteView
SteveSandersonMS eb9c4e5
Tidy
SteveSandersonMS 4b41722
Typo
SteveSandersonMS 40acef8
Stop using DefaultLayout in router E2E tests, as it interferes with p…
SteveSandersonMS 251bd42
Delete PageDisplay component
SteveSandersonMS cfcb67d
Move context declarations for clarity
SteveSandersonMS de640ae
Fix XML doc
SteveSandersonMS ac349ba
Update E2EPerf app
SteveSandersonMS 184e67f
Update project templates
SteveSandersonMS 0f8a1df
Update src/Components/Components/src/LayoutView.cs
SteveSandersonMS f9a893f
CR: Check the SetParametersAsync tasks complete successfully
SteveSandersonMS 0454c6a
CR: Rename ComponentRouteData->RouteData
SteveSandersonMS dca1b78
CR: Rename PageComponentType->PageType
SteveSandersonMS 0236e1e
CR: Rename PageParameters->RouteValues
SteveSandersonMS 262f535
CR: Quotes
SteveSandersonMS cfcf62c
CR: Tweak delegate caching
SteveSandersonMS 4943442
CR: IsAssignableFrom
SteveSandersonMS 5ba0521
Update ref source
SteveSandersonMS 150376b
Update MVC functional test
SteveSandersonMS b805da3
Update template test baselines
SteveSandersonMS File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
9 changes: 7 additions & 2 deletions
9
src/Components/Blazor/Templates/src/content/BlazorWasm-CSharp/Client/App.razor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,10 @@ | ||
| <Router AppAssembly="typeof(Program).Assembly"> | ||
| <Router AppAssembly="@typeof(Program).Assembly"> | ||
| <Found Context="routeData"> | ||
| <RouteView RouteData="@routeData" DefaultLayout="@typeof(MainLayout)" /> | ||
| </Found> | ||
| <NotFound> | ||
| <p>Sorry, there's nothing at this address.</p> | ||
| <LayoutView Layout="@typeof(MainLayout)"> | ||
| <p>Sorry, there's nothing at this address.</p> | ||
| </LayoutView> | ||
| </NotFound> | ||
| </Router> | ||
1 change: 0 additions & 1 deletion
1
src/Components/Blazor/Templates/src/content/BlazorWasm-CSharp/Client/Pages/_Imports.razor
This file was deleted.
Oops, something went wrong.
9 changes: 8 additions & 1 deletion
9
src/Components/Blazor/testassets/Microsoft.AspNetCore.Blazor.E2EPerformance/App.razor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,8 @@ | ||
| <Router AppAssembly=typeof(Program).Assembly /> | ||
| <Router AppAssembly=typeof(Program).Assembly> | ||
| <Found Context="routeData"> | ||
| <RouteView RouteData="@routeData" /> | ||
| </Found> | ||
| <NotFound> | ||
| Sorry, there's nothing here. | ||
| </NotFound> | ||
| </Router> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,11 @@ | ||
| <!-- | ||
| Configuring this stuff here is temporary. Later we'll move the app config | ||
| into Program.cs, and it won't be necessary to specify AppAssembly. | ||
| --> | ||
| <Router AppAssembly=typeof(StandaloneApp.Program).Assembly /> | ||
| <Router AppAssembly=typeof(StandaloneApp.Program).Assembly> | ||
| <Found Context="routeData"> | ||
| <RouteView RouteData="@routeData" DefaultLayout="@typeof(MainLayout)" /> | ||
| </Found> | ||
| <NotFound> | ||
| <LayoutView Layout="@typeof(MainLayout)"> | ||
| <h2>Not found</h2> | ||
| Sorry, there's nothing at this address. | ||
| </LayoutView> | ||
| </NotFound> | ||
| </Router> |
1 change: 0 additions & 1 deletion
1
src/Components/Blazor/testassets/StandaloneApp/Pages/_Imports.razor
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
src/Components/Components/src/Auth/AuthorizeRouteView.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System.Threading.Tasks; | ||
| using Microsoft.AspNetCore.Authorization; | ||
| using Microsoft.AspNetCore.Components.Auth; | ||
| using Microsoft.AspNetCore.Components.RenderTree; | ||
|
|
||
| namespace Microsoft.AspNetCore.Components | ||
| { | ||
| /// <summary> | ||
| /// Combines the behaviors of <see cref="AuthorizeView"/> and <see cref="RouteView"/>, | ||
| /// so that it displays the page matching the specified route but only if the user | ||
| /// is authorized to see it. | ||
| /// | ||
| /// Additionally, this component supplies a cascading parameter of type <see cref="Task{AuthenticationState}"/>, | ||
| /// which makes the user's current authentication state available to descendants. | ||
| /// </summary> | ||
| public sealed class AuthorizeRouteView : RouteView | ||
SteveSandersonMS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| // We expect applications to supply their own authorizing/not-authorized content, but | ||
| // it's better to have defaults than to make the parameters mandatory because in some | ||
| // cases they will never be used (e.g., "authorizing" in out-of-box server-side Blazor) | ||
| private static readonly RenderFragment<AuthenticationState> _defaultNotAuthorizedContent | ||
| = state => builder => builder.AddContent(0, "Not authorized"); | ||
| private static readonly RenderFragment _defaultAuthorizingContent | ||
| = builder => builder.AddContent(0, "Authorizing..."); | ||
|
|
||
| private readonly RenderFragment _renderAuthorizeRouteViewCoreDelegate; | ||
| private readonly RenderFragment<AuthenticationState> _renderAuthorizedDelegate; | ||
| private readonly RenderFragment<AuthenticationState> _renderNotAuthorizedDelegate; | ||
| private readonly RenderFragment _renderAuthorizingDelegate; | ||
|
|
||
| public AuthorizeRouteView() | ||
| { | ||
| // Cache the rendering delegates so that we only construct new closure instances | ||
| // when they are actually used (e.g., we never prepare a RenderFragment bound to | ||
| // the NotAuthorized content except when you are displaying that particular state) | ||
| RenderFragment renderBaseRouteViewDelegate = builder => base.Render(builder); | ||
| _renderAuthorizedDelegate = authenticateState => renderBaseRouteViewDelegate; | ||
| _renderNotAuthorizedDelegate = authenticationState => builder => RenderNotAuthorizedInDefaultLayout(builder, authenticationState); | ||
| _renderAuthorizingDelegate = RenderAuthorizingInDefaultLayout; | ||
| _renderAuthorizeRouteViewCoreDelegate = RenderAuthorizeRouteViewCore; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// The content that will be displayed if the user is not authorized. | ||
| /// </summary> | ||
| [Parameter] | ||
| public RenderFragment<AuthenticationState> NotAuthorized { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// The content that will be displayed while asynchronous authorization is in progress. | ||
| /// </summary> | ||
| [Parameter] | ||
| public RenderFragment Authorizing { get; set; } | ||
|
|
||
| [CascadingParameter] | ||
| private Task<AuthenticationState> ExistingCascadedAuthenticationState { get; set; } | ||
|
|
||
| /// <inheritdoc /> | ||
| protected override void Render(RenderTreeBuilder builder) | ||
| { | ||
| if (ExistingCascadedAuthenticationState != null) | ||
SteveSandersonMS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| // If this component is already wrapped in a <CascadingAuthenticationState> (or another | ||
| // compatible provider), then don't interfere with the cascaded authentication state. | ||
| _renderAuthorizeRouteViewCoreDelegate(builder); | ||
| } | ||
| else | ||
| { | ||
| // Otherwise, implicitly wrap the output in a <CascadingAuthenticationState> | ||
| builder.OpenComponent<CascadingAuthenticationState>(0); | ||
| builder.AddAttribute(1, nameof(CascadingAuthenticationState.ChildContent), _renderAuthorizeRouteViewCoreDelegate); | ||
| builder.CloseComponent(); | ||
| } | ||
| } | ||
|
|
||
| private void RenderAuthorizeRouteViewCore(RenderTreeBuilder builder) | ||
| { | ||
| builder.OpenComponent<AuthorizeRouteViewCore>(0); | ||
| builder.AddAttribute(1, nameof(AuthorizeRouteViewCore.RouteData), RouteData); | ||
| builder.AddAttribute(2, nameof(AuthorizeRouteViewCore.Authorized), _renderAuthorizedDelegate); | ||
| builder.AddAttribute(3, nameof(AuthorizeRouteViewCore.Authorizing), _renderAuthorizingDelegate); | ||
| builder.AddAttribute(4, nameof(AuthorizeRouteViewCore.NotAuthorized), _renderNotAuthorizedDelegate); | ||
| builder.CloseComponent(); | ||
| } | ||
|
|
||
| private void RenderContentInDefaultLayout(RenderTreeBuilder builder, RenderFragment content) | ||
| { | ||
| builder.OpenComponent<LayoutView>(0); | ||
| builder.AddAttribute(1, nameof(LayoutView.Layout), DefaultLayout); | ||
| builder.AddAttribute(2, nameof(LayoutView.ChildContent), content); | ||
| builder.CloseComponent(); | ||
| } | ||
|
|
||
| private void RenderNotAuthorizedInDefaultLayout(RenderTreeBuilder builder, AuthenticationState authenticationState) | ||
| { | ||
| var content = NotAuthorized ?? _defaultNotAuthorizedContent; | ||
| RenderContentInDefaultLayout(builder, content(authenticationState)); | ||
| } | ||
|
|
||
| private void RenderAuthorizingInDefaultLayout(RenderTreeBuilder builder) | ||
| { | ||
| var content = Authorizing ?? _defaultAuthorizingContent; | ||
| RenderContentInDefaultLayout(builder, content); | ||
| } | ||
|
|
||
| private class AuthorizeRouteViewCore : AuthorizeViewCore | ||
| { | ||
| [Parameter] | ||
| public RouteData RouteData { get; set; } | ||
|
|
||
| protected override IAuthorizeData[] GetAuthorizeData() | ||
| => AttributeAuthorizeDataCache.GetAuthorizeDataForType(RouteData.PageType); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System; | ||
| using System.Reflection; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace Microsoft.AspNetCore.Components | ||
| { | ||
| /// <summary> | ||
| /// Displays the specified content inside the specified layout and any further | ||
| /// nested layouts. | ||
| /// </summary> | ||
| public class LayoutView : IComponent | ||
rynowak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| private static readonly RenderFragment EmptyRenderFragment = builder => { }; | ||
|
|
||
| private RenderHandle _renderHandle; | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the content to display. | ||
| /// </summary> | ||
| [Parameter] | ||
| public RenderFragment ChildContent { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the type of the layout in which to display the content. | ||
| /// The type must implement <see cref="IComponent"/> and accept a parameter named <see cref="LayoutComponentBase.Body"/>. | ||
| /// </summary> | ||
| [Parameter] | ||
| public Type Layout { get; set; } | ||
|
|
||
| /// <inheritdoc /> | ||
| public void Attach(RenderHandle renderHandle) | ||
rynowak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| _renderHandle = renderHandle; | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public Task SetParametersAsync(ParameterView parameters) | ||
| { | ||
| parameters.SetParameterProperties(this); | ||
| Render(); | ||
| return Task.CompletedTask; | ||
| } | ||
|
|
||
| private void Render() | ||
| { | ||
| // In the middle goes the supplied content | ||
| var fragment = ChildContent ?? EmptyRenderFragment; | ||
|
|
||
| // Then repeatedly wrap that in each layer of nested layout until we get | ||
| // to a layout that has no parent | ||
| var layoutType = Layout; | ||
| while (layoutType != null) | ||
| { | ||
| fragment = WrapInLayout(layoutType, fragment); | ||
| layoutType = GetParentLayoutType(layoutType); | ||
| } | ||
|
|
||
| _renderHandle.Render(fragment); | ||
| } | ||
|
|
||
| private static RenderFragment WrapInLayout(Type layoutType, RenderFragment bodyParam) | ||
| { | ||
| return builder => | ||
| { | ||
| builder.OpenComponent(0, layoutType); | ||
| builder.AddAttribute(1, LayoutComponentBase.BodyPropertyName, bodyParam); | ||
| builder.CloseComponent(); | ||
| }; | ||
| } | ||
|
|
||
| private static Type GetParentLayoutType(Type type) | ||
| => type.GetCustomAttribute<LayoutAttribute>()?.LayoutType; | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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: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.
There was a problem hiding this comment.
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-levelApp.razorfile. 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 outsideApp.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 inApp.razor.