Aspnetcore: AddServerSizeBlazor should return a builder

Created on 12 Apr 2019  Â·  12Comments  Â·  Source: dotnet/aspnetcore

This will make it more seamless to wire up scale out providers and other potential SignalR options. Today to wire up the service in preview 4 it looks like this:

C# services.AddServerSideBlazor().AddSignalR().AddAzureSignalR();

Alternatively, if we don't want to expose the ISignalRBuilder then a more targeted builder might be useful if we want to add support for an AddAzureSignalR extension method anyways.

cc @rynowak @javiercn @SteveSandersonMS @danroth27 @bradygaster

Done area-blazor enhancement

Most helpful comment

Discussed this in person with @davidfowl and we came up with the following plan.

This is a balance of:

  • Having the correct branding
  • Not leaking real implementation details
  • Not hiding SignalR

note: The fact that we use SignalR isn't an implementation detail. Users need to know about it and care about it. However the fact that we use a type named BlazorHub is an implementation detail and we intend to hide it and make it non-public.

ConfigureServices

We will keep AddServerSideBlazor() and make it return it's own builder type. We'll accept callbacks of HubOptions so you can configure the hub. Configuring options specific to the hub we use is valuable.

We won't return the ISignalRServerBuilder. The other methods supported on that type are GLOBAL for all SignalR usage. We don't want to give the impression that those options only affect Blazor. Those other options are also things like adding protocols or redis scaleout. It's important that users understand SignalR to do that.

The AddAzureSignalR() gesture is the most important one, and it's going away.

Examples:

Vanilla

```C#
services.AddServerSideBlazor();

With Hub Options

```C#
services.AddServerSideBlazor(options =>
{
    options.MaximumReceiveMessageSize = long.MaxValue; // #YOLO
});

With Redis

C# services.AddServerSideBlazor(options => { options.MaximumReceiveMessageSize = long.MaxValue; // #YOLO }); services.AddSignalR().AddStackExchangeRedis();

Configure

We will add a marker interface to the convention builder returned by MapHub<>. SignalR and Server-Side-Blazor will each define it's own concrete type that applies this marker interface. This allows us to define unique functionality via extensions for vanilla-hubs, all signalr, and the blazor hub.

```C#
app.UseEndpoints(endpoints =>
{
endpoints.MapHub("/foo").DoABarrelRoll();
endpoints.MapServerSideBlazor().AddComponent("app");
});

All 12 comments

I think we want components to be and feel like something on their own, with SignalR being an "implementation detail", so that would clash a bit with that proposal.

If that is not the case anymore, then a separate builder would just do the trick. We can just return a custom builder and make it implement ISignalRBuilder so that those methods are available.

At that point, we can also just make MapBlazorHub() -> MapHub<BlazorHub>(BlazorHub.DefaultPath)

Do we like either of these?

```C#
services.AddServerSideBlazor().AddAzureSignalR();

