[Blazor] `InputBase` - allow correct async handling of `ValueChanged` callbacks with `SetCurrentValueAsStringAsync()`
- Dominant language
- C#
- Stars
- 38.4k
- Forks
- 10.9k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 276
Description
Relates to #44105 and #54279, cc @MackinnonBuck @lewing @SteveSandersonMS
## Background and Motivation
In the current `InputBase` design, binding to the underlying `input` element is implemented using a synchronous setter (`CurrentValue_set`). The asynchronous `ValueChanged` callback is invoked from this synchronous setter as a "fire and forget" task (line 78):
https://github.com/dotnet/aspnetcore/blob/905557eaca32f5ce9fd17a25e5187ceb7a75cf1c/src/Components/Web/src/Forms/InputBase.cs#L69-L82
While there have already been some less common cases where this has caused issues (e.g., manual binding using the `Value`, `ValueChanged`, and `ValueExpression` parameters), the introduction of `@bind-Value:after` is likely to cause many more users to hit this issue. Developers use it much more frequently now (e.g., for continuous saving of form values).
Consider this example. The exception is "lost" - neither `blazor-error-ui`, `ErrorBoundary`, nor the browser/server console will capture it:
```razor
@page "/"
@**@
@code {
string model = string.Empty;
private Task DoSomethingAfterValueChanged()
{
Console.WriteLine("This executes.");
throw new InvalidOperationException("[1] This exception is lost in async-over-sync call from InputBase.CurrentValue_set.");
// Exception not logged in Console
// Exception not caught by ErrorBoundary
// Exception not caught by Blazor global error UI (the yellow strip of death :-D)
}
}
```
In contrast, a plain `input` HTML element behaves as expected.
I propose changes to `InputBase` and its derived components and enable proper asynchronous handling of the `ValueChanged` callback, including the `@bind-Value:after` variant.
## Proposed API
To resolve the async-over-sync issue in `InputBase` and its derived components, I propose to introduce a new `protected async Task SetCurrentValueAsStringAsync()` method. This method will encapsulate the logic currently sitting in the `set_CurrentValueAsString` property setter, including proper asynchronous invocation of `ValueChanged.InvokeAsync(Value)`.
```diff csharp
namespace Microsoft.AspNetCore.Components.Forms;
public abstract class InputBase : ComponentBase, IDisposable
{
+ protected async Task SetCurrentValueAsStringAsync(TValue value);
}
```
### Example Implementation
I already proposed a PR which implements this approach
* #54279
(The PR is a bit older - I’ll need to merge changes from `master`, do some polishing, and add E2E tests. I’ll elaborate on this if the new API gets approved.)
The `InputBase` will move the logic from `CurrentValueAsString` setter to the new method:
```csharp
protected async Task SetCurrentValueAsStringAsync(TValue value)
{
var hasChanged = !EqualityComparer.Default.Equals(value, Value);
if (hasChanged)
{
_parsingFailed = false;
Value = value;
await ValueChanged.InvokeAsync(Value);
EditContext?.NotifyFieldChanged(FieldIdentifier);
}
}
```
The existing `set_CurrentValueAsString` property setter will trigger this new method asynchronously without awaiting its completion (same as now, to preserve backwards compatibility):
```csharp
protected TValue? CurrentValue
{
get => Value;
set => _ = SetCurrentValueAsStringAsync(value);
}
```
#### Updates to Bindings:
All bindings that currently rely on `CurrentValueAsString` in `InputBase` will be updated to use `SetCurrentValueAsStringAsync()` for proper asynchronous handling. For example:
```csharp
builder.AddAttribute(11, "onchange", EventCallback.Factory.CreateBinder(
this,
async (string ___value) => await SetCurrentValueAsStringAsync(___value),
CurrentValueAsString
));
```
### Benefits
- **Asynchronous Handling**: The new approach ensures that `ValueChanged.InvokeAsync(Value)` is awaited, resolving the issue of exceptions being suppressed or "lost" in async-over-sync calls.
- **Backward Compatibility**: Existing third-party components depending on `CurrentValueAsString` will remain functional. The setter will continue to execute asynchronously as a "fire and forget" fallback when synchronous usage is unavoidable.
- **Error Propagation**: Exceptions thrown in the `ValueChanged` callback will now propagate correctly to Blazor error handling mechanisms (`ErrorBoundary`, `blazor-error-ui`, etc.).
## Usage Examples
The developers of input-components (mainly component libraries developers) are expected to adopt the new approach. Instead of using the `CurrentValueAsString` property setter, their `InputBase`-derived components will use this type of binding:
```csharp
builder.AddAttribute(11, "onchange", EventCallback.Factory.CreateBinder(
this,
async (string ___value) => await SetCurrentValueAsStringAsync(___value),
CurrentValueAsString
));
```
which is equivalent to
```razor
<... @bind-value:set="CurrentValue" @bind-value:get="SetCurrentValueAsStringAsync" ... />
```
## Alternative Designs
```razor
```
A brand new set of input components (`NewInputText`, `NewInputSelect`, etc.) could provide a clear separation for developers adopting the updated pattern while maintaining backward compatibility with the existing `Input*` components.
## Risks
As the primary goal of proposed approach is to preserve backwards compatibility and avoid breaking changes, the developers will still be able to use the synchronous `CurrentValueAsString` setter, where the issues described above remain unresolved.
Contributor guide
Assessment
This issue has not been assessed yet.