Aspnetcore: [Discussion] Async suffix for controller action names will be trimmed by default

[pranavkm]

As part of addressing https://github.com/aspnet/AspNetCore/issues/4849, ASP.NET Core MVC will trim the suffix Async from action names by default. This affects routing and link generation.

Consider
```C#
public class ProductController : Controller
{
public async Task ListAsync()
{
var model = await DbContext.Products.ToListAsync();
return View(model);
}
}

Prior to 3.0, the action will be routeable via `Product/ListAsync`. Link generation would require specifying the `Async` suffix e.g.

`<a asp-controller="Product" asp-action="ListAsync">List</a>`

In 3.0, the action will be routeable via `Product/List` and link generation would require not specifying the `Async` suffix e.g.

`<a asp-controller="Product" asp-action="List">List</a>`

This change does not affect names specified using the `ActionNameAttribute`. This behavior can be disabled by setting `MvcOptions.SuppressAsyncSuffixInActionNames` to `false` as part of the application startup:

```C#
services.AddMvc(options =>
{
   options.SuppressAsyncSuffixInActionNames = false; 
});

[sgjsakura]

Tip: In your code sample you missed the Task wrapper for return type.

Maybe you should also update the code sample in the reference.

Best wishes~

[manigandham]

Why? Adding magic to change names seems like a source of bugs. There's no reason to use the async suffix in the first place for all of the actions.

[poke]

@manigandham See the linked issue for details. The code fix to make a method asynchronous adds the Async suffix to the method name.

Note that there's not really any magic going on. The generated action name will just omit “Async” at the end. And remember that the framework already does something similar for controller names.

[grprakash]

Why? Adding magic to change names seems like a source of bugs. There's no reason to use the async suffix in the first place for all of the actions.

Meaningful conventions are not magic. Its been very useful in situations like "Controller", "Attribute" etc

[Genbox]

In case there is an Index and IndexAsync action, which one will take precedence?

[manigandham]

Wouldn't it be better to update the code fix itself? The async suffix convention seems outdated since async is the majority and actions aren't called by users directly anyway.

[Morcatko]

I agree with @manigandham. I don't believe .NET (or any framework in general) should be driven by "bugs" in its code fixes, but the other way.
Another thing - this fixes code fix vs route issue, but breaks case when someone uses nameof(method) (for whatever reason)

[BlueMarmalade]

what if you have two methods with the same names; one is async and one is not? Isn't it strange to have to rename the synchronous method to something completely different? (But maybe i'm just not getting it ;P)

[poke]

@BlueMarmalade If you already have two separate actions with the same name but one is ~Async, then I would highly rethink your design because that seems to be a rather bad idea to begin with.

As for what happens if you have both, I would assume that they both take the same route and as such the first action will win.

[TsengSR]

@poke

As for what happens if you have both, I would assume that they both take the same route and as such the first action will win.

I'd expect an AmbiguousActionException, which we got before when two actions did match the same route, i.e. two get methods which only differ in query parameters

@BlueMarmalade

what if you have two methods with the same names; one is async and one is not? Isn't it strange to have to rename the synchronous method to something completely different? (But maybe i'm just not getting it ;P)
I assume you can still do that, by explicitly adding an

    [HttpGet("IndexAsync")]
    public async Task<IActionResult> IndexAsync() { .. }

    [HttpGet]
    public async Task<IActionResult> Index() { .. }

to have it explictly. As of today, you had to do the exact opposite to reach the goal

    [HttpGet("index")]
    public async Task<IActionResult> IndexAsync() { .. }

which is counter intuitive since most actions will be async.

[terrajobst]

Wouldn't it be better to update the code fix itself? The async suffix convention seems outdated since async is the majority and actions aren't called by users directly anyway.

No. In fact, we're talking about adding more sync overloads for APIs that were 100% async before. Async is viral and not all code can be async for practical reasons.

On top, even if new tech could be a 100% async, the overwhelming majority of APIs and techologies are sync. It's an important part of the developer experience to have consistency. We found that consistency across multiple technologies is way more important than a local optimum. The reason being that most developers have to write code in various areas and not having to deal with different (or even conflicting) guidance is important.

I agree with the issue opener: MVC should ignore the Async suffix.

[NickCraver]

FWIW, while I agree with the behavior here is likely the wanted one most of the time, I just hit in in practice on some sample apps that had MyAction _and_ MyActionAsync. If this was compile-time or start-up time that'd be one thing...but I'm only seeing errors only at runtime. For example:

Samples.AspNetCore.Controllers.TestController.DuplicatedQueriesAsync (Samples.AspNetCore3)
Samples.AspNetCore.Controllers.TestController.DuplicatedQueries (Samples.AspNetCore3)
   at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ReportAmbiguity(CandidateState[] candidateState)
   at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.ProcessFinalCandidates(HttpContext httpContext, CandidateState[] candidateState)
   at Microsoft.AspNetCore.Routing.Matching.DefaultEndpointSelector.Select(HttpContext httpContext, CandidateState[] candidateState)
   at Microsoft.AspNetCore.Routing.Matching.DfaMatcher.MatchAsync(HttpContext httpContext)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)

Can we improve this 2-matching-routes scenario (which worked before and now breaks) by alerting the user in some way before having to specifically hit one of the duplicate routes at runtime and debug what's breaking?

[NickCraver]

In addition to the duplicate scenario above, this simple case breaks:

