Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Covariant THub-Parameter for IHubContext #50705

Open
1 task done
angelaki opened this issue Sep 14, 2023 · 7 comments · May be fixed by #55600
Open
1 task done

Covariant THub-Parameter for IHubContext #50705

angelaki opened this issue Sep 14, 2023 · 7 comments · May be fixed by #55600
Labels
area-signalr Includes: SignalR clients and servers

Comments

@angelaki
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

Right now, the THub-Paramter of IHubContext<THub, T> isn't covariant what makes the following cast invalid:

IHubContext<ConcreteHub, IHubClient>? concrete = null;
IHubContext<Hub<IHubClient>, IHubClient>? loose = null;

loose = concrete;

I need this because I'd like to redirect the HubContext to my concrete's hub context without needing to know the concrete type for consumers:

serviceCollection.AddSingleton<IHubContext<Hub<IHubClient>, IHubClient>>(sp => sp.GetRequiredService<IHubContext<ConcreteHub, IHubClient>>());

This also maybe is a bit related to #41538? Is there a reason for not making it covariant? A soon fix would be awesome.

Describe the solution you'd like

Quite easy: public interface IHubContext<out THub, T>

Additional context

No response
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-signalr Includes: SignalR clients and servers label Sep 14, 2023
@BrennanConroy
Copy link
Member

BrennanConroy commented Sep 14, 2023 •
edited

More related to #30167 (comment). We added support for IHubContext<THub> -> IHubContext as well as making IHubContext<THub> -> IHubContext<Hub> work, but didn't do the same for IHubContext<THub, T> since it was a bit more complex and wasn't being explicitly asked for.

@angelaki
Copy link
Author

Oh wow, that actually already is a bit older. Any issues with just making it covariant? Guess that'd be the easiest solution?

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@angelaki
Copy link
Author

angelaki commented May 6, 2024

Sorry for pinging, but been a while. Any news on this?

@BrennanConroy
Copy link
Member

You're welcome to try it out and open a pull request if it works. We'd likely want a similar test to:

aspnetcore/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs

Line 4623 in da3aa27

public async Task CanSendThroughIHubContextBaseHub()

@angelaki angelaki linked a pull request May 8, 2024 that will close this issue
Open
@angelaki
Copy link
Author

angelaki commented May 8, 2024

I've created this pretty simple PR right now. Seems right to me, since the not generic version has a covariant parameter, too. But actually no idea what kind of test to implement for it? And ... tbh ... Guess I (or even my machine) won't make this super huge project build within a day.

@BrennanConroy
Copy link
Member

Basically copy the test in

aspnetcore/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs

Line 4623 in da3aa27

public async Task CanSendThroughIHubContextBaseHub()
but instead of using MethodHub, use HubT.

Guess I (or even my machine) won't make this super huge project build within a day.

You could try using CodeSpaces on GitHub which would build in the cloud.

@angelaki
Copy link
Author

Don't get me wrong, I really appreciate your effort helping me running this project. And maybe one day I'll become a great ASP.Net contributor ;) But Codespace won't let me choose a machine and I guess you have all that projects up and running? If you could add that requested test, I'd be super thankful :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-signalr Includes: SignalR clients and servers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Covariant THub-Parameter angelaki/aspnetcore
3 participants
@BrennanConroy @wtgodbe @angelaki