```c#
services.AddServerSideBlazor();
services.AddSignalR().AddAzureSignalR();

I'm fine with both, but I like the second one more. I'm not a fan of wrapping these types of methods because it breaks composition and then:

  • You don't get things added for free when the underlying abstraction adds new options.
  • You are forced to expose options for all the things you combine.
  • You get to duplicate all things you want to expose.
  • When we version/update things this becomes a mess.

I prefer to strive for getting the design to a place where it can compose well with the rest of the pipeline, that way there is no duplication and things can evolve independently.

As an example of when this goes bad, just look at our past experience with Identity. We've had to tweak things on each minor release.

You can achieve all of those things you mention if you expose ISignalRBuilder or a type that implements that.

SignalR being an "implementation detail"

I don't think we should hide SignalR as an implementation detail when using Server-Side Blazor. SignalR dramatically changes how you scale your app, beyond just "recommending" Azure SignalR. I don't want users of Server-Side Blazor to be in the dark about that.

There's a purist part of me that kinda wants to force users (through marker service detection) to have both .AddServerSideBlazor and .AddSignalR (and have Server-Side Blazor throw an error, like MVC does, if .AddSignalR wasn't called). Now, I don't think that's the best customer experience though, so I can tell that purist side to sit down and shut up.

What about an approach that makes SignalR configuration a first-class part of .AddServerSideBlazor but doesn't require a single builder:

services.AddServerSideBlazor()
    .ConfigureSignalR(sR => sR.AddAzureSignalR());

That puts "SignalR" right in the configuration so that it's not hidden, but it also allows Server-Side Blazor to be abstracted a little from SignalR.

If that feels messy, I think I most prefer having the Blazor builder implement/wrap/whatever ISignalRBuilder so that the native configuration shows up.

I would be fine with option2 of what fowler suggested and also with getting rid of MapBlazorHub in favor of MapHub. It's other people the ones that need convincing. I love that its trivial to plug in server-side blazor and that we shouldn't hide signalr at all.

I kinda wonder if it's makes sense to lean into SignalR + Blazor branding more. We're now using the term Blazor for both models. Does it make sense to say Server Side Blazor or SignalR Blazor or BlazR or even BlzR

This is all to do with branding.

Right now I prefer to see two lines of code because we haven't tried this pattern anywhere else. Need to do some thinking about this.

SignalR Blazor

Ok, so maybe it's because I've spent a lot of time working on it and have a fondness for it, but that really clicks for me.

ideas are happening

I’m not convinced that would help clarity. “SignalR Blazor” raises as many questions as it answers. It equally suggests “client-side Blazor that connects to a SignalR hub for whatever reason”.

There are lots of things that are different between the server and client hosting models. The term “server-side Blazor” encompasses all those and is the one thing customers have understood so far.

As for the Startup.cs code, I think either of Fowler’s options would be fine! I agree it’s good to maximise composability and not hide the SignalR config.
Steve


From: Andrew Stanton-Nurse notifications@github.com
Sent: Saturday, April 13, 2019 4:07:33 AM
To: aspnet/AspNetCore
Cc: Steve Sanderson; Mention
Subject: Re: [aspnet/AspNetCore] AddServerSizeBlazor should return a builder that derives from ISignalRBuilder (#9316)

SignalR Blazor

Ok, so maybe it's because I've spent a lot of time working on it and have a fondness for it, but that really clicks for me.

[ideas are happening]https://camo.githubusercontent.com/0ce2cd0d27ed1c485743867b93e6753a65430ee5/68747470733a2f2f6d656469612e67697068792e636f6d2f6d656469612f64796445536576454f716f3962444f5342462f67697068792e676966

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com/aspnet/AspNetCore/issues/9316#issuecomment-482771136, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABDOMlC4kcdHuXXa4PgrQ-c7xNGPYvY0ks5vgUn1gaJpZM4crgJp.

Here's another example of whart happens when you hide the hub but don't expose a way to configure it:
https://github.com/aspnet/AspNetCore/issues/9570

I don't think we should hide the hub, endpoints.MapHub<BlazorHub>(BlazorHub.DefaultPath) looks clean enough IMO to be on the templates and discovery is not a problem given that its already on the initial template.

Thats my take, based on the fact that the same thing happens with Identity all the time.

Discussed this in person with @davidfowl and we came up with the following plan.

This is a balance of:

  • Having the correct branding
  • Not leaking real implementation details
  • Not hiding SignalR

note: The fact that we use SignalR isn't an implementation detail. Users need to know about it and care about it. However the fact that we use a type named BlazorHub is an implementation detail and we intend to hide it and make it non-public.

ConfigureServices

We will keep AddServerSideBlazor() and make it return it's own builder type. We'll accept callbacks of HubOptions so you can configure the hub. Configuring options specific to the hub we use is valuable.

We won't return the ISignalRServerBuilder. The other methods supported on that type are GLOBAL for all SignalR usage. We don't want to give the impression that those options only affect Blazor. Those other options are also things like adding protocols or redis scaleout. It's important that users understand SignalR to do that.

The AddAzureSignalR() gesture is the most important one, and it's going away.

Examples:

Vanilla

```C#
services.AddServerSideBlazor();

With Hub Options

```C#
services.AddServerSideBlazor(options =>
{
    options.MaximumReceiveMessageSize = long.MaxValue; // #YOLO
});

With Redis

C# services.AddServerSideBlazor(options => { options.MaximumReceiveMessageSize = long.MaxValue; // #YOLO }); services.AddSignalR().AddStackExchangeRedis();

Configure

We will add a marker interface to the convention builder returned by MapHub<>. SignalR and Server-Side-Blazor will each define it's own concrete type that applies this marker interface. This allows us to define unique functionality via extensions for vanilla-hubs, all signalr, and the blazor hub.

```C#
app.UseEndpoints(endpoints =>
{
endpoints.MapHub("/foo").DoABarrelRoll();
endpoints.MapServerSideBlazor().AddComponent("app");
});

Was this page helpful?
0 / 5 - 0 ratings

Related issues

groogiam picture groogiam  Â·  3Comments

githubgitgit picture githubgitgit  Â·  3Comments

Kevenvz picture Kevenvz  Â·  3Comments

rbanks54 picture rbanks54  Â·  3Comments

ipinak picture ipinak  Â·  3Comments