Enable Blazor Server and WebAssembly components to run in the same document #48294

MackinnonBuck · 2023-05-18T00:14:22Z

Enable Blazor Server and WebAssembly components to run in the same document

This PR enables common interactivity scenarios for SSR'd Blazor apps with components rendered in "Server" and/or "WebAssembly" render modes.

There is still some remaining work and open questions to be resolved before Blazor Server and WebAssembly can run on the same page in complete harmony. Most of these can be addressed in future PRs:

Update browser-side navigation to take into account the possibility of multiple .NET runtimes being present.
Update JS initializer behavior to account for multiple runtimes (e.g., should there be multiple calls to afterStarted()?).
Handle spinning up .NET runtimes after the initial page load.
Handle closing circuits when there are no server components remaining.
Implement "auto" mode logic.
Figure out a good testing strategy - how can we be sure each feature works correctly when using Server and WebAssembly at the same time?
- Probably easiest to pick out the scenarios that we know are tricky and test those.

Fixes #48032

Note: consider reverting this commit before merging.

MackinnonBuck · 2023-05-18T00:15:45Z

src/Components/Components.slnf

Feel free to ignore the changes to the BlazorUnitedApp sample. This was just to make it easy to test changes. I may later revert the commit with changes to the sample project.

src/Components/Web.JS/src/Boot.Server.Common.ts

src/Components/Web.JS/src/Boot.Web.ts

src/Components/Web.JS/src/GlobalExports.ts

src/Components/Web.JS/src/Rendering/WebRendererInteropMethods.ts

src/Components/Web.JS/src/Services/ComponentDescriptorDiscovery.ts

src/Components/Web.JS/src/Services/NavigationManager.ts

src/Components/Web/src/WebRenderer.cs

MackinnonBuck · 2023-05-18T00:44:10Z

