Blazor example uses `async void` instead of `async Task`.
ScarletKuro opened this issue · 2 comments
Description
This code section
<div class="card bg-light mb-3" style="width:30rem">
<div @onclick="Toggle" class="card-header">
<h2 class="card-title">Toggle (<code>Expanded</code> = @expanded)</h2>
</div>
@if (expanded)
{
<div class="card-body">
<p class="card-text">@ChildContent</p>
</div>
}
</div>
@code {
[Parameter]
public bool Expanded { get; set; }
[Parameter]
public EventCallback<bool> ExpandedChanged { get; set; }
[Parameter]
public RenderFragment? ChildContent { get; set; }
private bool expanded;
protected override void OnParametersSet() => expanded = Expanded;
private async void Toggle()
{
expanded = !expanded;
await ExpandedChanged.InvokeAsync(expanded);
}
}
Uses async void Toggle instead of async Task Toggle, even though the @onclick="Toggle" event handler supports Task methods as well.
This is considered a code smell, and we should avoid providing bad examples, as async void should generally be avoided in most cases.
Therefore, it should be replaced with private async Task Toggle() or private async Task ToggleAsync().
Page URL
Content source URL
Document ID
e687c953-409d-49b9-664a-19ca1f836e4b
Article author
🏂🎁 Happy Holidays! ❄️⛄
Stand-by! ... A green dinosaur 🦖 will be along shortly to assist.
Hello @ScarletKuro ... This is just a coding mistake here. We normally don't use async void for event handlers that await tasks.
I'll wager without even looking that due to the quantity of examples that we've accumulated over the years that we probably have a few other lurking 😈 around the docs. Therefore, I'll fix this up and perform a sweep of the whole doc set (and the sample apps repo) to find and update any other instances.
I also think it would be a good idea to remark on this subject in the Event Handlers article. For event handlers that don't await tasks, my understanding from the product unit is that it's fine to use async void. I doubt that many examples exist of that use case. All we currently say in the article is that returning tasks from event handlers is supported. That's not enough coverage. I'd like to clarify this subject with a new section in the article.
Thanks for writing in on this. I'll schedule this for work, fix this code example right now, and then work the rest of the issue next week (I hope! We're all bugging out for the ⛄ soon!).