[Blazor] Add `IRazorComponentsBuilder.AddWebAssemblyComponents()` #48664

MackinnonBuck · 2023-06-08T00:05:45Z

Add `IRazorComponentsBuilder.AddWebAssemblyComponents()`

Adds the IRazorComponentsBuilder.AddWebAssemblyComponents() extension method. This allows Blazor WebAssembly components to be rendered from a Blazor app that utilizes SSR.

Fixes #46354

src/Components/WebAssembly/Server/src/Microsoft.AspNetCore.Components.WebAssembly.Server.csproj

SteveSandersonMS

Approving because it's clear this is close to being ready and you can decide for yourself whether it's right to swap around the dependency direction as per my comment.

Also I think we're now at a point where the E2E tests for Endpoint rendering could be extended to include some cases where we insert interactive Server/WebAssembly components and observe that they are actually interactive. Since we already have the core structure for those E2E tests (i.e., there's an app, and the tests know how to start it, etc.) it might not be a massive amount of extra work to introduce one or two tests to show interactivity is possible now. Would you consider that? If you feel it's outside the scope of what you want to handle here that's fine - we just have to get to that at some point :)

MackinnonBuck · 2023-06-09T00:00:22Z

src/Components/Endpoints/src/DependencyInjection/WebAssemblyComponentsEndpointOptions.cs

+/// <summary>
+/// Options to configure interactive WebAssembly components in a razor components application.
+/// </summary>
+public sealed class WebAssemblyComponentsEndpointOptions
+{
+    /// <summary>
+    /// Gets or sets <see cref="PathString"/> that indicates the prefix for Blazor WebAssembly framework files.
+    /// </summary>
+    public PathString FrameworkFilesPathPrefix { get; set; }
+}


This API allows the path prefix passed to UseBlazorFrameworkFiles to be configurable in a "full-stack Blazor"-style app.

Is this something we could avoid?

What's the reason for wanting to avoid it? Is it to keep the API smaller for the initial release? Without this API, I don't think there's a way for customers to configure the path prefix passed to UseBlazorFrameworkFiles(). I was using this API in our own test app so that Components.TestServer could reference both BasicTestApp as well as a new, minimal WebAssembly project.

Seems reasonable to allow this configurability if it's what's need to host multiple independent WebAssembly apps on a single server app.

On the naming though, I was a bit misled at first since I thought this would be the path to the _framework directory (inclusive), whereas from the implementation I see it's the path prefix before the _framework directory.

What if this were renamed to PathPrefix or StaticAssetsPathPrefix?

Also, does this affect the path at which we host the wwwroot content from a referenced WebAssembly project?

This only impacts the path from which _framework folder content is loaded. The paths from which wwwroot content gets served can be configured with UseStaticFiles().

I'm worried PathPrefix might not be clear enough in the context of calling AddWebAssemblyComponents(). I think StaticAssetsPathPrefix is a bit too general given that it's specifically _framework files that we're configuring.

Although, I can't think of a case where the _framework path prefix would be different than the path prefix for wwwroot content. So maybe it's okay to go with StaticAssetsPathPrefix? But maybe then there could still be some confusion around whether calling UseStaticFiles() is still necessary.

Perhaps we keep the current name but update the XML comment to make this clearer?

It also affects _content, doesn't it? Or if not I think that would be at risk of clashing. And possibly any other framework-supplied files such as hot reload scripts, though I'm not so sure about that.

I don't feel strongly about the name, so if you think FrameworkFilesPathPrefix is the safest choice in terms of accuracy and understandability, that's fine with me.

I could totally be missing something, but from what I can tell it looks like _content isn't affected by this:

aspnetcore/src/Components/WebAssembly/Server/src/ComponentsWebAssemblyApplicationBuilderExtensions.cs

Line 37 in 2971950

