Skip to content

Trim Async suffix on SignalR Hub method names #7554

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.

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

Closed
halter73 opened this issue Feb 13, 2019 · 5 comments
Closed

Trim Async suffix on SignalR Hub method names #7554

halter73 opened this issue Feb 13, 2019 · 5 comments
Assignees
Labels
area-signalr Includes: SignalR clients and servers breaking-change This issue / pr will introduce a breaking change, when resolved / merged.

Comments

@halter73
Copy link
Member

See #7420

@muratg We'll probably want to do this sooner than later since it's breaking.

@halter73 halter73 added breaking-change This issue / pr will introduce a breaking change, when resolved / merged. area-signalr Includes: SignalR clients and servers labels Feb 13, 2019
@muratg
Copy link
Contributor

muratg commented Feb 13, 2019

Thanks @halter73. @bradygaster FYI.

@halter73 halter73 self-assigned this Feb 20, 2019
@halter73 halter73 added this to the 3.0.0-preview4 milestone Feb 20, 2019
@analogrelay
Copy link
Contributor

Should we do this in SignalR? I don't think it makes the same kind of sense here, since SignalR is RPC, but MVC is a URL pattern.

In MVC, it makes a lot of sense, because you may have an action like:

public class CustomersController
{
    public async Task GetAllAsync()
    {
    }
}

And you want to access it at /Customers/GetAll, not /Customers/GetAllAsync. This makes total sense to me.

However, in SignalR you are calling a method, rather than sending a request to a URL. So if you have a Hub method:

public class CustomersHub
{
    public async Task GetAllAsync()
    {
    }
}

The code you want to write on the .NET client would definitely be:

var customers = /* Get Hub Connection */;
// Assume we had perfect client generation, etc.
var customers = await customers.GetAllAsync();

Stripping Async from the method name doesn't make as much sense there for me.

I realize it's not a perfect analogy, since JavaScript, Java and C++ are all viable clients and don't have this pattern. However, in this case, I think we should not be stripping suffixes from method names since the user is defining a method which will be called like a method rather than defining a method that ends up generating a URL pattern.

@halter73
Copy link
Member Author

halter73 commented Apr 9, 2019

Good point @anurse about RPC. The real benefit of this change seems to be related to the default way MVC does routing based on action names.

@halter73
Copy link
Member Author

halter73 commented Apr 9, 2019

If we were to do this at all, the Async suffix probably should be optional, but I'd rather keep things simple and have one true name for every hub method.

@analogrelay
Copy link
Contributor

Triage Decision: This doesn't make sense for SignalR, let's close.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-signalr Includes: SignalR clients and servers breaking-change This issue / pr will introduce a breaking change, when resolved / merged.
Projects
None yet
Development

No branches or pull requests

3 participants