[WIP] Server-side blazor support #12
Conversation
csnewman
changed the title
test/BlazorSignalR.Test.RazorComponents/Controllers/TokenController.cs
Outdated
Show resolved
Hide resolved
|
@AndreasTruetschel Any progress? |
|
I'm currently working on this.
|
|
Thanks. I assume you're just creating a "BrowserHttpMessageHandler"/"BlazorHttpMessageHandler" or something of the sorts that will be used regardless of whether it's server side of client side blazor? |
|
Would it also be possible for the |
csnewman
changed the title
Sure. I wanted to do this anyway. |
This would be a solution but needs the complete replacement and therefore reimplementation (or fork) of the @csnewman Do you have a better idea for this? |
Afaik the webassembly one uses WebAssemblyJsRuntime explicitly. I also assume the JS probably isn't included as the server side blazor uses a different js file in the browser. I therefore get the impression that a custom message handler will have to be made (well copied). |
|
I looked into this. The typescript code of the original implementation has to be changed massively, as they do not use ordinary interop but rely on the fact that this thing runs on web assembly. If this is forked and adapted in order to be used both, client and server-side it will incure a massive hit in performance IMO as this will need to work with json serialization then. I don't think this will be possible in general, when we do not know that we are running client-side within wasm. |
|
Maybe a slight change of course is a better option. Currently Maybe instead we can detect if running on server side blazor and disable any non-js transports in |
|
Thats seems to be a reasonable approach. However we need a |
|
This is working now for both server and client-side projects. I tried to re-use and/or link most of the stuff in the server projects as it is not possible for now to combine these without copying a lot of source code from the blazor repo. It seems they are currently working on this so maybe we can do this soon. Still open issues:
@csnewman Can you please review the latest changes? |
|
It feels like a lot of baggage having a custom |
| /// <summary> | ||
| /// A browser-compatible implementation of <see cref="HttpMessageHandler"/> | ||
| /// </summary> | ||
| internal class BlazorHttpMessageHandler : HttpMessageHandler |
There was a problem hiding this comment.
As mentioned already, I see little purpose for having this this heavy weight custom HttpMessageHandler just for a one-off negotiation request.
There was a problem hiding this comment.
The BlazorServerSentEventsTransport internally also uses an http-client. Replacing the current solution needs us to rewrite the negotiation in js together with the parts the BlazorServerSentEventsTransport does with it. I don't think this will be less costly for at least the development and maintenance costs. For runtime statistics we had to test this as I cannot estimate the performance characteristics of the two solutions. The current solution also gives as the benefit if we ever wanted to support some kind of managed (well C# implemented) transport to have a solution for HTTP-transmission out of the box.
There was a problem hiding this comment.
Hmmm, I'm quite torn about this.
One side of me thinks when on server side blazor, we should never use c# implemented transports because this risks the request being sent from server (API might change etc) is too high, and we can tune the performance more of the custom JS implementations, therefore removing the need for this large setup. The negotiation code should be quite short (we can look at the typescript client code), and I imagine the usage of it in the server sent event transport is quite minimal/easy to convert.
Though I also think that this could be useful in the future, for when we need to bodge some more functionality, or maybe someone wants to add a custom transport.
There was a problem hiding this comment.
I suggest to retain the current solution. When we ever need to, due to perfromance problems or other issues, we can investigate with replacing the current solution (partially) with a js-implemented one. This also does not bloat this PR with too many changes that are not actually necessary to implement the desired set of features.
There was a problem hiding this comment.
Via that argument, I would argue BlazorHttpMessageHandler is more bloat and unneeded for the actual features we need. It is going to become a large burden to maintain, as we will have to track the Blazor repo to see if any bugs etc are detected with the more or less copied implementation?
You can either remove it, or I can if you prefer.
I don't think this is a problem in our code but a bug in the BrowserConsoleLogger. I will file a bug there eventually. Edit: This is the issue: BlazorExtensions/Logging#28 |
Ah. I see the people over BlazorExtensions/Logging#28 have been rather unhelpful. |
|
|
||
| factory.Services.AddLogging(builder => builder | ||
| //.AddBrowserConsole() // Add Blazor.Extensions.Logging.BrowserConsoleLogger // This is not yet available for Blazor 0.8.0 https://github.com/BlazorExtensions/Logging/pull/22 | ||
| .AddBrowserConsole() |
There was a problem hiding this comment.
Did this work for you? I received runtime errors when adding in this manner
This is a prototype to make available the library server-side, running with Razor Components. The biggest issues are solved but it is not running server-side yet.
To support this, the IUriHelper needs to be injected as server, as it was needed with the js runtime on the blazor 0.9.0 update. This will incur breaking changes.
There are some code parts, that need to know whether it is running on blazor, this is currently done by checking for loaded assemblies (in particual if the Microsoft.AspNetCore.Mvc assembly is loaded). This is a temporary workaround only and need to be replaced. Maybe there is a way of checking whether we are running on Mono WASM or we let the user configure this in the options.
Fixes #23