fix: Form validation happens before FluentSelect's bind-SelectedOption sets a new value

[AntonZhernosek]

🐛 Bug Report

When part of an edit form with validation, if the edit form contains a FluentSelect with bind-SelectedOption provided, property validation is triggered before being updated by the fluent select, potentially causing validation failure

💻 Repro or Code Sample

Here's a simple code repo:

1. Create a new blazor project
2. Package dependencies are
   • Microsoft.FluentUI.AspNetCore.Components >= 4.11.4
   • Blazored.FluentValidation - latest (2.2.0)
3. Program.cs includes standard FluentUI config

Code sample for Home.razor

@page "/"
@rendermode InteractiveServer

@using Blazored.FluentValidation
@using FluentValidation
@using Microsoft.FluentUI.AspNetCore.Components

<EditForm Model="@EditModel">
    <FluentValidationValidator />

    <FluentSelect TOption="Property1"
                    Required=@true
                    Label="Property 1"
                    Items=@property1Choices
                    Placeholder="Select..."
                    Appearance="Appearance.Filled"
                    Position="SelectPosition.Below"
                    OptionValue="@(x => x.Id.ToString())"
                    OptionText="@(x => x.Name)"
                    @bind-SelectedOption=@EditModel.Property1>
    </FluentSelect>

    <FluentValidationMessage For="@(() => EditModel.Property1)" />

    <FluentSelect TOption="Property2"
                    Required=@true
                    Label="Property 2"
                    Items=@property2Choices
                    Placeholder="Select..."
                    Appearance="Appearance.Filled"
                    Position="SelectPosition.Below"
                    OptionValue="@(x => x.Id.ToString())"
                    OptionText="@(x => x.Name)"
                    @bind-SelectedOption=@EditModel.Property2>
    </FluentSelect>

    <FluentValidationMessage For="@(() => EditModel.Property2)" />
</EditForm>

@code {
    private Model EditModel { get; set; } = new();

    private List<Property1> property1Choices =
    [
        new()
        {
            Id = 1,
            Name = "First"
        },
        new()
        {
            Id = 2,
            Name = "Second"
        },
    ];

    private List<Property2> property2Choices =
    [
        new()
        {
            Id = 1,
            Name = "First"
        },
        new()
        {
            Id = 2,
            Name = "Second"
        },
    ];

    public sealed class Model
    {
        public Property1? Property1 { get; set; }
        public Property2? Property2 { get; set; } 
    }

    public sealed class Property1
    {
        public required int Id { get; set; }
        public required string Name { get; set; }
    }

    public sealed class Property2
    {
        public required int Id { get; set; }
        public required string Name { get; set; }
    }

    public sealed class ModelValidator : AbstractValidator<Model>
    {
        public ModelValidator()
        {
            RuleFor(x => x.Property1)
                .NotNull()
                .WithMessage("Property 1 should not be null!");

            RuleFor(x => x.Property2)
                .NotNull()
                .WithMessage("Property 2 should not be null!");
        }
    }
}

🤔 Expected Behavior

I select a new option, the field has a green outline and the model is valid.

😯 Current Behavior

If I select one of the options in a fluent select, the field will turn red and a validation failure message will appear. If I keep selecting again, the message will disappear.

If I get access to the edit context and debug from OnValidationStateChanged event handler, you'll see that validation occurs on the previously selected value, not the current one. In this case, validation occurs on a null value, even if I set a new value from the select dropdown

💁 Possible Solution

Didn't have time to look into the source code or recent changes. The issue does not happen with package version 4.11.3 and happens with 4.11.4 or greater.

🌍 Your Environment

Happens on windows and in standard aspnet debian container, Net9, library version 4.11.4+

[joriverm]

maybe related to my PR of #3294 ?
maybe worth testing with the demo before and after commit 894a143

[AntonZhernosek]

maybe related to my PR of #3294 ?

Had a look at this and the logic, as well as doing some more testing. It appears that the PR actually fixes an issue around this example. The actual issue seems to come from a concurrency problem.

Before the PR, the FieldIdentifier for the selected option used to be the SelectedOption property itself, and not the model being edited. With the PR changes, the FieldIdentifier now comes from the expression that can be provided by bind-SelectedOption. It actually means that EditContext is now properly bound. As an example, you can attach this to the validator in my original post

            RuleFor(x => x.Property1)
                .Must(_ => false)
                .WithMessage("Always fails");

This will not produce a validation failure on 4.11.3, and does produce a failure on 4.11.4 (as it should actually)

There are 2 more useful diagnostics that I can attach to this code.

