Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design proposal: Bind get/set/after modifiers #39837

Closed
SteveSandersonMS opened this issue Jan 28, 2022 · 14 comments
Closed

Design proposal: Bind get/set/after modifiers #39837

SteveSandersonMS opened this issue Jan 28, 2022 · 14 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components design-proposal This issue represents a design proposal for a different issue, linked in the description enhancement This issue represents an ask for new feature or an enhancement to an existing one
Milestone

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jan 28, 2022

Summary

Provides ways of customizing how Blazor's @bind reads and writes a value, solving several common problems at once.

Motivation and goals

After thinking through the "multiple event handlers" proposal (#39815), I've started to think the real problem we should solve is quite different. That is:

  • I'm not convinced we need multiple event handlers.
    • All the problems that people reported - in which they thought they wanted multiple overlapping event handlers - were really problems about @bind (see below).
    • And other SPA frameworks with attribute-like events haven't found it necessary to support the multiple same-named ones either (react, vue).
  • There are in fact some real problems with @bind today:
    A. No safe way to run async logic after a binding updates (#22394). This is a basic scenario even beginners will hit early.
    B. The magic consistency mechanism built into @bind (essential for Blazor Server apps) can't be used if you're implementing a value/onchange pair manually (#17281 #38520). This is a potential severe gotcha you may not notice until in production.
    C. When building a component with a Value/ValueChanged pair, there are two ways to do it, but neither are safe:
    • If you do a @value/@onchange pair manually, you lose consistency guarantees (can lose keystrokes on Server)
    • Or, if you @bind to a property with a get/set pair, you have to discard the Task returned by ValueChanged.InvokeAsync

In scope

  • Provide a safe and easy way to run async logic after a binding has updated
  • Provide a safe and easy way to implement a component with a Value/ValueChanged pair

These two directly address A and C above. If these two are done, I think B is solved implicitly because there's no longer any reason to want to do a manual value/onchange pair. If the behavior you want is conceptually a two-way binding, then @bind would now always have as much power as you need.

Also, I'm keen to do this in an idiomatically Blazory way. For example, there should not be some kind of new BindableValue that you have to instantiate at runtime. Instead, typical components should be built only with basic C# types and patterns (e.g., string and Func<string, Task>).

Out of scope

  • Although the design should clearly guide people to use @bind safely (that is, in a way that won't drop keystrokes in async update cases like Blazor Server), I don't think we can realistically stop people from using extra powers to do something misguided.

Proposed solution

There are really two distinct cases to deal with. I've considered extensively designs that would solve both using only a single concept, but didn't feel that any of them led to enough elegance or convenience. So in the end I'm proposing two different but closely-related new mechanisms.

bind:after

To handle the "run async logic after binding", consider a @bind:after directive attribute:

<input @bind="searchText" @bind:after="PerformSearch" />

@code {
    string searchText;

    async Task PerformSearch()
    {
        // ... do something asynchronously with 'searchText' ...
    }
}

I really like @bind:after because it's so basic. Any beginner can understand this. Even just seeing its name in autocompletion is probably enough to work out how to use it. This solves problem A in a super direct way.

It's also very good for guiding people not to break the safety mechanism for not losing keystrokes in Blazor Server, since your custom logic doesn't run until we've already assigned the new value to searchText synchronously - all existing guarantees remain. Presumably we'd invoke PerformSearch before the initial re-render, and as usual, have it trigger a further re-render asynchronously. This would happen automatically if the Razor compiler generates an event handler that does the assignment and then returns the task given by EventCallback.Factory.Create(yourBindAfterExpression).InvokeAsync().

bind:get and bind:set

This one's less obvious, but for components that take a Value/ValueChanged pair, you'd be able to take the scary code you have to write today:

<input value="@Value" @onchange="@(EventCallback.Factory.CreateBinder<TValue>(this, _ => ValueChanged.InvokeAsync(_), Value))" />

@code {
    [Parameter] public TValue Value { get; set; }
    [Parameter] public EventCallback<TValue> ValueChanged { get; set; }
}

... and simplify it to this:

<input @bind:get="@Value" @bind:set="@ValueChanged" />

@code {
    [Parameter] public TValue Value { get; set; }
    [Parameter] public EventCallback<TValue> ValueChanged { get; set; }
}

With benefits:

  • Simpler code
  • Now you also get binding's automatic formatting features on the value= side
  • Doesn't risk losing keystrokes like the old one did (and of course it can still preserve the task returned from ValueChanged)

The behavior of course is that you're controlling parts of the code that @bind generates on both the reading and writing sides. The difference between @bind:set and @bind:after is that @bind:after does the value writing for you before calling your code, whereas @bind:set just calls your code (and passes the new value, having gone through @bind's built-in parsing logic).

Questions you may have:

  • Why is it @bind:get+@bind:set and not just @bind+@bind:set?
    • Because if you see <input @bind="@val" @bind:set="@MyMethod" /> often, it creates confusion:
      • It looks as if the @bind:set is what makes it a two-way binding, and that you could make it one-way by removing that. Whereas in fact that would be wrong (you'd still have a two-way binding, just one that behaves differently now).
      • It looks as if it would be equivalent to write <input value="@val" @bind:set="@MyMethod />, and it almost is, but not quite because the formatting logic would differ. Much better not to create the ambiguity and have one correct solution.
    • We can avoid the above problems by having a compiler rule that @bind:get and @bind:set must always be used as a pair - you can't just have one of them and not the other (nor can you have them with @bind). So none of the weird cases will arise.
  • Couldn't you use @bind:set to achieve (in effect) @bind:after, and hence we don't need @bind:after?
    • Theoretically yes. You could @bind:set to a method that writes to your field and then runs your async logic. However, this is far less obvious for newcomers, and is less convenient in common cases. And it invites mistakes: if you do something async before setting the field, the UI will temporarily revert and generally behave badly. So it's valuable to have @bind:after for convenience and to guide correct usage. We can regard @bind:get/@bind:set as a more advanced case mainly for people implementing bindable components, as it gives them a really clean and safe solution, and such developers are advanced enough to understand that they just shouldn't do async work before calling ValueChanged.
  • Can you use all three at once, e.g., <input @bind:get="@value" @bind:set="@MyMethod" @bind:after="@DoStuff" />?
    • Sure, why not? I think that the generated logic should await MyMethod before calling DoStuff, since "after" feels like it means "after all the work involved in calling set". It's an edge case but I can't think of any problems this will cause nor any major increase in implementation cost.
  • Do other @bind modifiers like @bind:event and @bind:format work with this?
    • Yes, and that's partly why it's a big improvement over manual value/onchange pairs.

Risks / unknowns

Depends on the exact design chosen.

In the above design, it's increasing the conceptual surface area of @bind a bit and introducing new rules about which combinations of modifiers can be used together. I don't think this is a killer problem because the compiler can enforce all this cleanly. However it is potentially more for people to learn. Mitigation: only tell most people about @bind:after, as that's the most widely needed and is super easy to understand.

Examples

See above

@SteveSandersonMS SteveSandersonMS added the design-proposal This issue represents a design proposal for a different issue, linked in the description label Jan 28, 2022
@SteveSandersonMS SteveSandersonMS added area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one labels Jan 28, 2022
@SteveSandersonMS SteveSandersonMS added this to the 7.0-preview2 milestone Jan 28, 2022
@pranavkm
Copy link
Contributor

@Bind:set="@ValueChanged"

Perhaps @bind:onset since it's an event as opposed to a straight up setter unlike the getter? But I like this proposal a whole lot better.

@mkArtakMSFT
Copy link
Member

@Bind:set="@ValueChanged"

Perhaps @bind:onset since it's an event as opposed to a straight up setter unlike the getter? But I like this proposal a whole lot better.

It'll also make it easier to understand that it's a handler, but will break the "get/set" combination, which is also easy to remember. Choosing between easy to remember and intuitive / help me understand what's going on here - I'd chose the latter. So, @bind:get/@bind:onset seems fine to me.

@javiercn
Copy link
Member

I somewhat dislike onset since its a valid word in English and it's ambiguous as what we really mean is "on set". I do think get, set are more comprehensible and succinct. That's my take.

@javiercn
Copy link
Member

Do other @bind modifiers like @bind:event and @bind:format work with this?

  • Yes, and that's partly why it's a big improvement over manual value/onchange pairs.

Does bind:set need to receive information about the binding to do its job? (format for example). I'm trying to understand a bit better how this thing composes. It would be ideal if even when you decide to provide @bind:set you get to reuse other pieces of functionality like format.

@javiercn
Copy link
Member

@SteveSandersonMS overall a very solid proposal, I like the direction this goes in an the fact that people can reuse our custom bind "reconciliation" logic.

@SteveSandersonMS
Copy link
Member Author

SteveSandersonMS commented Jan 31, 2022

I somewhat dislike onset since its a valid word in English and it's ambiguous as what we really mean is "on set". I do think get, set are more comprehensible and succinct. That's my take.

I agree. I don't really see onset as being "more accurate" since it's equally possible to confuse (sounds like the underlying DOM event is called set rather than change/input/etc). I prefer get/set as that's the more naturally memorable name based on existing conventions. Your point about onset being a valid word in its own right is a good one too.

Does bind:set need to receive information about the binding to do its job? (format for example).

I have to actually implement it to get to 100% solid certainty, but from what I anticipate, no. My intent is that the bind:set callback receives the value already parsed to the inferred type T, so we've already finished using the format. The callback just replaces the part of the existing logic where we do the actual assignment. You're exactly correct to understand the intent here is that we're building on existing parts of the binding flow, which is one of the advantages over using a manual value/onchange pair.

@smartprogrammer93
Copy link

I love this proposal. Not kidding, huge improvement. Typical of @SteveSandersonMS .

@Dreamescaper
Copy link

Will I be able to use :after (and others) with components?

<MyComponent @bind-SomeValue="_value" @bind-SomeValue:after="HandleValue" />

@javiercn
Copy link
Member

Will I be able to use :after (and others) with components?

Yes

@szalapski
Copy link

Really like this proposal.

@mrpmorris
Copy link

:after is nice!

@mkArtakMSFT mkArtakMSFT removed this from the 7.0-preview3 milestone May 23, 2022
javiercn added a commit to dotnet/razor-compiler that referenced this issue Jun 21, 2022
Adds support for `@bind:get, @Bind:set, @Bind:after` to the Razor Compiler.

See dotnet/aspnetcore#39837 for details
@javiercn
Copy link
Member

@vgb1993
Copy link

vgb1993 commented Jul 8, 2022

Nice!

How about using bind:before instead of bind:set? I did find it a bit missleading

@SteveSandersonMS
Copy link
Member Author

@vgb1993 We don't think that will be such a common requirement, and for people who do need that, it's important that they use @bind:set because they must assign the value synchronously, even if they then do something async after. This is mentioned above, though I appreciate it is not emphasised.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components design-proposal This issue represents a design proposal for a different issue, linked in the description enhancement This issue represents an ask for new feature or an enhancement to an existing one
Projects
None yet
Development

No branches or pull requests

10 participants