<a asp-controller="Test" asp-action="MyActionAsync">Link Text</a>

c# public async Task<IActionResult> MyActionAsync() { }

Why should this break? It's only being handled on one side of the fence. And again, users will only find this out at runtime and only if they go test every route.

[pflannery]

services.AddMvc(options => { options.SuppressAsyncSuffixInActionNames = false; });

Having this option wouldn't hurt I just don't think it should be imposed out of the box.
I think this would be better as an opt-in flag like options.RemoveAsyncSuffixFromActionNames = true.

The original issue is easily resolved by changing return View() to return View("Index").
Either that or OP can drop the suffix "Async" from the method name all together.

People should be left to their own naming conventions without interruption.

Strongly typed areas, controllers and views

There are similar issues that have been around a while that I think also need addressing with the UrlHelper and ControllerBase.

• Trim the "Controller" suffix from the controller name so we can use the nameof operator.
  i.e. UrlHelper.Url(nameof(SomeController.ActionName), nameof(SomeController))
• Ability to determine area routing using the controller AreaAttribute.
  i.e. UrlHelper.Url<SomeController>(nameof(SomeController.ActionName))

Also view resolution:

• Have the option to generate View class names without the "folder location" prefixes to allow us to use nameof for strongly typed resolution. i.e. return View(nameof(Views.Index))

[terrajobst]

People should be left to their own naming conventions without interruption.

I disagree. The default path ("pit of success") should generate in solutions that follow the standard .NET naming conventions. Of course people should be able to enable whatever naming conventions they want. But I assert that following the common conventions shouldn't require configuration changes because we shouldn't penalize folks for doing the right thing. A successful framework cannot be neutral w.r.t naming conventions. The value is in consistency.

[wagich]

As @pflannery mentions, this turns out to be a massive headache when using nameof() to "strongly" type action and controller names, which I (used) to do everywhere.
It's much nicer being able to refactor controllers and actions without having to use a bunch of consts or even shudder sprinkling strings everywhere…

I'm very much not opposed to the new behavior, but it really should do the same transformation on passed-in names when constructing a path/url.

[terrajobst]

@wagich

I'm very much not opposed to the new behavior, but it really should do the same transformation on passed-in names when constructing a path/url.

That one I agree with.

[artkrv]

My teammate waitsted entire day on migration 2.2 --> 3.1 because of this issue.

[BlueMarmalade]

i still don't see the point of this change. i'm sure alot of apps have hard coded paths to certain controller methods. changing it like this is just one of those changes that tries to make things easier by hiding/removing stuff by default, but it has little purpose and actually makes things more confusing if anything.

[Cammeritz]

coded paths to certain controller methods.

Yeah, we wasted two days finding out what the issue was while trying to fix the problem in 100 different ways. We multiple Applications that listen to those paths and now we had to change them all.

Also, this did not work for us:

services.AddMvc(options =>
{
   options.SuppressAsyncSuffixInActionNames = false; 
});

Great move Microsoft 👏

[Cammeritz]

Also, this did not work for us:

Okay, so now for some weird reason it works. but it's still a decision that i think is quiet stupid.

But for some reason it only works if we use it together with the AddRazorPages method.

services.AddRazorPages().AddMvcOptions(options => 
{
  options.EnableEndpointRouting = false;
  options.SuppressAsyncSuffixInActionNames = false;
});

[PillowMetal]

Migrated from 2.2 to 3.1 and had an Ajax call to an action where both the Ajax URL and the target action had an Async suffix but was getting a 404 error. Thankfully it only took me 2 hours to figure out the suffix was being stripped (by eventually noticing that calling other action names worked fine and so did my desired action when I gave it the same name). Even with the headache it caused, I do like the consistency in behavior between this new stripping of action suffixes and the current stripping of controller suffixes, especially with the proliferation of Async suffixes. But I agree with the others that now this same transformation has to happen with nameof.

[mkArtakMSFT]

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

[Luk164]

Why is this not mentioned anywhere in the documentation? I searched for hours to get to this issue. Also Visual Studio intelliSense does not pick this up for some reason. If I omit the Async in the asp-action it says that it is not found. I would appreciate a better warining in documentation and an intelliSense fix.

[poke]

@Luk164 It is mentioned in the 2.2 to 3.0 migration guide.

[Luk164]

@poke Thx, though I do not know anyone who checks old migration guides when starting a development. A simple warning in the startup or controller creation guides would suffice.

[justdmitry]

With this "feature" it's now harder to write "good" code, it makes more space for using magic strings.

When generating links for user for different purposes (redirect, link in emails etc):

Instead of

LinkGenerator.GetUriByAction(nameof(PassController.GetApplePassAsync), nameof(PassController), someRouteValues...

I should write now:

LinkGenerator.GetUriByAction("GetApplePass", "Pass", someRouteValues...

Is it possible to "fix" UrlHelper / LinkGenerator to trim "Controller" and "Async" suffixes too?

[mkArtakMSFT]

@pranavkm what are your thoughts regarding this:

Is it possible to "fix" UrlHelper / LinkGenerator to trim "Controller" and "Async" suffixes too?

[pranavkm]

Is it possible to "fix" UrlHelper / LinkGenerator to trim "Controller" and "Async" suffixes too?

I think that's what we vaguely decided on part of https://github.com/dotnet/aspnetcore/issues/14103.