Support @bind:get, @bind:set, @bind:after

[javiercn]

Adds support for bind:get, bind:set, bind:after

This PR introduces a new syntax for bind that allows specifying an action to run after the binding or specifying the setter for the binding directly.

There are two companion PRs to this one:

• [Blazor] Add support for RazorLangVersion 7.0 sdk#24383 <- Adds support for RazorLangVersion 7.0
• [Blazor] Compiler runtime APIs needed for bind:get, bind:set, bind:after aspnetcore#40143 <- Adds the required runtime APIs to support the generated code.

There are in general three scenarios to cover:

• Binding to elements like div, button, etc.
• Binding to input elements like <input type=text />.
• Binding to components.

When binding to components the receiving property can either be an Action<T>, Func<T,Task>, Func<T,ValueTask> or an EventCallback<T> and we need to generate different code for each one of them.

When binding to elements we rely on new overloads of CreateBinder that take in an EventCallback<T> with the appropriate T for the elements that we know how to bind to in addition to the existing overloads that receive an Action<T>. For example, for T = bool?:

public static EventCallback<ChangeEventArgs> CreateBinder(
        this EventCallbackFactory factory,
        object receiver,
        EventCallback<bool?> setter,
        bool? existingValue,
        CultureInfo? culture = null)
    {
        return CreateBinderCore<bool?>(factory, receiver, setter, culture, ConvertToNullableBool);
    }

Within the compiler we take care of creating the EventCallback using a CreateInferredEventCallback helper when the developer uses bind:set or bind:after. For example:

            __builder.AddAttribute(2, "myevent", global::Microsoft.AspNetCore.Components.EventCallback.Factory.CreateBinder(this, global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.CreateInferredEventCallback(this, callback: ValueChanged, value: ParentValue), ParentValue));

and

            __builder.AddAttribute(2, "myevent", global::Microsoft.AspNetCore.Components.EventCallback.Factory.CreateBinder(this, global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.CreateInferredEventCallback(this, callback: __value => { ParentValue = __value; return global::Microsoft.AspNetCore.Components.EventCallback.Factory.Create(this, callback: DoSomething).InvokeAsync(); }, value: ParentValue), ParentValue));

For components we also have to deal with the cases where the component property is an Action<T> or a Func<T,<<awaitable>>> where <<awaitable>> follows the same rules as the C# compiler for determining if using await is supported and covers Task, ValueTask, and any future addition. In these cases we can't leverage CreateInferredCallback and instead we rely on two helper methods InvokeSynchronousDelegate and InvokeAsynchronousDelegate that are provided by the runtime. These two methods help us make possible to invoke the provided expression by the developer no matter whether it is a method group or a lambda and take care of producing an error when the developer tries to bind to an Action with a function returning an awaitable result.

For example, for:

namespace Test
{
    public class MyComponent : ComponentBase
    {
        [Parameter]
        public int Value { get; set; }
        [Parameter]
        public Func<int, Task> ValueChanged { get; set; }
    }
}
<MyComponent @bind-Value:get="ParentValue" @bind-Value:after="Update" />
@code {
    public int ParentValue { get; set; } = 42;
    public Task Update() => Task.CompletedTask;
}

we generate code like the one below:

            async __value => { ParentValue = __value; await global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.InvokeAsynchronousDelegate(Update); });

[SteveSandersonMS]

I have to admit I don't really have any sense of what this is for. Could you briefly summarise what the IPropertySymbol values here represent (e.g., where they come from) and what difference it makes whether we want to regard them as awaitable or not?

[javiercn]

We need for component bindings in the form of Value and ValueChanged where ValueChanged can be Action<T> or Func<T, Task>.

Unlike for other bindings when we can always create an EventCallback, in this case we can't (in the synchronous case at least). So what this method does is, when we are inspecting the ValueChanged property, (which we already know is a delegate) we check to see if it has a return type, and if that return type is awaitable, which covers Task, ValueTask, and any other potential awaitable in the future.

The logic in this method is the same that the roslyn compiler uses to make that determination.

We use this information during the bindloweringphase to generate either an Task returning callback or a void returning callback

[SteveSandersonMS]

I'd be interested to see how these helpers will look (e.g., their range of overloads). Does that info exist anywhere yet?

[javiercn]

https://github.com/dotnet/aspnetcore/pull/40143/files#diff-75ff7c1daf5ea067e9a29e6ddba5450cf62daab8ff131de1ed77bd2816478de7R55-R126

[SteveSandersonMS]

I'm not sure if breaking this over multiple lines is for an important reason, but it does add a lot of noise to the baseline diffs.

Is there any chance you could put this part of the code formatting back to how it was before, to minimize baseline churn, and then maybe do a separate PR (or at least an independent commit for it at the end of this PR after the code review is done)?

[javiercn]

It's quite painful to do this, instead I would suggest the following:

• Clone the PR:
  • run git diff origin/main --diff-filter=M -- 'src/Microsoft.AspNetCore.Razor.Language/test/TestFiles/IntegrationTests/**' to isolate these changes
  • run git diff origin/main --diff-filter=A -- 'src/Microsoft.AspNetCore.Razor.Language/test/TestFiles/IntegrationTests/**' to only look at new baselines

[SteveSandersonMS]

the way the PR/Commits are structured will make our reviewing life's miserable, so we need to discuss how best break down this PR for easier review

Yep, it is a bit of a challenge, but I think we can handle it. The main issue is just trying to keep the baseline diffs to a human-readable level. I suggest avoiding the existing test case name changes until right at the end when everything is done and signed-off, and similarly revert the change that splits up the generated CreateInferredEventCallback call over multiple lines until everything else is signed off.

Likewise, if you could avoid doing any squash-rebasing from here on, it will make it possible just to review the subsequent commits and not have to go back over the existing changes.

[SteveSandersonMS]

So far, this looks like a great enhancement!

[javiercn]

🆙 📅

[javiercn]

@SteveSandersonMS is there any concrete feedback on the PR you want us to discuss or are we good to move forward?

[mckaragoz]

Is there any article that explains a bit more? Don't we need to call ValueChanged anymore, or we will call in after method?

[boukenka]

@mckaragoz Please have a look at ASP.NET Core Blazor data binding