[Blazor] `InputBase` - allow correct async handling of `ValueChanged` callbacks with `SetCurrentValueAsStringAsync()`

[hakenr]

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):

aspnetcore/src/Components/Web/src/Forms/InputBase.cs

Lines 69 to 82 in 905557e

	protected TValue? CurrentValue
	{
	get => Value;
	set
	{
	var hasChanged = !EqualityComparer<TValue>.Default.Equals(value, Value);
	if (hasChanged)
	{
	Value = value;
	_ = ValueChanged.InvokeAsync(Value);
	EditContext?.NotifyFieldChanged(FieldIdentifier);
	}
	}
	}

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:

@page "/"

<EditForm Model="model">
    <ErrorBoundary>
        <InputText @bind-Value="model" @bind-Value:after="DoSomethingAfterValueChanged" />
    </ErrorBoundary>
    @*<input type="text" @bind-value="model" @bind-value:after="DoSomethingAfterValueChanged" />*@
</EditForm>

@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).

namespace Microsoft.AspNetCore.Components.Forms;

public abstract class InputBase<TValue> : ComponentBase, IDisposable
{
+  protected async Task SetCurrentValueAsStringAsync(TValue value);
}

Example Implementation

I already proposed a PR which implements this approach

• [Blazor] InputBase - SetCurrentValueAsync + SetCurrentValueAsStringAsync (fix for #44105) #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:

protected async Task SetCurrentValueAsStringAsync(TValue value)
{
    var hasChanged = !EqualityComparer<TValue>.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):

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:

builder.AddAttribute(11, "onchange", EventCallback.Factory.CreateBinder<string>(
    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:

builder.AddAttribute(11, "onchange", EventCallback.Factory.CreateBinder<string>(
    this, 
    async (string ___value) => await SetCurrentValueAsStringAsync(___value), 
    CurrentValueAsString
));

which is equivalent to

<... @bind-value:set="CurrentValue" @bind-value:get="SetCurrentValueAsStringAsync" ... />

Alternative Designs

<NewInputText @bind-Value="model" @bind-Value:after="DoSomethingAsync" />

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.

[javiercn]

@hakenr thanks for putting this forward.

I think the proposal looks reasonable. I think we can't avoid having the setter perform the Fire and Forget, and we should update it to dispatch the error to the renderer via DispatchExceptionAsync, which it currently doesn't.

It might be interesting to see if there is any functional difference between doing that and using SetCurrentValueAsStringAsync. The main difference that I can already tell is that NotifyFieldChanged will behave differently depending on whether SetCurrentValueAsStringAsync is used or the property setter is used.

In the first case, NotifyFieldChanged will execute after the callback has completed, while in the latter case it will always execute immediately, whether or not InvokeAsync perform asynchronous work.

This might be a breaking behavior, so we need to think a bit about how this impacts things like @bind. I'm worried about edge cases where the values might get out of sync with the DOM in unpredictable ways. To make an extreme case for this, imagine the callback performs 1s of work before NotifyFieldChanged is triggered.

That seems like a meaningful difference between the two models.

By all means I'm supportive of this change, I think it's a good change, however, we need to be careful with the behavior changes that we introduce by it.

I think a good starting point would be to wrap the call to InvokeAsync inside a try..catch block with DispatchExceptionAsync to see if that achieves the outcome that we want.

If that's not good enough for some reason, I think we should test the SetCurrentValueAsStringAsync and see if there are any binding problems if we call NotifyFieldChanged after a long callback (simulated for example, with Task.Delay) and determine if there are binding issues when we fire events from the browser at a rapid pace. If we find no problems, I think we can implement this behavior, but I think in the worst case we might need to consider if we need to make this behavior opt-in. The reason for this, is again, that it can change rendering behavior in unpredictable ways (any side-effect that NotifyFieldChanged could cause, would be part of a different render batch, which might break your app rendering logic).

[hakenr]

I understand your concern about the NotifyFieldChanged outcome being processed in a separate render batch. However, isn’t this the way it should have been working all along? While we can address the exception behavior using DispatchExceptionAsync, the underlying "dirty" sync-over-async approach will remain, and we’ll never make it fully "correct."

Now consider this scenario: If someone creates an @bind-Value:after handler that modifies the Model in a way that resolves potential invalid values (e.g., normalizing a property like a ZIP code), should validations triggered by NotifyFieldChanged act on the original model or the updated, normalized version?

Take this simple example:

<EditForm Model="model">
	<DataAnnotationsValidator />
	<ValidationSummary />
	<InputText @bind-Value="model.Name" @bind-Value:after="HandleValueChanged" />
	<button type="submit">Submit</button>
</EditForm>

@code {
	private Model model = new Model();

	private async Task HandleValueChanged()
	{
		await Task.Delay(1000); // Simulate some async work
		if (string.IsNullOrWhiteSpace(model.Name))
		{
			model.Name = "Default name";
		}
	}

	private class Model
	{
		[Required]
		public string Name { get; set; } = "Initial name";
	}
}

What should happen when the user clears the input field and submits the form?

Currently, this results in the following state:

Here’s an animation of the behavior:

Do we want to keep this or do we want the validations to act after the ValueChanged finishes?
If we want to validate immediately, why not to put the NotifyFieldChanged in front of await ValueChanged to get consistent behavior for sync and async? (Which would be also a breaking change as with the current state, the validation acts on fixed model when the ValueChanged handler is synchronous.)

UPDATE:
If you comment out the await Task.Delay(..) from the above sample (making the callback complete synchronously), the outcome is:

[hakenr]

(What I'm trying to say: It is not only about the exceptions, but there are more issues caused by the sync-over-async/fire-and-forget.)

[javiercn]

@hakenr thanks for the additional details.

We've discussed this further in the team and we think we prefer starting with handling the exception via DispatchExceptionAsync. the reason for it, is that we have identified other cases that will produce issues when the callback is asynchronous.

One such example is if your callback takes long enough, and another render gets triggered in between for some other reason. The value in the form will revert to the old value.

Making this change would not only require us to make this change, but we would also need to do work on all our components to make sure that they behave correctly in this situation.

We think that handling the error and dispatching it properly addresses the main issue that we had with the sync over async bit, and we would be open to taking a more limited fix.

Despite the outcome, we want to thank you for bringing this forward. It took us about 1h of discussion within the team to understand the challenges that this subtle behavior change introduces, and in the end, we prefer to be conservative and avoid breaking people.

[javiercn]

Moving this out of triage, but feel free to update the PR if you wish to do so and we will be happy to take in the more scoped fix.

[hakenr]

@javiercn Thanks for taking the time to discuss this. I understand your concerns about changing the behavior of existing components and the extra work required to cover all scenarios.

Unfortunately, the proposed solution of handling and dispatching the exception still follows the fire-and-forget approach, without giving derived components the opportunity to handle it more effectively.

Did you consider an option where the default processing remains synchronous (fire-and-forget, as it is now) while introducing an asynchronous alternative (such as SetCurrentValueAsStringAsync()) to allow derived components to take responsibility and handle it asynchronously?

(Unfortunately, I don't align with the currently proposed solution of "handling and dispatching the exception only," as it doesn't fix the problem but merely patches one of its symptoms. I’m not willing to invest my effort in such a pull request or put my name behind it, sorry.)