added core 3.0 signalr #14369

bradygaster · 2019-09-17T22:36:37Z

included all the stuff for signalr 3core 3.0. cc @BrennanConroy @anurse @halter73 @RickAndMSFT

halter73 · 2019-09-17T23:23:14Z

aspnetcore/release-notes/aspnetcore-3.0.md

-@bradygaster  to provide
+The SignalR JavaScript client has changed from being `@aspnet/signalr` to `@microsoft/signalr`. To react to this change, you will need to change your references in `package.json` files, require statements, and ECMAScript import statements. 
+
+In the JavaScript and .NET Clients for SignalR, we've added support for automatic reconnection via the `withAutomaticReconnect()` method on the `HubConnectionBuilder`. By default, the client will try to reconnect immediately and after 2, 10, and 30 seconds. Enlisting in automatic reconnect is opt-in, but simple via this new method.


It might be nice to call out that this will give you a new ConnectionId which is different from the OG SignalR clients. Maybe something like the following would be good.

Suggested change

In the JavaScript and .NET Clients for SignalR, we've added support for automatic reconnection via the `withAutomaticReconnect()` method on the `HubConnectionBuilder`. By default, the client will try to reconnect immediately and after 2, 10, and 30 seconds. Enlisting in automatic reconnect is opt-in, but simple via this new method.

In the JavaScript and .NET Clients for SignalR, we've added support for automatic reconnection via the `withAutomaticReconnect()` method on the `HubConnectionBuilder`. By default, the client will try to reconnect immediately and retry after 2, 10, and 30 seconds if necessary. If the client successfully reconnects, it will be given a new connection ID. Enlisting in automatic reconnect is opt-in, but simple via this new method.

@halter73 I'll migrate your suggestion over to #14371

halter73 · 2019-09-17T23:31:24Z

aspnetcore/release-notes/aspnetcore-3.0.md

+        if (IsUserAllowedToDoThis(resource.HubMethodName, context.User.Identity.Name) &&
+            context.User != null &&
+            context.User.Identity != null &&
+            context.User.Identity.Name.EndsWith("@jabbr.net", StringComparison.OrdinalIgnoreCase))


All this null checking seems unnecessary since the context.User.Identity.Name passed into IsUserAllowedToDoThis above would have already nullrefed at this point if there was no user set anyway.

BTW

context.User != null && context.User.Identity != null && context.User.Identity.Name.EndsWith("@jabbr.net", StringComparison.OrdinalIgnoreCase)

and

context.User?.Identity?.Name.EndsWith("@jabbr.net", StringComparison.OrdinalIgnoreCase) == true

are equivalent expressions, but the latter might be a little too cute for a doc sample.

The best approach is probably to add exit early if no user name is set. Ex:

if (context.User?.Identity?.Name == null) { return Task.CompletedTask; }

Then you can remove the other null checking.

@halter73 I'll stop working on #14371 if you can move over to #14371. That way I can accept your suggestion commits without copy/paste.

halter73 · 2019-09-17T23:41:49Z

aspnetcore/release-notes/aspnetcore-3.0.md

+}
+```
+
+Now, individual Hub methods can be decorated with the name of the policy the code will need to check at run-time. As clients attempt to call individual Hub methods, the `DomainRestrictedRequirement` handler will run and control access to the methods. Based on the way the `DomainRestrictedRequirement` controls access, all logged-in users should be able to call the `SendMessage` method, only users who’ve logged in with a `@jabbr.net` email address will be able to view users’ histories, and – with the exception of `[email protected]` – will be able to ban users from the chat room.


I think only having one user who cannot ban other users is unusual/unexpected making this sample somewhat harder to follow. I think it would be a little bit clearer if [email protected] was the only user who could ban users.

halter73 · 2019-09-17T23:52:41Z

aspnetcore/release-notes/aspnetcore-3.0.md

+            // process content
+        }
+    }
+}


I would use the new IAsyncEnumerable sample here, since it's simpler than the ChannelReader version:

https://github.com/aspnet/AspNetCore.Docs/blob/4aa859cb075f3b1b250aeccfcbcabd5e810b974e/aspnetcore/signalr/streaming.md#L88-L106

@halter73 can you make that change in a commit on #14371 ?

halter73 · 2019-09-17T23:54:31Z

aspnetcore/release-notes/aspnetcore-3.0.md

+}
+```
+
+Clients would use the SignalR `Subject` (or an RxJS Subject) as an argument to the `streamContent` parameter of the Hub method above.


