Elmish.WPF icon indicating copy to clipboard operation
Elmish.WPF copied to clipboard

Multi-message binding

Open TysonMN opened this issue 4 years ago • 11 comments
trafficstars

Sometimes I would like if a binding could send multiple messages.

Here are three approaches that come to mind for how to handle multiple messages.

Specific composite message

let update = function
  | A -> handleA
  | B -> handleB
  | AandB -> handlA >> handleB

let bindings () =
  [ "A" |> Binding.cmd A
    "B" |> Binding.cmd B
    "AandB" |> Binding.cmd AandB ]

Generic composite message

let rec update = function
  | A -> handleA
  | B -> handleB
  | CompositeMsg msgs -> msgs |> List.map update |> List.fold (>>) id

let bindings () =
  [ "A" |> Binding.cmd A
    "B" |> Binding.cmd B
    "AandB" |> Binding.cmd (CompositeMsg [ A; B ]) ]

First-class binding support

This approach would involve slightly expanding our binding API. (I will ignore the wrapDispatch argument in the following example.) For example, one of the overloads of Binding.cmd is https://github.com/elmish/Elmish.WPF/blob/7d841f20983020fbe1cb841abbb1ffac87eb94a4/src/Elmish.WPF/Binding.fs#L2402-L2403

We could add an overload that accepts an argument of type 'msg list. Then it would be possible to write

let update = function
  | A -> handleA
  | B -> handleB

let bindings () =
  [ "A" |> Binding.cmd A
    "B" |> Binding.cmd B
    "AandB" |> Binding.cmd [ A; B ] ]

TysonMN avatar Jan 20 '21 18:01 TysonMN

Furthermore, the advantage of the last case increases when considering the more complicated variant of update that also returns the Cmds to execute.

TysonMN avatar Jan 20 '21 18:01 TysonMN

That last approach would also make it possible to send no messages, which is also something I want to do sometimes. My current workaround is to create a message called NoOp and have update return the same model in that case.

TysonMN avatar Jan 20 '21 20:01 TysonMN

Sounds like a good idea to properly support this. Feel free to work on it!

cmeeren avatar Jan 20 '21 21:01 cmeeren

I would be happy to help on this. Shall we duplicate all API returning 'msg to have an overload returning 'msg list? Do you want to go for a breaking change and only have the list API? Or were you thinking about something a little more clever?

Evangelink avatar Aug 14 '21 20:08 Evangelink

My plan is for an easy change. Mostly just a couple compostable functions. I will reply more here and elsewhere within the next couple days.

TysonMN avatar Aug 14 '21 21:08 TysonMN

Do you want to go for a breaking change and only have the list API?

No. I have already introduced one breaking change in v4, which is the removal of the optional wrapDispatch parameter from every method. I would prefer to not introduce any others. There are methods I want to axe, but my plan is to keep them in v4 but with the Obsolete attribute to indicate that they are deprecated.

In v3, the only binding API is method-based. In v4, my plan is to have both a method-based binding API and a function-based API. The advantage of methods is that they can be overloaded, which allows there to be fewer names as well as shorter/simpler names. The advantage of function is that the compiler errors are better when something is wrong. In contrast, a complier error involving an overloaded method is almost useless. It often says something like "here are all the overloads, but it is not clear which one you are trying to use". And since functions cannot be overloaded, each one needs a unique name.

My plan for v4 is to have both a function-based API and a method-based API and to use each to its strength. First, everything that is possible will be possible via the function-based API. Second, every method will be implemented my calling one or more functions from the function-based API. Also no method will call another method for two reasons: (1) if a user is having a problem calling a particular method (and the complier error is not helpful), then the user can copy-paste the implementation of that method and now have code that will only give helpful compiler errors because it only uses the function-based API; (2) it is easier to verify that the function-based API exposes everything that is possible.

Shall we duplicate all API returning 'msg to have an overload returning 'msg list?

Definitely not! I already made that mistake with the wrapDisaptch feature in v3.

Instead, the goal is to add such crosscutting features like that via a composable API. For example, see how such a tiny commit added such a high-impact improvement (and as I look at that commit, I see how I could have made it smaller by extracting parts that can be expressed by a behavior preserving transformation...i.e. a refactor).

Or were you thinking about something a little more clever?

To the API, the smallest change to add support for this is to add

Binding.mapMsgsWithModel

with the type signature

'msg list -> 'model -> 'msg list

(or maybe with those inputs swapped, I haven't decided which order to use yet).

Does that all make sense?

TysonMN avatar Aug 15 '21 14:08 TysonMN

I also meant to explicitly say...

I still like idea of a method-based API both because it is more terse and because it can help improve discoverability.

TysonMN avatar Aug 15 '21 15:08 TysonMN

Not sure I get the full picture. I see where (haven't looked yet at the how) to put the Binding.mapMsgsWithModel but I am not sure how to plug that back to what you call the method-based API without adding a new overload for each member.

Evangelink avatar Aug 15 '21 15:08 Evangelink

Initially, no changes to the method-based API

TysonMN avatar Aug 15 '21 16:08 TysonMN

(To repeat myself and/or add to what I said and/or summarize what I said)

I want to expose everything that is possible via the function-based binding API. Then we can add overloads to the method-based binding API to succinctly express common patterns. Simply put, I want the method-based API to be syntactic sugar.

In particular, I think we could be rather opinionated in the method-based API. For example, in all method overloads that create a SubModel binding, we could add laziness based on reference equality (of the model). This will likely both maintain correctness and improve performance in most cases. The two cases in which it might not preserve correctness is if the model is mutable or if this sub-model contains a command that is sometimes not executable based on a command parameter (because that parameter is stored in the UI, so it is not part of a reference equality check of the model).

In short, when adding new features, just focus on the function-based API.

TysonMN avatar Aug 15 '21 19:08 TysonMN

(Meta: @Evangelink, I believe that I have now replied or processed all of your recent communication. If you are still expecting a reply from me somewhere, just let me know.)

TysonMN avatar Aug 15 '21 19:08 TysonMN