Skip to content

Add API to System.Web adapters to enable explicitly mapping Blazor pages #46170

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
twsouthwick opened this issue Jan 19, 2023 · 11 comments
Closed
Labels
area-blazor Includes: Blazor, Razor Components

Comments

@twsouthwick
Copy link
Member

twsouthwick commented Jan 19, 2023

Background and Motivation

We created a set of adapters and guidance (https://github.com/dotnet/systemweb-adapters) to simplify the migration process from ASP.NET to ASP.NET Core. As part of this, we have recommended people set up a ASP.NET Core application that will attempt to serve requests and fallback to the original ASP.NET framework application via YARP. This works well for many things, but it fails when trying to use Blazor. This is because currently both Blazor and YARP are fallback endpoints and this conflicts with each other.

See #42003 for an existing issue on dotnet/aspnetcore that is tracking the need for this.

Proposed API

For the dotnet/systemweb-adapters, I have implemented an extension method that will accomplish this need. However, as this is a feature planned for .NET 8, I want to ensure we have identified a few things:

  1. The API in the web adapters should be useable for .NET 6 and .NET 7, but provide a way to migrate to the official .NET 8 API
  2. We don't want to conflict with the API design for .NET 8

Here is the public API for this that we are currently exposing on the Microsoft.AspNetCore.SystemWebAdapters package:

namespace Microsoft.AspNetCore.Builder;

public static class BlazorEndpointRouteBuilderExtensions
{
+    public static IEndpointConventionBuilder MapBlazorPages(this IEndpointRouteBuilder endpoints, string page);
+    public static IEndpointConventionBuilder MapBlazorPages(this IEndpointRouteBuilder endpoints, string page, params Assembly[] assemblies)
}

Usage Examples

An example usage would be:

app.MapBlazorHub();

// This is the default way to set up Blazor pages
//app.MapFallbackToPage("/_Host");

// Proposed API
app.MapBlazorPages("/_Host");

app.MapReverseProxy();

Risks

  • How do we prevent this API from conflicting with an API for .NET 8 that will accomplish a similar thing but still available for down level?
@twsouthwick twsouthwick added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 19, 2023
@twsouthwick
Copy link
Member Author

@Tratcher Tratcher added area-blazor Includes: Blazor, Razor Components api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jan 19, 2023
@ghost
Copy link

ghost commented Jan 19, 2023

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@Tratcher
Copy link
Member

  • How do we prevent this API from conflicting with an API for .NET 8 that will accomplish a similar thing but still available for down level?

A) match the name and parameters of the .NET 8 API and then exclude the SystemWeb version for the .NET 8 TFM. This allows a silent swap. Too much magic?
B) Intentionally give it a different name in SystemWeb and then obsolete it for the .NET 8 TFM.

@twsouthwick twsouthwick changed the title Add API to enable explicitly mapping Blazor pages Add API to System.Web adapters to enable explicitly mapping Blazor pages Jan 19, 2023
@davidfowl
Copy link
Member

These APIs shouldn't be added to the System.Web adapters repository. We can ship docs that users can use for now and add these in .NET 8 in the right place.

@danroth27
Copy link
Member

These APIs shouldn't be added to the System.Web adapters repository. We can ship docs that users can use for now and add these in .NET 8 in the right place.

@davidfowl Delivering this support in .NET 8 is certainly the plan. But .NET 8 won't ship for almost a year. To enable incremental migration to Blazor sooner we'd like to deliver this API as part of the System.Web Adapters and then transition users to the .NET 8 API once it is released.

@davidfowl
Copy link
Member

Then it would need to be exposed as a flag instead of an API within the System.Web adapters instead of an API

@twsouthwick
Copy link
Member Author

We've decided to just add docs for migration that will include a working snippet instead and point people to the .NET 8 API when available.

@davidfowl out of curiosity what do you mean by "exposed as a flag"?

@davidfowl
Copy link
Member

I was just saying that if we wanted to expose this feature in the System.Web.Adapters that we woud not expose it as a top level API call, instead that would be internal and there would be a top level flag that could enable or disable the mapping blazor pages. That would allow us to keep the flag and change the underlying implementation without exposing these APIs from the wrong place.

@mkArtakMSFT
Copy link
Contributor

@davidfowl, @twsouthwick it's not clear whether there is still any work you need from Blazor as part of this issue or not? If not, can you please close this? Thanks!

@davidfowl
Copy link
Member

I think this is still valuable to add to the framework.

@twsouthwick
Copy link
Member Author

@davidfowl agree it should be in .NET 8. I chatted with @javiercn and he is planning on doing that and I'm assuming will drive any API changes

@ghost ghost locked as resolved and limited conversation to collaborators Mar 2, 2023
@halter73 halter73 removed the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Aug 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

No branches or pull requests

6 participants