builder.MapWhen(ctx => ctx.Request.Path.StartsWithSegments(pathPrefix, out var rest) && rest.StartsWithSegments("/_framework") &&

.

Let's do PathPrefix for now - one could make the argument that the fact that it only affects _framework files is an uninteresting detail to the developer.

I could totally be missing something, but from what I can tell it looks like _content isn't affected by this:

Is that a bug then? I thought the scenario here was "hosting unrelated WebAssembly apps on separate paths". If that's the case we can't have them both using /_content.

MackinnonBuck · 2023-06-09T00:01:44Z

src/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cs

+            .AddWebAssemblyComponents(options =>
+            {
+                options.FrameworkFilesPathPrefix = "/WasmMinimal";
+            });


This is so that we don't inherit the baggage from BasicTestApp when all we need is a super minimal WebAssembly app to enable WebAssembly interactivity.

MackinnonBuck · 2023-06-09T00:05:38Z

src/Components/test/testassets/Components.WasmMinimal/Program.cs

+using System.Reflection;
+using Microsoft.AspNetCore.Components.WebAssembly.Hosting;
+
+Assembly.Load(nameof(TestContentPackage));


Without this, the TestContentPackage assembly does not get loaded, and RootComponentTypeCache can't discover the component.

Would we consider updating RootComponentTypeCache to attempt to load an assembly if it couldn't be found?

This is very interesting. Is this just a quirk of the framework that is already the case in .NET 7? For example, if you used <component ... render-mode="WebAssembly"> (as opposed to declaring the root component in Program.cs) could you get the same issue? My guess is yes.

Rather than block your PR on it, I would recommend filing an issue for us to dig into (unless you think you are actively causing this issue in your PR here, which doesn't appear to be the case to me).

Would we consider updating RootComponentTypeCache to attempt to load an assembly if it couldn't be found?

Assuming you mean code that is actually running on WebAssembly (e.g., inside WebAssemblyHostBuilder) then yes sure, why not. It's not a security concern in that case.

There could still be a problem since trimming may have removed the component if there was no statically-observable evidence that it was going to be used. However in practice that would be uncommon since we don't trim user assemblies by default. This may change in the future.

...tassets/Components.TestServer/RazorComponents/Components/WebAssemblyInteractiveCounter.razor

MackinnonBuck · 2023-06-09T00:30:00Z

Thanks for the feedback, @SteveSandersonMS! I've added some basic E2E tests to validate interactivity.

I'm re-requesting a review because I've added some new APIs since you last looked (and brought WebAssembly.Server into the shared framework).

javiercn · 2023-06-09T10:28:58Z

src/Components/Endpoints/src/DependencyInjection/WebAssemblyRazorComponentsBuilderExtensions.cs

+    /// <param name="builder">The <see cref="IRazorComponentsBuilder"/>.</param>
+    /// <param name="configure">A callback to configure <see cref="WebAssemblyComponentsEndpointOptions"/>.</param>
+    /// <returns>An <see cref="IRazorComponentsBuilder"/> that can be used to further customize the configuration.</returns>
+    public static IRazorComponentsBuilder AddWebAssemblyComponents(this IRazorComponentsBuilder builder, Action<WebAssemblyComponentsEndpointOptions>? configure = null)


Why does this not go on Webassembly.Server?

😄 #48664 (comment)

I initially put it in WebAssembly.Server because it's consistent with how AddServerComponents is in Components.Server, and inverting the direction of the dependency requires moving WebAssembly.Server into the shared framework.

Does this align with your thinking, or was there another reason you think it belongs in WebAssembly.Server, @javiercn?

amcasey · 2023-06-09T18:21:51Z

Hard to see how the CI failure in Http2WebSocketTests.HEADERS_Received_SecondRequest_ConnectProtocolReset() could be related - probably flaky.

`Http2WebSocketTests.HEADERS_Received_SecondRequest_ConnectProtocolReset` failed in #48664

MackinnonBuck · 2023-06-09T21:17:17Z

I can't figure out why Microsoft.AspNetCore.Components.WebAssembly.Server is not getting picked up during source build.

@wtgodbe Do you know why this could be happening?

Edit: For context, I added Microsoft.AspNetCore.Components.WebAssembly.Server to the shared framework and made it a dependency of Microsoft.AspNetCore.Components.Endpoints. But even though Components.Endpoints is getting built in source build, Components.WebAssembly.Server is not, causing a compilation error:

'IApplicationBuilder' does not contain a definition for 'UseBlazorFrameworkFiles' and no accessible extension method 'UseBlazorFrameworkFiles' accepting a first argument of type 'IApplicationBuilder' could be found (are you missing a using directive or an assembly reference?)

(UseBlazorFrameworkFiles is defined in Components.WebAssembly.Server)

wtgodbe · 2023-06-09T21:29:23Z

I can't figure out why Microsoft.AspNetCore.Components.WebAssembly.Server is not getting picked up during source build.

@wtgodbe Do you know why this could be happening?

Some of our custom reference resolution logic is turned off for source build, so it may be referencing an older version of the project. The source build binlog will probably contain useful info about what's getting resolved here. @MichaelSimons is this the kind of situation we've used source build patches for in the past?

MichaelSimons · 2023-06-12T13:16:46Z

Some of our custom reference resolution logic is turned off for source build, so it may be referencing an older version of the project. The source build binlog will probably contain useful info about what's getting resolved here.

Taking a cursory look, the WebAssembly components where it looks like UseBlazorFrameworkFiles is defined are excluded from SourceBuild - https://github.com/dotnet/aspnetcore/blob/main/src/Components/WebAssembly/Directory.Build.props.

@MichaelSimons is this the kind of situation we've used source build patches for in the past?

No

MackinnonBuck · 2023-06-14T01:14:18Z

I've moved AddWebAssemblyComponents back to WebAssembly.Server because there were some challenges with bringing WebAssembly.Server into the shared framework at this time.

@SteveSandersonMS @javiercn Do you think this is good to merge?

src/Components/WebAssembly/Server/src/RazorComponentsBuilderExtensions.cs

src/Components/Endpoints/src/DependencyInjection/WebAssemblyComponentsEndpointOptions.cs

SteveSandersonMS

Looks great!

…mponentsEndpointOptions.cs Co-authored-by: Steve Sanderson <[email protected]>

Add 'AddWebAssemblyComponents()'

e9141cc

MackinnonBuck requested a review from a team as a code owner June 8, 2023 00:05

ghost added area-blazor Includes: Blazor, Razor Components labels Jun 8, 2023

SteveSandersonMS reviewed Jun 8, 2023

View reviewed changes

src/Components/WebAssembly/Server/src/Microsoft.AspNetCore.Components.WebAssembly.Server.csproj Show resolved Hide resolved

SteveSandersonMS approved these changes Jun 8, 2023

View reviewed changes

PR feedback + new APIs + basic E2E tests

94b5069

MackinnonBuck requested review from a team and wtgodbe as code owners June 8, 2023 23:54

MackinnonBuck commented Jun 9, 2023

View reviewed changes

MackinnonBuck requested review from SteveSandersonMS and javiercn June 9, 2023 00:30

javiercn reviewed Jun 9, 2023

View reviewed changes

Update Microsoft.AspNetCore.Components.WebAssembly.Server.csproj

e2bf79c

amcasey added a commit that referenced this pull request Jun 9, 2023

Retry failing helix test

2618032

`Http2WebSocketTests.HEADERS_Received_SecondRequest_ConnectProtocolReset` failed in #48664

amcasey mentioned this pull request Jun 9, 2023

Retry failing helix test #48694

Merged

Keep shipping WebAssembly.Server as a package

04ed250

amcasey added a commit that referenced this pull request Jun 9, 2023

Retry failing helix test (#48694)

c55ee62

`Http2WebSocketTests.HEADERS_Received_SecondRequest_ConnectProtocolReset` failed in #48664

MackinnonBuck added 2 commits June 9, 2023 13:24

Fix tests

49842f5

Update Framework.slnf

71cd4f7

MackinnonBuck added 4 commits June 12, 2023 10:21

Create Directory.Build.props

63dab0c

Move AddWebAssemblyComponents back to WebAssembly.Server

133f167

Update SharedFramework.Local.props

688b30a

Update PublicAPI.Unshipped.txt

a944a22

javiercn reviewed Jun 14, 2023

View reviewed changes

src/Components/WebAssembly/Server/src/RazorComponentsBuilderExtensions.cs Show resolved Hide resolved

Use Add{Server|WebAssembly}RenderMode()

d8fa166

SteveSandersonMS reviewed Jun 15, 2023

View reviewed changes

src/Components/Endpoints/src/DependencyInjection/WebAssemblyComponentsEndpointOptions.cs Outdated Show resolved Hide resolved

SteveSandersonMS approved these changes Jun 15, 2023

View reviewed changes

MackinnonBuck and others added 2 commits June 15, 2023 08:08

Update src/Components/Endpoints/src/DependencyInjection/WebAssemblyCo…

0f56f1a

…mponentsEndpointOptions.cs Co-authored-by: Steve Sanderson <[email protected]>

Rename FrameworkFilesPathPrefix to PathPrefix

52f364f

javiercn approved these changes Jun 15, 2023

View reviewed changes

MackinnonBuck merged commit 56f2ba4 into main Jun 15, 2023

MackinnonBuck deleted the mbuck/enable-webassembly-components branch June 15, 2023 22:45

ghost added this to the 8.0-preview6 milestone Jun 15, 2023

[Blazor] Add IRazorComponentsBuilder.AddWebAssemblyComponents() #48664

[Blazor] Add IRazorComponentsBuilder.AddWebAssemblyComponents() #48664

Uh oh!

Conversation

MackinnonBuck commented Jun 8, 2023

Add IRazorComponentsBuilder.AddWebAssemblyComponents()

Uh oh!

Uh oh!

SteveSandersonMS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MackinnonBuck commented Jun 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amcasey commented Jun 9, 2023

Uh oh!

MackinnonBuck commented Jun 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wtgodbe commented Jun 9, 2023

Uh oh!

MichaelSimons commented Jun 12, 2023

Uh oh!

MackinnonBuck commented Jun 14, 2023

Uh oh!

Uh oh!

Uh oh!

SteveSandersonMS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[Blazor] Add `IRazorComponentsBuilder.AddWebAssemblyComponents()` #48664

[Blazor] Add `IRazorComponentsBuilder.AddWebAssemblyComponents()` #48664

Add `IRazorComponentsBuilder.AddWebAssemblyComponents()`

MackinnonBuck commented Jun 9, 2023 •

edited

Loading