Skip to content

[Blazor] Add Blazor Web Endpoints to the endpoint data source#57086

Merged
javiercn merged 3 commits intomainfrom
javiercn/fix-webjs-endpoints
Aug 5, 2024
Merged

[Blazor] Add Blazor Web Endpoints to the endpoint data source#57086
javiercn merged 3 commits intomainfrom
javiercn/fix-webjs-endpoints

Conversation

@javiercn
Copy link
Copy Markdown
Member

@javiercn javiercn commented Jul 30, 2024

Register the blazor.web.js file and the opaque-redirect endpoints into the endpoint data source.

Fixes #57081

@javiercn javiercn requested a review from a team as a code owner July 30, 2024 17:41
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Jul 30, 2024
convention(endpoint);
}

foreach (var convention in _finallyConventions)
Copy link
Copy Markdown
Member

@halter73 halter73 Jul 31, 2024

Choose a reason for hiding this comment

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

RazorComponentEndpointDataSource ought to implement GetGroupedEndpoints and call _groupConventions first and _groupFinallyConventions last like RouteEndpointDataSource, ControllerActionEndpointDataSource, and PageActionEndpointDataSource do.

// Apply group conventions before entry-specific conventions added to the RouteHandlerBuilder.
if (groupConventions is not null)
{
foreach (var groupConvention in groupConventions)
{
groupConvention(builder);
}
}

// Group metadata has the lowest precedence.
for (var i = 0; i < groupConventions.Count; i++)
{
groupConventions[i](builder);
}

The default virtual implementation of GetGroupedEndpoints provides an approximation of this, but it's not as good because group conventions are forced to run after the _conventions and _finallyConventions associated with MapRazorComponent. It might not be super relevant for these endpoints, but I know WithOpenApi looks for already-added metadata in its convention callbacks. The default implementation also prevents a group from injecting any "inner" endpoint filters.

// Any metadata already on the RouteEndpoint must have been applied directly to the endpoint or to a nested group.
// This makes the metadata more specific than what's being applied to this group. So add it after this group's conventions.
foreach (var metadata in routeEndpoint.Metadata)
{
routeEndpointBuilder.Metadata.Add(metadata);
}

These are pretty niche problems. We tried to make the default implementation of GetGroupedEndpoints as good as possible, but we might as well align behavior with our other EndpointDataSources for maximum correctness.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@halter73 I'm going to consider adding support for that outside of scope for this change. I've filed #57120 to track it separately, since we are planning on patching this, I want to keep this PR small.

We can discuss the other one separately, as we haven't seen it used in conjunction with MapGroup in the wild I think in a way that causes problems.

Copy link
Copy Markdown
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

Indecently, my comment about the default implementation of GetGroupedEndpoints demonstrates how you could work around #57086 today by doing something like the following:

var blazorGroup = app.MapGroup("");

blazorGroup.MapRazorComponent<App>();
    .AddInteractiveServerRenderMode();

blazorGroup.Add(endpointBuilder =>
{
    if (endpointBuilder is RouteEndpointBuilder routeEndpointBuilder)
    {
        if (string.Equals(routeEndpointBuilder.RoutePattern.RawText, "_framework/blazor.web.js", StringComparison.OrdinalIgnoreCase))
        {
            routeEndpointBuilder.Metadata.Add(new AllowAnonymousAttribute());
        }
    }
});

This works because the default GetGroupedEndpoints creates RouteEndpointBuilder's out of RouteEndpoints retuned by the abstract Endpoints factory. I still think this is worth fixing and backporting to 8.0 though.

Copy link
Copy Markdown
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I'll wait to approve until after Stephen's comments are resolved.

@javiercn javiercn merged commit d73a7af into main Aug 5, 2024
@javiercn javiercn deleted the javiercn/fix-webjs-endpoints branch August 5, 2024 10:42
@dotnet-policy-service dotnet-policy-service Bot added this to the 9.0-rc1 milestone Aug 5, 2024
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.

Blazor _framework/blazor.web.js endpoint is not configurable.

3 participants