Should we mention that with the C# client you can just pass an IAsyncEnumerable or ChannerReader as an argument?

yes - can you make this or want @Rick-Anderson or i to wordsmith it?

I already did this in #14371

* Update aspnetcore-3.0.md * copy halter suggestion over * PR feedback * edits

Rick-Anderson · 2019-09-18T03:41:34Z

@halter73 if any of these comments didn't make it into #14371 you can make commits directly against #14250

BrennanConroy · 2019-09-18T15:16:08Z

aspnetcore/release-notes/aspnetcore-3.0.md

+
+ASP.NET Core 3.0 SignalR added:
+
+* Streaming support, which enables streaming return values from server-side methods. This is useful for when fragments of data will come in over a period of time.


This is not new in 3.0...

This is not new in 3.0...

@BrennanConroy I'll remove that from #14240. This has been merged into #14240 so leave comments in #14240. I'll remove the following from ASP.NET Core 3.0 SignalR added: to
## Generic Host

That is, the remaining SignalR content.

ack. i globbed server-to-client in there by accident. @Rick-Anderson - you got this change or need me to hop on it?

* What's new in 3.0 * wrok * wrok * work * work * work * Add Razor components bit * Nits * Fix a link * work * work * work * Edit pass on what's new in 3.0 doc (#14355) * Add additional coverage * added core 3.0 signalr (#14369) * added core 3.0 signalr * PR of Bradys PR (#14371) * Update aspnetcore-3.0.md * copy halter suggestion over * PR feedback * edits * minor tweaks * work * react to anurse * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * react to feedback * Update aspnetcore-3.0.md (#14403) * Update aspnetcore-3.0.md * Update aspnetcore-3.0.md * nothing new * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Daniel Roth <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Daniel Roth <[email protected]> * react to feedback * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Daniel Roth <[email protected]> * react to feedback * Add option for RCL and note default is for RC dev * Blazor!!!!!!!! WOOT! * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Stephen Halter <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Stephen Halter <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * Update aspnetcore/release-notes/aspnetcore-3.0.md Co-Authored-By: Andrew Stanton-Nurse <[email protected]> * react to feedback * react to feedback * react to feedback * Add ANCM updates (#14425) * Add ANCM updates * Update aspnetcore-3.0.md * react to feedback * work * work * Apply suggestions from code review Co-Authored-By: Daniel Roth <[email protected]> * react to dan roth suggestions * Updates + add a blurb on enhanced endpoint routing

added core 3.0 signalr

c576433

Rick-Anderson mentioned this pull request Sep 17, 2019

Update 22-to-30.md #14370

Merged

halter73 reviewed Sep 17, 2019

View reviewed changes

PR of Bradys PR (#14371)

3820a21

* Update aspnetcore-3.0.md * copy halter suggestion over * PR feedback * edits

Rick-Anderson merged commit 33d6dc1 into whats/new/3.0/ra Sep 18, 2019

Rick-Anderson deleted the whats/new/3.0/bradygaster branch September 18, 2019 03:41

BrennanConroy reviewed Sep 18, 2019

View reviewed changes


		ASP.NET Core 3.0 SignalR added:

		* Streaming support, which enables streaming return values from server-side methods. This is useful for when fragments of data will come in over a period of time.

added core 3.0 signalr #14369

added core 3.0 signalr #14369

Uh oh!

Conversation

bradygaster commented Sep 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

halter73 Sep 17, 2019 • 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!

halter73 Sep 17, 2019 • 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!

Rick-Anderson commented Sep 18, 2019

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

halter73 Sep 17, 2019 •

edited

Loading

halter73 Sep 17, 2019 •

edited

Loading