Use SignalR instead of client-side polling

[IEvangelist]

Note

This pull request is targeting a newly created signalr branch, that I plan on using for a new SignalR Learn module.

Summary of proposed changes in this PR:

• Use .Count property rather than function .Count().
• Add required NuGet packages for SignalR client and server projects.
• Introduce code-behind for OrderDetails component (remove the use @code { ... } directive)
  • OrderDetails.razor
  • OrderDetails.razor.cs
• Remove client polling for changes, instead use HubConnection.
• Simplify Marker by inheriting Point.
• Use top-level statements in server Program and use async initialization routine.
• Implement in-proc worker service to fake the backend processing of the order.
• Use a shared set of consts for communicating hub method names and event names (helps to alleviate misalignment with server and client).

Intended code for new Microsoft Learn module, also relates to dotnet/AspNetCore.Docs#23357

/cc @bradygaster @danroth27 @SteveSandersonMS

[SteveSandersonMS]

Thanks for suggesting these changes, @IEvangelist!

Do you have a sense of what impact these changes would have on all the written instructions for the various chapters under docs? I think the main determinant of whether we want to make a given design change to the code is what effect it has on the understandability of the steps people have to follow under docs (secondarily, a goal would be minimizing the concept count of the workshop, at least concepts not directly central to Blazor).

To merge the PR we'd need all the docs and all the save-points to be correspondingly updated. But that's a lot of work so before you get too deep into it, I'd recommend working out an overview of how impactful this would be on the docs. Ideally we'd be reducing the number of non-Blazor-specific steps they are carrying out, and the amount of non-Blazor-specific code concepts.

[SteveSandersonMS]

Oh wait, it looks like you're not aiming to merge into master, but rather are creating a different branch. You might want to delete all the Blazor-related docs and save-points from your signalr branch since they don't seem relevant to it. Not sure what your longer term plan is for the signalr branch and to what extent you intend to maintain it.

[IEvangelist]

Thanks for suggesting these changes, @IEvangelist!

Do you have a sense of what impact these changes would have on all the written instructions for the various chapters under docs? I think the main determinant of whether we want to make a given design change to the code is what effect it has on the understandability of the steps people have to follow under docs (secondarily, a goal would be minimizing the concept count of the workshop, at least concepts not directly central to Blazor).

To merge the PR we'd need all the docs and all the save-points to be correspondingly updated. But that's a lot of work so before you get too deep into it, I'd recommend working out an overview of how impactful this would be on the docs. Ideally we'd be reducing the number of non-Blazor-specific steps they are carrying out, and the amount of non-Blazor-specific code concepts.

Hi @SteveSandersonMS,

The idea was to have this be in its own branch, specific to signalr as an alternative to the client-side polling. I do not plan on having it impact any of the docs or existing instructions. After a few discussions it was decided that this was the cleanest approach for this. We want to keep the Learn module focused on Pizza APIs. Does that make sense, or would you propose an alternate approach?

[SteveSandersonMS]

Right yes, I just spotted that. If this doesn't affect master then you can basically do what you like 😄

[IEvangelist]

Right yes, I just spotted that. If this doesn't affect master then you can basically do what you like 😄

Sounds good, I still value your thoughts on the changes though. Would love a formal review.

[bradygaster]

Whilst I like any addition of SignalR and feel this does reflect a nice usage of "all our stuff" together, I'd question having forks in the docs unless you're thinking of eventually having something like a pivot in the doc. Options tend to confuse folks, so I want to make sure we're providing the RIGHT option.

@SteveSandersonMS do you have any objection to the actual implementation @IEvangelist is going with here? Is it more a concern of the implication on the docs and other content pieces or, is this a detraction from The Blazor Way (TM) and if so, is there a compromise?

[SteveSandersonMS]

@bradygaster We designed the blazor-workshop to teach people Blazor. To give people the best chance of really learning this stuff in a 2-day in-person workshop, it needs that focus. Even Blazor alone these days contains more than you can fully cover in 2 days, so minimizing the amount of adjacent concepts from ASP.NET Core is important.

My concern with including this in master is just that I don't know from this PR what impact it has on the workshop flow through the docs, because those changes aren't in this PR (and I know it will be a significant effort for @IEvangelist to go through every step in docs to update them and all the save-points). If it has no impact at all on docs, then in theory I'd be fine with SignalR/BackgroundService becoming some new implementation detail. However personally I'm a bit too focused on .NET 6 RC2/GA right now to dig too deeply into the new changes here and really investigate if there are drawbacks like potential bugs or security issues in any of the new code. I hope that sounds reasonable!

[IEvangelist]

Hi again @SteveSandersonMS and @bradygaster,

To be clear, I wasn't planning on targeting master. That is why we have a new branch named signalr, and I plan on having my SignalR Learn module point there instead. I'll make it clear that it's an adaptation of the existing Pizza app, focusing on refactoring the client-side polling into using SignalR. Perhaps depending on how well the module does, then we could reevaluate its value, and I could then propose updates to the docs to formally introduce the concepts here - post .NET 6.

Does that sound good?

[SteveSandersonMS]

@IEvangelist Using a separate branch is fine with me, like mentioned before.

[IEvangelist]

@IEvangelist Using a separate branch is fine with me, like mentioned before.

@SteveSandersonMS - Perfect, just need an approval to merge. Thanks