@inject ILogger<Home> Logger;

    protected override void OnParametersSet()
    {
        EditContext.OnFieldChanged += OnFieldChanged;
        base.OnParametersSet();
    }

    private void OnFieldChanged(object? sender, FieldChangedEventArgs args)
    {
        Logger.LogInformation("Field changed");
    }

Using this to log when the EditContext has received a field change notification.

    public sealed class Model
    {
        public Property1? Property1
        {
            get;
            set
            {
                Console.WriteLine("");
                Console.WriteLine("Property 1 set");
                Console.WriteLine("");

                field = value;
            }
        }

        public Property2? Property2
        {
            get;
            set
            {
                Console.WriteLine("");
                Console.WriteLine("Property 2 set");
                Console.WriteLine("");

                field = value;
            }
        }
    }

And then this to log when the edited model's property are actually being set.

If you debug with this setup, you'll see that there are 3 field change events being fired, and the actual property is being set afterwards.

Now, if I use this little class over here over the actual FluentSelect, you'll see that the property setter is called before any field notifications are sent, and the validator works as expected in the original post (the field is green and there's no validation failure)

public sealed class CustomSelect<TOption> : FluentSelect<TOption>
	where TOption : notnull
{
	protected override async Task ChangeHandlerAsync(ChangeEventArgs e)
	{
		await Task.Delay(500);
		await base.ChangeHandlerAsync(e);
	}
}

So, actually, the issue is that ChangeHandlerAsync happens first/too fast, before the SelectedOptionChanged has a chance to finish. Haven't tested this, but it might be relevant for everything else that inherits ListComponentBase too

[joriverm]

@AntonZhernosek : just wondering, is that Task.Delay in your custom control needed to reproduce the issue? its just that i noticed #3455 5 minutes ago :)

[AntonZhernosek]

@AntonZhernosek : just wondering, is that Task.Delay in your custom control needed to reproduce the issue? its just that i noticed #3455 5 minutes ago :)

Adding a delay to the call fixes the issue, not reproduces it.

The logic for any component that is bound to a property is supposed to be

1. Set the new value to the property
2. Fire off EditContext.FieldChanged()

For FluentSelect, and I assume for any other component deriving for it, it's the other way around. CustomSelect from my example above adds delay to step no.2, so that step no.1 finishes execution before it. There's no other relevance that this component poses, it's completely for debugging purposes.

It might be worth a try to create an empty override with no inlining on ChangeHandlerAsync at the base component to see if it helps, or wrapping the call to it into InvokeAsync, to see if this fixes the execution priority

[vnbaaij]

The outcome is that they are turned around. It is not actually so in code. FluentSelect (as all off the List Components) is derived from FluentInputBase. So ChangeHanlerAsync is called first. I don't know what is happening that it doesn't actually picks up the value immediately then.

[AntonZhernosek]

This actually makes a lot of sense than.

FluentSelect uses ChangeHandlerAsync for onchange event. This gets triggered first, which internally compares the new value with the old one, sets the value and makes a call to EditContext.NotifyFieldChanged. This seems to trigger a few times before SelectedOptionChanged actually gets awaited.

After SelectedOptionChanged, ChangeHandlerAsync is invoked again. However, at this point, FluentInputBase.Value would be the same as the value that FluentSelect.InternalValue, because it's already been invoked before from onchange event.

Without messing with any other internal state management, I think this issue could be fixed by making an additional call to EditContent.NotifyFieldChanged after calling ChangeHandlerAsync in ListComponentBase.RaiseChangedEventsAsync.

[vnbaaij]

In the PR mentioned, I fixed it by adding an override for ChangeHanlerAsync in ListComponentBase and give it a small delay. So just like you did in solving it in your own code.

[AntonZhernosek]

Sorry, didn't see the previous PR because I think I was typing out a response literally as it was submitted lol.

I think as a production solution this could still be a problem (I've only used it for debugging purposes), because someone (for some reason) could still create a longer-running operation as SelectedOptionChanged handler. So if the time it takes for it to run is longer than this delay, than you'd still get the same issue.

After looking at the code a bit more yesterday, I think a full fix would be to add this at the bottom of ListComponentBase.RaiseChangedEventsAsync

        if (FieldBound)
        {
            EditContext?.NotifyFieldChanged(FieldIdentifier);
        }

I think this problem happens because FluentSelect uses ChangeHandlerAsync as the onchange event handler, which executes before ListComponentBase.OnSelectedItemChangedHandlerAsync. So calling ChangeHandlerAsync a second time from RaiseChangedEventsAsync doesn't result in NotifyFieldChanged being called