Skip to content

Static pages amid global interactivity #54948

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
25 changes: 9 additions & 16 deletions src/Components/Components/src/ComponentFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,16 @@ private static ComponentTypeInfoCacheEntry GetComponentTypeInfo([DynamicallyAcce
public IComponent InstantiateComponent(IServiceProvider serviceProvider, [DynamicallyAccessedMembers(Component)] Type componentType, IComponentRenderMode? callerSpecifiedRenderMode, int? parentComponentId)
{
var (componentTypeRenderMode, propertyInjector) = GetComponentTypeInfo(componentType);
IComponent component;

if (componentTypeRenderMode is null && callerSpecifiedRenderMode is null)
{
// Typical case where no rendermode is specified in either location. We don't call ResolveComponentForRenderMode in this case.
component = _componentActivator.CreateInstance(componentType);
}
else
{
// At least one rendermode is specified. We require that it's exactly one, and use ResolveComponentForRenderMode with it.
var effectiveRenderMode = callerSpecifiedRenderMode is null
? componentTypeRenderMode!
: componentTypeRenderMode is null
? callerSpecifiedRenderMode
: throw new InvalidOperationException($"The component type '{componentType}' has a fixed rendermode of '{componentTypeRenderMode}', so it is not valid to specify any rendermode when using this component.");
component = _renderer.ResolveComponentForRenderMode(componentType, parentComponentId, _componentActivator, effectiveRenderMode);
}
// In the typical case where no rendermode is specified in either location, we don't even call
// ComputeEffectiveRenderMode and hence don't call ResolveComponentForRenderMode either.
var effectiveRenderMode = componentTypeRenderMode is null && callerSpecifiedRenderMode is null
? null
: _renderer.ResolveEffectiveRenderMode(componentType, parentComponentId, componentTypeRenderMode, callerSpecifiedRenderMode);

var component = effectiveRenderMode is null
? _componentActivator.CreateInstance(componentType)
: _renderer.ResolveComponentForRenderMode(componentType, parentComponentId, _componentActivator, effectiveRenderMode);
Copy link
Member Author

Choose a reason for hiding this comment

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

Conceptually the change here is factoring out the "resolve rendermode" logic into a new virtual method on the renderer, so that we can implement the new rule about static pages suppressing callsite rendermodes. Doing so happens to simplify ComponentFactory quite a bit.


if (component is null)
{
Expand Down
3 changes: 3 additions & 0 deletions src/Components/Components/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
#nullable enable
Microsoft.AspNetCore.Components.RouteAttribute.Static.get -> bool
Microsoft.AspNetCore.Components.RouteAttribute.Static.set -> void
virtual Microsoft.AspNetCore.Components.RenderTree.Renderer.ResolveEffectiveRenderMode(System.Type! componentType, int? parentComponentId, Microsoft.AspNetCore.Components.IComponentRenderMode? componentTypeRenderMode, Microsoft.AspNetCore.Components.IComponentRenderMode? callerSpecifiedRenderMode) -> Microsoft.AspNetCore.Components.IComponentRenderMode?
21 changes: 21 additions & 0 deletions src/Components/Components/src/RenderTree/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,27 @@ void NotifyExceptions(List<Exception> exceptions)
}
}

/// <summary>
/// Determines the rendermode to use for a component when one is specified either at the call site or on the component type.
/// Renderer subclasses may override this to impose additional rules about rendermode selection.
/// </summary>
/// <param name="componentType">The type of component that was requested.</param>
/// <param name="parentComponentId">The parent component ID, or null if it is a root component.</param>
/// <param name="componentTypeRenderMode">The <see cref="IComponentRenderMode"/> declared on <paramref name="componentType"/>.</param>
/// <param name="callerSpecifiedRenderMode">The <see cref="IComponentRenderMode"/> declared at the call site (for example, by the parent component).</param>
/// <returns></returns>
/// <exception cref="InvalidOperationException"></exception>
protected internal virtual IComponentRenderMode? ResolveEffectiveRenderMode(Type componentType, int? parentComponentId, IComponentRenderMode? componentTypeRenderMode, IComponentRenderMode? callerSpecifiedRenderMode)
{
// To avoid confusion, we require that you don't specify the rendermode in two places. This means that
// the resolution is trivial - just use the nonnull one (if any).
return callerSpecifiedRenderMode is null
? componentTypeRenderMode!
: componentTypeRenderMode is null
? callerSpecifiedRenderMode
: throw new InvalidOperationException($"The component type '{componentType}' has a fixed rendermode of '{componentTypeRenderMode}', so it is not valid to specify any rendermode when using this component.");
}

/// <summary>
/// Determines how to handle an <see cref="IComponentRenderMode"/> when obtaining a component instance.
/// This is only called when a render mode is specified either at the call site or on the component type.
Expand Down
6 changes: 6 additions & 0 deletions src/Components/Components/src/RouteAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,10 @@ public RouteAttribute(string template)
/// Gets the route template.
/// </summary>
public string Template { get; }

/// <summary>
/// Gets or sets a flag to indicate whether the page should be rendered statically.
/// The effect of this flag is to suppress any <code>@rendermode</code> directives in the root component.
/// </summary>
public bool Static { get; set; }
}
8 changes: 5 additions & 3 deletions src/Components/Components/src/Routing/RouteTable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Components.Routing;
internal sealed class RouteTable(TreeRouter treeRouter)
{
private readonly TreeRouter _router = treeRouter;
private static readonly ConcurrentDictionary<(Type, string), InboundRouteEntry> _routeEntryCache = new();
private static readonly ConcurrentDictionary<(Type, string), InboundRouteEntry> _parameterProcessingRouteEntryCache = new();

public TreeRouter? TreeRouter => _router;

Expand All @@ -23,9 +23,11 @@ internal static RouteData ProcessParameters(RouteData endpointRouteData)
{
if (endpointRouteData.Template != null)
{
var entry = _routeEntryCache.GetOrAdd(
// When building this cache, we include static routes because even though the interactive router doesn't use them for
// matching, we still need to process the parameters for them after they are matched during endpoint routing.
var entry = _parameterProcessingRouteEntryCache.GetOrAdd(
(endpointRouteData.PageType, endpointRouteData.Template),
((Type page, string template) key) => RouteTableFactory.CreateEntry(key.page, key.template));
((Type page, string template) key) => RouteTableFactory.CreateEntry(key.page, key.template, includeStaticRoutes: true));

var routeValueDictionary = new RouteValueDictionary(endpointRouteData.RouteValues);
foreach (var kvp in endpointRouteData.RouteValues)
Expand Down
37 changes: 21 additions & 16 deletions src/Components/Components/src/Routing/RouteTableFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,35 +83,40 @@ static void GetRouteableComponents(List<Type> routeableComponents, Assembly asse

internal static RouteTable Create(List<Type> componentTypes, IServiceProvider serviceProvider)
{
var templatesByHandler = new Dictionary<Type, string[]>();
var templatesByHandler = new Dictionary<Type, IReadOnlyList<string>>();
foreach (var componentType in componentTypes)
{
// We're deliberately using inherit = false here.
//
// RouteAttribute is defined as non-inherited, because inheriting a route attribute always causes an
// ambiguity. You end up with two components (base class and derived class) with the same route.
var templates = GetTemplates(componentType);

templatesByHandler.Add(componentType, templates);
// We exclude static routes because this is the interactive router.
var templates = GetTemplates(componentType, includeStaticRoutes: false);
if (templates.Count > 0)
{
templatesByHandler.Add(componentType, templates);
}
}
return Create(templatesByHandler, serviceProvider);
}

private static string[] GetTemplates(Type componentType)
private static IReadOnlyList<string> GetTemplates(Type componentType, bool includeStaticRoutes)
Copy link
Member Author

Choose a reason for hiding this comment

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

Replacing string[] with IReadOnlyList<string> is a possibly-excessive optimization so that we can just build the list in a single pass and don't have to then .ToArray() on it. TBH I could have skipped this change but it's not really difficult and I don't think there's any drawback now it's done.

{
var routeAttributes = componentType.GetCustomAttributes(typeof(RouteAttribute), inherit: false);
var templates = new string[routeAttributes.Length];
for (var i = 0; i < routeAttributes.Length; i++)
var templates = new List<string>(routeAttributes.Length);
foreach (RouteAttribute routeAttribute in routeAttributes)
{
var attribute = (RouteAttribute)routeAttributes[i];
templates[i] = attribute.Template;
if (includeStaticRoutes || !routeAttribute.Static)
{
templates.Add(routeAttribute.Template);
}
}

return templates;
}

[UnconditionalSuppressMessage("Trimming", "IL2067", Justification = "Application code does not get trimmed, and the framework does not define routable components.")]
internal static RouteTable Create(Dictionary<Type, string[]> templatesByHandler, IServiceProvider serviceProvider)
internal static RouteTable Create(Dictionary<Type, IReadOnlyList<string>> templatesByHandler, IServiceProvider serviceProvider)
{
var routeOptions = Options.Create(new RouteOptions());
if (!OperatingSystem.IsBrowser() || RegexConstraintSupport.IsEnabled)
Expand Down Expand Up @@ -141,10 +146,10 @@ internal static RouteTable Create(Dictionary<Type, string[]> templatesByHandler,
return new RouteTable(builder.Build());
}

private static TemplateGroupInfo ComputeTemplateGroupInfo(string[] templates)
private static TemplateGroupInfo ComputeTemplateGroupInfo(IReadOnlyList<string> templates)
{
var result = new TemplateGroupInfo(templates);
for (var i = 0; i < templates.Length; i++)
for (var i = 0; i < templates.Count; i++)
{
var parsedTemplate = RoutePatternParser.Parse(templates[i]);
var parameterNames = GetParameterNames(parsedTemplate);
Expand All @@ -159,15 +164,15 @@ private static TemplateGroupInfo ComputeTemplateGroupInfo(string[] templates)
return result;
}

private struct TemplateGroupInfo(string[] templates)
private struct TemplateGroupInfo(IReadOnlyCollection<string> templates)
{
public HashSet<string> AllRouteParameterNames { get; set; } = new(StringComparer.OrdinalIgnoreCase);
public (RoutePattern, HashSet<string>)[] ParsedTemplates { get; set; } = new (RoutePattern, HashSet<string>)[templates.Length];
public (RoutePattern, HashSet<string>)[] ParsedTemplates { get; set; } = new (RoutePattern, HashSet<string>)[templates.Count];
}

internal static InboundRouteEntry CreateEntry([DynamicallyAccessedMembers(Component)] Type pageType, string template)
internal static InboundRouteEntry CreateEntry([DynamicallyAccessedMembers(Component)] Type pageType, string template, bool includeStaticRoutes)
{
var templates = GetTemplates(pageType);
var templates = GetTemplates(pageType, includeStaticRoutes);
var result = ComputeTemplateGroupInfo(templates);

RoutePattern? parsedTemplate = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ public RouteTable Build()
{
var templatesByHandler = _routeTemplates
.GroupBy(rt => rt.Handler)
.ToDictionary(group => group.Key, group => group.Select(g => g.Template).ToArray());
.ToDictionary(group => group.Key, group => (IReadOnlyList<string>)group.Select(g => g.Template).ToArray());
return RouteTableFactory.Create(templatesByHandler, _serviceProvider);
}
catch (InvalidOperationException ex) when (ex.InnerException is InvalidOperationException)
Expand Down
16 changes: 16 additions & 0 deletions src/Components/Endpoints/src/Builder/ComponentTypeMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,25 @@ public ComponentTypeMetadata([DynamicallyAccessedMembers(Component)] Type compon
Type = componentType;
}

/// <summary>
/// Initializes a new instance of <see cref="ComponentTypeMetadata"/>.
/// </summary>
/// <param name="componentType">The component type.</param>
/// <param name="isStaticPage">A flag indicating whether the page's route is declared as static.</param>
public ComponentTypeMetadata([DynamicallyAccessedMembers(Component)] Type componentType, bool isStaticPage)
: this(componentType)
{
IsStaticRoute = isStaticPage;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We can discuss this in API review, but is there a particular reason for naming the parameter isStaticPage while the property is name IsStaticRoute?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah good point. It should probably be IsStaticRoute all the way.

Copy link
Member Author

Choose a reason for hiding this comment

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

And more broadly as @javiercn has already suggested, the whole name @staticpage could do with change/clarification. We want a name that means "suppress global interactivity for this page" but @suppressglobalinteractivity doesn't look great!

}

/// <summary>
/// Gets the component type.
/// </summary>
[DynamicallyAccessedMembers(Component)]
public Type Type { get; }

/// <summary>
/// Gets a flag indicating whether the page's route is declared as static.
/// </summary>
public bool IsStaticRoute { get; }
Copy link
Member Author

Choose a reason for hiding this comment

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

This could go into an entirely separate metadata class but then that's an extra metadata entry we have to look up on every request, and I don't see there being much reason for it.

}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal void AddEndpoints(
// Name is only relevant for Link generation, which we don't support either.
var builder = new RouteEndpointBuilder(
null,
RoutePatternFactory.Parse(pageDefinition.Route),
RoutePatternFactory.Parse(pageDefinition.Route.Template),
order: 0);

// Require antiforgery by default, let the page override it.
Expand All @@ -47,7 +47,7 @@ internal void AddEndpoints(
// We do not support link generation, so explicitly opt-out.
builder.Metadata.Add(new SuppressLinkGenerationMetadata());
builder.Metadata.Add(HttpMethodsMetadata);
builder.Metadata.Add(new ComponentTypeMetadata(pageDefinition.Type));
builder.Metadata.Add(new ComponentTypeMetadata(pageDefinition.Type, pageDefinition.Route.Static));
builder.Metadata.Add(new RootComponentMetadata(rootComponent));
builder.Metadata.Add(configuredRenderModesMetadata);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ static ComponentApplicationBuilder GetBuilderForAssembly(ComponentApplicationBui
{
pages.Add(new PageComponentBuilder()
{
RouteTemplates = routes.Select(r => r.Template).ToList(),
RouteTemplates = routes.ToList(),
AssemblyName = name,
PageType = candidate
});
Expand Down
32 changes: 26 additions & 6 deletions src/Components/Endpoints/src/Discovery/PageComponentBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.Components.Discovery;
[DebuggerDisplay($"{{{nameof(GetDebuggerDisplay)}(),nq}}")]
internal class PageComponentBuilder : IEquatable<PageComponentBuilder?>
{
private IReadOnlyList<string> _routeTemplates = Array.Empty<string>();
private IReadOnlyList<RouteAttribute> _routeTemplates = Array.Empty<RouteAttribute>();

/// <summary>
/// Gets or sets the assembly name where this component comes from.
Expand All @@ -24,7 +24,7 @@ internal class PageComponentBuilder : IEquatable<PageComponentBuilder?>
/// <summary>
/// Gets or sets the route templates for this page component.
/// </summary>
public required IReadOnlyList<string> RouteTemplates
public required IReadOnlyList<RouteAttribute> RouteTemplates
{
get => _routeTemplates;
set
Expand Down Expand Up @@ -61,7 +61,7 @@ public bool Equals(PageComponentBuilder? other)
{
return other is not null &&
AssemblyName == other.AssemblyName &&
RouteTemplates.SequenceEqual(other.RouteTemplates!, StringComparer.OrdinalIgnoreCase) &&
RouteTemplates.SequenceEqual(other.RouteTemplates!, RouteAttributeComparer.Instance) &&
EqualityComparer<Type>.Default.Equals(PageType, other.PageType);
}

Expand All @@ -81,13 +81,33 @@ public override int GetHashCode()
return hash.ToHashCode();
}

internal PageComponentInfo Build(string route, object[] pageMetadata)
internal PageComponentInfo Build(RouteAttribute route, object[] pageMetadata)
{
return new PageComponentInfo(route, PageType, route, pageMetadata);
return new PageComponentInfo(route.Template, PageType, route, pageMetadata);
}

private string GetDebuggerDisplay()
{
return $"Type = {PageType.FullName}, RouteTemplates = {string.Join(", ", RouteTemplates ?? Enumerable.Empty<string>())}";
return $"Type = {PageType.FullName}, RouteTemplates = {string.Join(", ", RouteTemplates?.Select(r => r.Template) ?? Enumerable.Empty<string>())}";
}

private class RouteAttributeComparer : IEqualityComparer<RouteAttribute>
{
public static RouteAttributeComparer Instance { get; } = new();

public bool Equals(RouteAttribute? x, RouteAttribute? y)
{
if (x is null)
{
return y is null;
}

return y is not null
&& string.Equals(x.Template, y.Template, StringComparison.OrdinalIgnoreCase)
&& x.Static == y.Static;
}

public int GetHashCode([DisallowNull] RouteAttribute value)
=> HashCode.Combine(value.Template, value.Static);
}
}
4 changes: 2 additions & 2 deletions src/Components/Endpoints/src/Discovery/PageComponentInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal class PageComponentInfo
internal PageComponentInfo(
string displayName,
[DynamicallyAccessedMembers(Component)] Type type,
string route,
RouteAttribute route,
IReadOnlyList<object> metadata)
{
DisplayName = displayName;
Expand All @@ -46,7 +46,7 @@ internal PageComponentInfo(
/// <summary>
/// Gets the routes for the page.
/// </summary>
public string Route { get; }
public RouteAttribute Route { get; }

/// <summary>
/// Gets the metadata for the page.
Expand Down
2 changes: 2 additions & 0 deletions src/Components/Endpoints/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
#nullable enable
Microsoft.AspNetCore.Components.Endpoints.ComponentTypeMetadata.ComponentTypeMetadata(System.Type! componentType, bool isStaticPage) -> void
Microsoft.AspNetCore.Components.Endpoints.ComponentTypeMetadata.IsStaticRoute.get -> bool
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,14 @@ private async Task RenderComponentCore(HttpContext context)

var endpoint = context.GetEndpoint() ?? throw new InvalidOperationException($"An endpoint must be set on the '{nameof(HttpContext)}'.");

var componentTypeMetadata = endpoint.Metadata.GetRequiredMetadata<ComponentTypeMetadata>();
var rootComponent = endpoint.Metadata.GetRequiredMetadata<RootComponentMetadata>().Type;
var pageComponent = endpoint.Metadata.GetRequiredMetadata<ComponentTypeMetadata>().Type;
var pageComponent = componentTypeMetadata.Type;

if (componentTypeMetadata.IsStaticRoute)
{
_renderer.SuppressRootComponentRenderModes();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the bit where we take the endpoint metadata and turn it into a behavior on the renderer.


Log.BeginRenderRootComponent(_logger, rootComponent.Name, pageComponent.Name);

Expand Down
Loading
Loading