Skip to content

Conversation

@halter73
Copy link
Member

We should delete SingleClientProxy<THub> in favor of SingleClientProxyWithInvoke<THub> for simplicity if nothing else. We should also switch to T Single(string connectionId) => Client(connectionId); because that now works in our case. We wouldn't even have to implement it.

I see the inconsistent experience with strongly-typed vs. non-strongly-typed hubs as a complete non-issue. It's FAR better to be "inconsistent" here! We're already being inconsistent by returning the exact same type from both Single() and Client() in the strongly-typed case instead of two different types (namely ISingleClientProxy and IClientProxy) in the default case.

I understand that we can't change IHubClients : IHubClients<IClientProxy> to IHubClients : IHubClients<ISingleClientProxy> in the default case without breaking apps. It's unfortunate, but if we could do that, we wouldn't have had to introduced Single() in the first place.

However, that doesn't mean we should nerf strongly-typed hubs! So what if they are more flexible with their Client property? Single() and Client() return the exact same type for strong typed hubs. It's already different.

Follow up to #41763 (comment). Hopefully the change and the tests make it clear what I'm trying to fix.

@halter73 halter73 requested a review from davidfowl May 31, 2022 17:21
@halter73 halter73 requested a review from BrennanConroy as a code owner May 31, 2022 17:21
@ghost ghost added the area-signalr Includes: SignalR clients and servers label May 31, 2022
@halter73 halter73 changed the title SignalR Allow returns from all single client proxies Allow returns from all single client proxies May 31, 2022
@MackinnonBuck
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@halter73 halter73 merged commit a6b1101 into main Jun 2, 2022
@halter73 halter73 deleted the halter73/less-proxies branch June 2, 2022 01:22
@ghost ghost added this to the 7.0-preview6 milestone Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-signalr Includes: SignalR clients and servers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants