Blazored / FluentValidation

A library for using FluentValidation with Blazor
https://blazored.github.io/FluentValidation/
MIT License
588 stars 84 forks source link

Validation occurs before binding completes when using a custom Razor component whose custom binding needs to call JSInterop #79

Closed mikequ-taggysoft closed 3 years ago

mikequ-taggysoft commented 3 years ago

I encountered this issuing when using this IntlTelInput component, which is a Blazor wrapper for intl-tel-input, a popular phone number validator js library.

The IntlTelInput Razor component wraps a basic input component like this:

<input @onchange="OnChange" type="tel" @ref="_telInput"/>

private async void OnChange (ChangeEventArgs e)
{
    CurrentValue = await _intlTelInputJsInterop.GetData(_inputIndex);
    if (CurrentValue is not null)
    {
        await _intlTelInputJsInterop.SetNumber(_inputIndex, CurrentValue.Number);
     }
}

And I use the IntlTelInput component itself in an EditForm like this:

<EditForm EditContext="_editContext" OnValidSubmit="OnValidSubmit" OnInvalidSubmit="OnInvalidSubmit">

    <FluentValidationValidator DisableAssemblyScanning="@true" />
    <ValidationSummary/>

    <IntlTelInput @bind-Value="_model.IntTelNumber"/>

    <button class="btn-primary">Submit</button>

</EditForm>

Note that the bound property model.IntTelNumber is not a string , but a custom type IntlTel. It is constructed by the JSInterop call as shown earlier. The object then exposes a Number property (for the phone number) and a IsValid property which represents the result of the number validation by intl-tel-input. My FluentValidation code in turn uses this IsValid property like this:

 RuleFor(x => x.IntTelNumber).Cascade(CascadeMode.Stop)
                .NotNull()
                .WithMessage("Please enter a phone number")
                .Must( x => x.IsValid)
                .WithMessage("Invalid phone number")

So here's the problem:

Whenever a user enters a number, and immediately click the submit button, the OnChange event handler in the Razor component triggers, which is supposed to call JSInterop and assign the generated object to model.IntTelNumber. But before this completes, the FluentValidation code already triggers, and at this time, it sees IntTelNumber as null, and therefore this validation fails.

After the JSInterop call completes, the FluentValidation code appears to trigger again, this time validating the property property.

The more bizarre part is, on a desktop browser, the first "failed validation" occurs so fast (can be only seen while debugging) that in the actual EditForm, OnValidSubmit is actually triggered. So everything turns out fine and the form validates normally.

But on any mobile browser, OnInvalidSubmit is triggered instead. The result is that the first time user enters the phone number and hits the submit button, the form does not submit. But if the user simply hits the button again, it validates property and submits the form.

I wonder if there is anything that can be done to mitigate this issue. Any advice would be greatly appreciated.

chrissainty commented 3 years ago

I believe this issue is related to the fact that Blazor Forms don't support async validation (see #38). As all interop is performed asynchronously I don't see how this will work. Just to be clear, this is a limitation in Blazor not in this library.

mikequ-taggysoft commented 3 years ago

@chrissainty Ahh I did read about that issue. I thought it only affected validation rules like MustAsync etc. Didn't think about the async onchange handler itself.

Any idea why desktop browsers would end up treating the form as valid while mobile browsers don't?

chrissainty commented 3 years ago

I'm not sure, that is an oddity.

I'm going to close this issue for now as I don't think there is anything we can do here and the async limitation is cover by #38

Liero commented 3 years ago

@chrissainty: this is not related to async validation.

@mikequ-taggysoft: the problem is, that the OnChange handler should be async Task and not async void. You must open issue in the https://github.com/R0landas/intl-tel-input-blazor project

mikequ-taggysoft commented 3 years ago

@Liero Yeah I did try changing that and it actually did not resolve this particular issue.

Liero commented 3 years ago

@mikequ-taggysoft: oh, I see why. It should be something like:

CurrentValue = await _intlTelInputJsInterop.SetNumber(_inputIndex, CurrentValue.Number);

It is not optimal, but it is _intlTelInputJsInterop's ussye.