src/JSInterop/Microsoft.JSInterop.JS/src/src/Microsoft.JSInterop.ts

  export function invokeMethod<T>(assemblyName: string, methodIdentifier: string, ...args: any[]): T {
-      return invokePossibleInstanceMethod<T>(assemblyName, methodIdentifier, null, args);
+      const dispatcher = getDefaultCallDispatcher();
+      return dispatcher.invokeDotNetStaticMethod<T>(assemblyName, methodIdentifier, ...args);


We could make it so that calling this method throws an error unless explicitly allowed via some sort of option. That would encourage people to use DotNetObjectReference instead.

src/Components/Web.JS/src/Boot.Web.ts

javiercn

Looks like a great start, I have left some feedback!

src/Components/Samples/BlazorUnitedApp.WebAssembly/Program.cs

src/Components/Web.JS/src/Services/ComponentDescriptorDiscovery.ts

src/Components/Samples/BlazorUnitedApp/Program.cs

javiercn · 2023-05-21T11:24:14Z

src/Components/Web/src/WebRenderer.cs

+        var jsRuntime = serviceProvider.GetRequiredService<IJSRuntime>();
+        _attachTask = jsRuntime.InvokeAsync<int>(


If we want to avoid this dance, we can change the Renderer ID for a GUID instead of an int.

I would avoid doing this inside the constructor and instead have a static factory method that we can call to initialize the renderer. If we need to / want to, we can have an interface for it, but I don't think we need it. It avoids the whole task "dance".

Additionally, for Blazor server we can pass in the ID directly on the call to Blazor Start.

We could similarly do something with web assembly, maybe with an environment variable or similar that we could read from config (Plugging in the environment variables provider). I believe this is possible since we have "harmonized" the boot process.

Yeah this is a bit awkward. There's nothing objectively incorrect about starting a task from a constructor [1], but it definitely pushes up against the boundaries of normal constructor behavior.

The factory method option is certainly a viable possibility, albeit with its own drawbacks (if you're not supposed to use the constructor any more, this becomes a viral concern since the same would be true for any derived classes). Right now we could get away with the factory method as only a minor breaking change.

However @javiercn's points about other approaches are also promising. Do we really need to support registering an arbitrary number of renderers dynamically? What limitations would we face if we just said there would only be at most one of a given type, and we have some convention like 0=server, 1=webassembly? Is it enough just for these two type not to clash?

[1] Well it's debatable. Some people probably would say they wouldn't accept it at all, e.g., https://blog.stephencleary.com/2013/01/async-oop-2-constructors.html

Actually, you bring up a good point. We can use a HILO strategy, where each mode gets a different "HI" value, and we use different LO values. To be clear, if the render mode is an Int32, take the two most significant bytes and give 0 to Server and 1 to Webassembly, (2 to Webview), and use 0 for the two least significant bytes. If in the future we want to support multiple renderers of the same type, each one will just "rev" the ID.

If we want to avoid this dance, we can change the Renderer ID for a GUID instead of an int.

This could work, but wouldn't it mean we're serializing and sending a much larger ID to JavaScript each time we render?

What limitations would we face if we just said there would only be at most one of a given type, and we have some convention like 0=server, 1=webassembly? Is it enough just for these two type not to clash?

This was the approach I took during prototyping, and it works, but I decided not to do that in this PR because:

We have to keep those hard-coded IDs in sync between .NET and JS code

We have to remember to assign new a renderer ID to each execution model we add in the future

It makes it more difficult to enable having multiple renderers per platform, in case that ever becomes useful (although I recognize Javier's HILO suggestion solves that)

We can use a HILO strategy, where each mode gets a different "HI" value, and we use different LO values. To be clear, if the render mode is an Int32, take the two most significant bytes and give 0 to Server and 1 to Webassembly, (2 to Webview), and use 0 for the two least significant bytes. If in the future we want to support multiple renderers of the same type, each one will just "rev" the ID.

I'm not sure I'm totally convinced yet that these alternate approaches are better than what we currently have. Even if we were to change the way to allocate renderer IDs, we would still have to call attachWebRendererInterop from .NET to provide JS with the dependencies it needs to perform rendering. So, we'd still have the problem of figuring out the right way make that invocation; it would still be problematic to attempt to render before that call completes, even if the .NET side doesn't rely on the return value of attachWebRendererInterop to assign a renderer ID. If we have to do the JS interop call anyway, what are we losing by using it to also allocate a renderer ID?

I do totally agree that it's not ideal for the WebRenderer constructor to initiate a JS interop call. I think the factory approach would be the best way to handle it, but given that it would be a breaking change (and that we've been invoking the JS interop call in the constructor for a while now already), it seems fine to me to keep things as they are.

Thoughts?

wouldn't it mean we're serializing and sending a much larger ID to JavaScript each time we render?

Yes, would be nice to avoid that.

and that we've been invoking the JS interop call in the constructor for a while now already

That is a good point - I agree that since this change doesn't actually worsen anything, it's not really necessary to force an architectural or API-breaking change here. Currently this is only an internal detail and if we want to make an API-breaking change to turn this into a factory, there's nothing better about doing that now than any time in the future.

We can see if @javiercn has further practical reasons for changing this but otherwise this looks good to me.

javiercn · 2023-05-21T11:39:10Z

src/Components/Web.JS/src/Boot.Server.Common.ts

+  dispatcher = DotNet.attachDispatcher({
+    beginInvokeDotNetFromJS: (callId, assemblyName, methodIdentifier, dotNetObjectId, argsJson): void => {
+      connection.send('BeginInvokeDotNetFromJS', callId ? callId.toString() : null, assemblyName, methodIdentifier, dotNetObjectId || 0, argsJson);
+    },
+    endInvokeJSFromDotNet: (asyncHandle, succeeded, argsJson): void => {
+      connection.send('EndInvokeJSFromDotNet', asyncHandle, succeeded, argsJson);
+    },
+    sendByteArray: (id: number, data: Uint8Array): void => {
+      connection.send('ReceiveByteArray', id, data);
+    },
+  });


Could we make this a bit more idiomatic?

In this multi dispatching world I would have expected for us to new up a dispatcher instance and configure it. With a server implementation and a webassembly implementation.

Just so I'm understanding correctly, is the only change you're suggesting that we change it from DotNet.attachDispatcher(...) to something like new DotNet.CallDispatcher(...)?

There already is a server and client implementation with the way things are currently set up. attachDispatcher() new-s up a new CallDispatcher instance, plus does some additional checks like "if this is the first dispatcher, make it the default" and "if this is not the first dispatcher, remove the default".

I guess the one advantage of keeping the attachDispatcher() way of doing things is that it can return an ICallDispatcher, which has a more restrictive public API compared to the non-exported CallDispatcher implementation.

src/Components/Web.JS/src/Boot.Web.ts

src/Components/Web.JS/src/Rendering/WebRendererInteropMethods.ts

src/Components/Samples/BlazorUnitedApp/Pages/FetchData.razor

src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs

SteveSandersonMS

Looks great to me! Would be best to get an ack from @javiercn about the renderer ID thing before merging if possible.

MackinnonBuck added 4 commits May 17, 2023 16:44

Enable basic scenarios

73b3eea

Updated test app

743704a

Note: consider reverting this commit before merging.

Fixed a few bugs

7bfd3ef

Build fix

55dbbfa

ghost added the area-blazor Includes: Blazor, Razor Components label May 18, 2023

MackinnonBuck commented May 18, 2023

View reviewed changes

Undo submodule change

c60cede

MackinnonBuck requested a review from SteveSandersonMS May 18, 2023 00:49

MackinnonBuck assigned javiercn and unassigned javiercn May 18, 2023

MackinnonBuck requested a review from javiercn May 18, 2023 03:05

MackinnonBuck added 3 commits May 18, 2023 16:21

Fixes + cleanup

11f1af2

Minor update to sample

2afbd46

Variable name cleanup

6756170

MackinnonBuck marked this pull request as ready for review May 18, 2023 23:33

MackinnonBuck requested a review from a team as a code owner May 18, 2023 23:33

MackinnonBuck commented May 18, 2023

View reviewed changes

src/Components/Web.JS/src/Boot.Web.ts Show resolved Hide resolved

Fix WebAssemblyHostTest

f678b4a

MackinnonBuck mentioned this pull request May 19, 2023

Update JSInProcessObjectReference.cs #48287

Merged

javiercn reviewed May 21, 2023

View reviewed changes

SteveSandersonMS reviewed May 22, 2023

View reviewed changes

src/Components/Samples/BlazorUnitedApp/Pages/FetchData.razor Outdated Show resolved Hide resolved

SteveSandersonMS reviewed May 22, 2023

View reviewed changes

src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs Show resolved Hide resolved

Revert sample project changes

62ead8a

MackinnonBuck force-pushed the mbuck/server-plus-wasm branch from 95fa3da to 62ead8a Compare May 22, 2023 21:49

MackinnonBuck requested review from SteveSandersonMS and javiercn May 22, 2023 23:21

SteveSandersonMS approved these changes May 23, 2023

View reviewed changes

MackinnonBuck added 3 commits May 23, 2023 09:20

Merge branch 'main' into mbuck/server-plus-wasm

af94b2a

PR feedback

1915288

Oops

83381be

javiercn approved these changes May 23, 2023

View reviewed changes

MackinnonBuck merged commit d9b636c into main May 23, 2023

MackinnonBuck deleted the mbuck/server-plus-wasm branch May 23, 2023 20:22

ghost added this to the 8.0-preview5 milestone May 23, 2023

		var jsRuntime = serviceProvider.GetRequiredService<IJSRuntime>();
		_attachTask = jsRuntime.InvokeAsync<int>(

Enable Blazor Server and WebAssembly components to run in the same document #48294

Enable Blazor Server and WebAssembly components to run in the same document #48294

Uh oh!

Conversation

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

Uh oh!

Enable Blazor Server and WebAssembly components to run in the same document

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

javiercn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

Uh oh!

SteveSandersonMS left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MackinnonBuck commented May 18, 2023 •

edited

Loading

SteveSandersonMS May 22, 2023 •

edited

Loading