Closed cmeeren closed 5 years ago
I'll quote the steps you listed, and then discuss them:
It seems to me that there are two different kinds of events happening, which are being conflated into a single event called UpdateText
. One kind of update is informative; it means "the text field was updated by some process, and I am informing you of its new value". The other kind is imperative; it means "you should now update the text field to contain this". What if these two were not conflated? I.e., what if the informative message was called TextUpdated
and the imperative message looked like UpdateText
, so that those steps looked like:
Everything looks the name, except that step 7 has vanished. As long as the UpdateText
message does not trigger a TextUpdated
message, there is no loop. And why should it? After all, the UpdateText
message means "the property shall now contain this value", and any observer who would respond to a TextUpdated
message can also safely treat the UpdateText
message as a TextUpdated
, because it knows that now the property contains that value.
This is a bit hard to explain coherently, sorry. I've probably muddled it rather than made it clear. Let me try to rephrase what I mean:
TextUpdated of newValue
is fired. Anyone who needs to observe the new value can safely consider that newValue
is what is now contained in the text field.UpdateText of newValue
is fired. Again, anyone who needs to observe the new value can safely consider that newValue
is what is now contained in the text field.UpdateText
inherently implies TextUpdated
, so there's no need to fire a TextUpdated
when UpdateText
happens; and by not doing so, you avoid the loop.
I hope that's not as muddled to you as it seems to me, and I've managed to express what I'm trying to say.
You might have a typo in step 5, because as I understand your description, the text field should not be updated to contain ab
.
In any case: I haven't been able to fully understand your reasoning, but it seems to conflict with the principle of a single source of truth in Elmish/Redux. The model should be the single source of truth. When the user types into a text field, he isn't strictly speaking changing the value of the text field directly, he's sending a message to update the value of the text in the model. This message goes to the update function (the reducer in Redux terms), which updates the central model. The fact that the text value changes in the model is then what triggers the view update.
Now, in WPF and Xamarin.Forms, the user is of course (from a technical standpoint) updating the text field value directly. And AFAIK it's this discrepancy that leads to problems.
As I understood your suggestion, it seems to rely on the parts of the app that depend on the text value to observe actions directly, whereas in Elmish/Redux, actions are only dispatched to the updater/store (sorry if I botch some terms; not used to Elm) and it's the central state update that triggers everything else to change. To put it concisely: For the single source of truth to apply, any change to the text value in the central model must force the actual text field value to change accordingly.
Sorry if I've misunderstood your reasoning or not made myself sufficiently clear above.
Yes, I think you're right that I was missing the Elmish principle of all updates coming via messages, because what I wrote had the inherent assumption that as the user typed, the model was being updated "live". I can only offer the defense that I wrote that in haste before a meeting, and should have waited until after my meeting when I'd have time to revise properly.
So forget my previous, somewhat muddled, message. But I do think there's one part of your example that I think is the wrong approach, which I maybe didn't communicate well due to being rushed, and that happens to be in step 5. Specifically, this part:
Since this is a change from its previous value ab, a new PropertyChanged is fired, and update loop 3 starts with message UpdateText "a".
Now, here's my understanding of what's going on:
UpdateText "a"
message means "set the value of model.TextField
to a
".model.TextField
was just assigned a value that is different from what it used to be. E.g., if model.TextField
was "ab" and is now "a", then the PropertyChanged event will fire.If that understanding is correct, then why would you cause PropertyChanged to dispatch the UpdateText
message? I think that's a mistake. That's like saying "The model was just set to a
, so now you should set the model to a
". In a system that's fast enough to keep up with the user's keystrokes, that is guaranteed to be a no-op. In a system that's not fast enough to keep up with the user's keystrokes, then you're setting the model back to an old, outdated value, with the bad results that you've already observed.
What I think should happen is:
UpdateText "a"
to be dispatched, and later UpdateText "ab"
. As long as the event queue is an actual queue, so that events can't swap their order, the model will be updated first to "a", and then to "ab", in that order.UpdateText "a"
is handled. This causes the model to be updated, which in turn causes the PropertyChanged event to fire. Here's where I differ from your example. Your example had the PropertyChanged event causing UpdateText "a"
to be dispatched, whereas I think it should dispatch the message TextUpdated "a"
, which is an informative message, not an imperative message.UpdateText "ab"
is handled before TextUpdated "a"
can be handled, because the user was typing faster than your code could run. This causes a TextUpdated "ab"
message to be dispatched.TextUpdated "a"
is handled. This makes other parts of the UI update, but not the text field. I bolded that because that's the part that breaks the loop. If TextUpdated "a"
causes the text field's value in the model to be updated, that sends us back to step 2 and the loop you observed. But if TextUpdated "a"
means "the text field was recently set to this value, so other parts of the model need to be adjusted to reflect that", then that keeps the single source of truth but breaks the loop.TextUpdated "ab"
is handled, which again causes other parts of the model to update themselves but does not cause the value of the text field itself to be updated.As an example of what I mean by "other parts of the model", say the text field is the user's name, and underneath there's a "Hello, (name)!" message being updated as the user types. The TextUpdated "worl"
would cause that message to display "Hello, worl!", and then when the TextUpdated "world"
message arrives milliseconds later, the message now becomes "Hello, world!". If you insert a 500ms lag in the PropertyChanged event handling, then what the user will see is that he types his name, and half a second later, the message updates to include his name (one character at a time, at the same speed as how fast he typed his original keystrokes). But as long as the data flow is one-way, from the user's text input to the other parts of the model like the "Hello, (name)!" message, then you can't get into a loop.
So although I muddied the waters a bit by my bad mental visualisation of the data model, what I said about conflating the imperative and informative messages (which I have called UpdateText
and TextUpdated
respectively) still holds. De-conflate the two and the loop should be broken.
The
UpdateText "a"
message means "set the value ofmodel.TextField
toa
".
Yes.
What causes
PropertyChanged
to fire is thatmodel.TextField
was just assigned a value that is different from what it used to be.
No. There are no events named PropertyChanged
. My bad, I think I see where the confusion comes from - when I said that PropertyChanged
was fired, I wrote PropertyChanged
because of consistency with UpdateSourceTrigger=PropertyChanged
. The actual event that fires is in fact a kind of TextChanged
on the actual text entry control, not a PropertyChanged
on any lower-level code, which would only be relevant in a VM (not in a pure Elm or Redux architecture).
I have updated the steps and renamed PropertyChanged
to TextChanged
. Does it make more sense now?
If this clarifies things, let me know if the rest of your latest comment is still relevant and I should read it more carefully (it seems to be based on the misunderstanding I hope to have clarified now).
Okay, so in your system, UpdateText "a"
causes PropertyChanged "a"
to fire if the property wasn't "a" before. And PropertyChanged "a"
causes UpdateText "a"
to fire every time. (I get this from your step 5, "a new PropertyChanged
is fired, and update loop 3 starts with message UpdateText "a"
"). Is that correct?
My change is that if UpdateText
fires PropertyChanged
, then PropertyChanged
should not fire UpdateText
. I think that this interaction between the messages is the key to breaking the loop, and I don't think it's affected at all by my misunderstanding PropertyChanged
to be an event. So I do think the rest of the comment is relevant and worth reading a bit more carefully.
Okay, so in your system,
UpdateText "a"
causesPropertyChanged "a"
to fire if the property wasn't "a" before. AndPropertyChanged "a"
causesUpdateText "a"
to fire every time. (I get this from your step 5, "a newPropertyChanged
is fired, and update loop 3 starts with messageUpdateText "a"
"). Is that correct?
Yes, but we may be thinking of different things when we say PropertyChanged
.
As for the rest of your comment, it seems the key part is point 2 – that's also where I spot the first key misunderstanding (whether mine or yours remains to be seen):
UpdateText "a"
is handled. This causes the model to be updated, which in turn causes the PropertyChanged event to fire. Here's where I differ from your example. Your example had the PropertyChanged event causingUpdateText "a"
to be dispatched, whereas I think it should dispatch the messageTextUpdated "a"
, which is an informative message, not an imperative message.
No, this sounds wrong, I am unsure where you are thinking the events are (in particular the PropertyChanged
event you speak of), and from where the messages are being dispatched. Notably, there's no PropertyChanged event in the model in Redux/Elmish (hence why I replaced the term PropertyChanged in the OP). Let me state in more detail how the loop works (type/member names below may not correspond fully to real-world names in WPF or XF):
TextBox
TextBox.TextChanged
fires. This event is wired to dispatch a message containing the current text of the TextBox
.TextBox
to be set to the text that the user just typed (and the model was just updated with). (If the TextBox
was not updated from the view rendering function, we would have no way at all to update the TextBox
to contain what was in the central state.)This describes a single loop. Note that the only required events are in the UI; any lower-level events are implementation details of the specific MVU architecture.
I see no way of discriminating between UpdateText
and TextUpdated
messages, because all messages in this example are being sent from the UI, and unfortunately a TextBox
(at least for Entry
in Xamarin.Forms; unsure about WPF) has no way of knowing whether its text was changed programatically or by the user.
I could also phrase the central issue like this:
With the central model as the single source of truth and a single view update function only depending on the central model (as is the case with Redux and, as I understand it, Elm), the view has to be updated in exactly one way (specified by the view update function) whenever the (relevant) state changes, or not at all. The issues then arises due to a race condition between the update loop and user input triggering further loops.
Again, I apologize if I am still misunderstanding you or still failing to adequately specify the situation that leads to the problem (note however that the underlying architecture is normal Redux/Elmish, with which I'm assuming familiarity, so my explanations are deliberately short on that end).
For a concrete example of this (at least I think), check out this "half-elmish" sample: https://github.com/fsprojects/Elmish.XamarinForms/tree/master/Samples/StaticView/StaticViewCounterApp
If you click the buttons and move the slider around rapidly for more than 5 seconds then it gets into a mess. I have also rolled my own elmish/redux while keeping a traditional MVVM approach because I'm not sure how to deal with this sort of issue.
@cmeeren - do you have a simple repro repo for the issue using text entry etc on XF? I'm asking because I've tried to make my own, and failed to cause any issues. Is the scenario feasible, because update and view both run on the UI thread - i.e. referring to your original description, steps 2 and 4, update 2 cannot start before update 1 has finished, as both are processed on the UI thread ?
The rules:
update
gets run on the UI thread. dispatch
can be called from any thread. The message will be processed by update
on the UI thread.view
gets called on the UI thread. In the future an option may be added to offload the view
function automatically. I just tested with the AllControls sample for Elmish.XamarinForms. I inserted System.Threading.Thread.Sleep(1000)
at the top of the update function, started the app, and edited the entries on the carousel page. It seems that the UI is blocked while the update function runs. So you don't get the feedback loop I describe, but the price you pay for it is the UI being unresponsive while the update function runs, which IMHO is also not optimal. My hybrid solution including viewmodels avoids both of these problems, but of course complicate the architecture somewhat.
@cmeeren I think the blocking you are seeing is part of the elmish design. The update function, by design is not supposed to do any async work inside it. It is supposed to "dispatch" any async work via "commands". The commands are built and then returned as the second part of the tuple in the result of the update function.
More info is at the link that @mtnrbq provided ( Elmish.XamarinForms docs ).
I stumbled too at the idea of no doing any async work inside the update function at the beginning. It seems to me that the elmish way is to "prepare" that async work using commands and then returning those commands from the update function to allow the elmish infrastructure to execute them. But once you get used to it, it actually makes things seem simpler (because the separations from the async work like db or web calls are more obvious).
@jptarqu I have no problem with doing async work outside of the update loop; my problem is that the model update itself sometimes can take long enough that the UI will visibly hang. Above I provoked it using Thread.Sleep
, but sufficiently large or complex apps/models/update functions can make this happen without artificial delays.
@cmeeren sorry, I did not think about that situation when the non-async work needed inside of the update function is actually heavy enough to delay the update function in a significant manner.
It would be interesting to know how the original elm community worked around situations where the non-sync work was too heavy and needed to be executed within that life of the update function. Maybe they would suggest turning that work into an async block that gets dispatched (because it is CPU bound even if it is not IO bound)?
My non-async work inside the update function is limited to business validation and setting the model's data. I did however got into a situation where one of the validations was long running because it required an api call, but for that, I just separated that piece into its own command. Once the command was fulfilled then the model's "valid" prop would be updated with the yes/no and the error messages.
But my situation and your situation are probably not similar at all, so I would be curious as to what would be the proper way to handle your situation would be.
@cmeeren - reflecting on this, it seems similar to the RX autocomplete pattern - i.e. while I'm allowing typing, I'm also trying to update my UI with responses from a potentially long running call.
Thoughts?
@mtnrbq I might be wrong, but I think that's a different situation. AFAIK there's no feedback loop in the situation you describe, and since it's an asynchronous operation, there's no blocking of the UI either.
@cmeeren ,sorry, I'm not getting the scenario - afaik most client side frameworks stress that you should not do heavy lifting on the UI thread otherwise the UI will become unresponsive, hence all the plumbing to handle asyncing the work off and providing safe methods for the UI to be updated when the work is done. i.e. changes to the UI are serialized. I can see two cases, but I'm not clear if either of them is the scenario you've got:
You want to block interactivity while the heavy lifting is done - use a progress indicator and disable the UI elements while the work is being done
You want the UI to remain responsive while doing the heavy lifting - async it off the UI thread and selectively enable / disable UI elements if needed to maintain coherence of the model
I'm feeling like it is a bit of an 'it depends' answer - if the non-async work in the update is taking too long, work out a way to make it async :(
I agree that heavy work can and should be offloaded, but I manage to provoke weird behaviour (as described) with a loop update time of just 10-20ms (using Thread.Sleep 10
or 20
in the update function). And in my (limited) experience, update times of 10-20 ms can easily be reached for non-trivial model sizes or view update functions (e.g. updating button states and validation statuses based on the input).
In an ideal world the update function run run instantaneously, but we all know that immutability comes with performance costs, and for quick inputs (like a user typing quickly) I have observed the behaviour I have described without needing to insert artificial delays.
@cmeeren: This sounds like a job for Rx.
Right now you have your TextChanged event (that fires after each character is typed), which then triggers your UpdateText message, which then starts the update loop.
You could use System.Reactive framework to turn the INotifyPropertyChanged "PropertyChanged" event in your VM into an Rx data stream. Then subscribe to the new PropertyChanged event stream and throttle it to 25ms (or whatever). Then from that throttled event handler you can check for the individual property changed name (ex: "UserName"), and send the appropriate UpdateText message. Or you could just recreate your Data model every time the PropertyChanged event stream is handled, and then send in the entire Data model as updated.
Disclaimer: I haven't actually used Elmish.WPF yet, but I am also experimenting with using MVVM with a data store pattern, so I'm still getting my head wrapped around the whole MVU idea.
@JordanMarr Elmish doesn't use viewmodels, so there's no PropertyChanged
in the first place. The only event in this whole story is the TextChanged
event on the TextBox that causes the message to be sent, but you can't do anything about that event in Elmish.WPF; implementation-wise, it's all wired up via XAML bindings to a dynamic object, all managed and kept internal by Elmish.WPF.
As for the MVU pattern: In short, a simple MVU cycle (for a text box) is:
(Sorry for any errors; I'm new to Elmish.)
Step 4 is where the problem exists: If the cycle completes before the user types another character, all is OK. But if the cycle takes too long and the text in the input field is different from what it was when the cycle started, it will be reverted to the text that started the first (now completed) cycle, and (in WPF and Xamarin.Forms at least) this will cause a new message to be dispatched (repeat the cycle from step 2).
@cmeeren - is the 'weird behaviour (as described)' you refer to above the unresponsive UI, or have you induced the looping you're describing ?
In Elmish.WPF, it's the looping. With only 10-20ms delay, it stops fairly quickly, but the symptoms are that the text field does not update correctly when you type too quickly into it. With longer delays you can notice the text field living its own life for a while.
In Elmish.XamarinForms (and AFAIK in "real" Elm), the update function blocks the UI, so the UI becomes unresponsive. (Characters typed while blocked seem to appear all at once in the next update, so it's not too bad, but it's still a UI block).
Note of course that for input fields, this only happens with UpdateSourceTrigger=PropertyChanged
(which is a prerequisite for e.g. as-you-type validation).
Can we enable blocking the UI until the update finishes easily? Even though I don't see the issue when in release mode, it's annoying when coding, I think I would prefer blocking and the UI lagging than having the value be different than what I typed. And if it would occur in production, blocking is most likely better too, come to think of it.
I agree that at least when typing, blocking (as in Elm and Fabulous (previously Elmish.XamarinForms)) is better than the alternative weird behavior. But AFAIK the non-blocking behavior is due to how the core Elmish update loop is implemented, and thus @et1975 might be the right person to weigh in on this.
I've been meaning to rewrite the dispatch loop implementation, but for now perhaps we should backport the dispatch loop from Fabulous to WPF and expose it under runSynchronously
or something.
EDIT: I just looked at Fabulous impl... oh wow, it's so much bigger than I remember.
I read this whole topic and I was surprised to learn that XamarinForms/Fabulous' update
function run on the UI thread. But if they run their own dispatch loop, it explains it all. What about Cmd
s ?
@cmeeren if you give the Elmish core 3.0 beta a try you might be able to resolve this. You'll need to override the default syncDispatch
of the Program
instance and sync the call on the UI thread, but it should be a one-liner.
You'll need to override the default
syncDispatch
of theProgram
instance
What do you mean? I can't find any accessible syncDispatch
property on the Program
instance using Elmish 3.0.0-beta-2.
Nevermind, was just a missing paket restore.
I have never used Elmish.WPF, but I have a Xamarin.Forms app with a custom Redux-like architecture (which is in many ways similar to Elm). In developing that app I had to grapple with a problem that I upon experimentation also see is present in Elmish.WPF, but which is not a problem in MVVM (using simple data bindings, with no central update loops). I want to start a discussion/brainstorming around this problem and am wondering if any of the contributors to this repo, or @dsyme (cc due to Elmish.XamarinForms), or anyone else for that matter, have any input.
Consider the following situation:
UpdateSourceTrigger=PropertyChanged
(AFAIK in Xamarin.Forms this is the default/only behaviour)In this situation, in both WPF and Xamarin.Forms, you get a feedback loop messing up the text in the input field. This should come as no surprise, as you are in effect triggering an event (
TextChanged
or whatever it's called) from its event handler (the dispatcher, through the subsequent view update function), it's just that in most cases, it works fine because the cycle completes before the user has done anything else, and the event is thus not fired since the input field value is still equal to what the update/view function want to set it to.Specifically, here’s what happens:
a
in the input field, causingTextChanged
to fire on the input controlUpdateText "a"
b
in the input field (before update loop 1 has completed)UpdateText "ab"
a
. Since this is a change from its previous valueab
, a newTextChanged
is fired, and update loop 3 starts with messageUpdateText "a"
.ab
. Again a newTextChanged
is fired, and update loop 4 starts with messageUpdateText "ab"
.This can easily be tested using the Counter sample in this repo with the following simple changes:
UpdateText of string
) to update itSystem.Threading.Thread.Sleep(TimeSpan.FromMilliseconds 100.)
first to ensure the update loop takes long enough for the problem to occur (I have of course observed this in a real app without having to block the update function withThread.Sleep
)UpdateSourceTrigger=PropertyChanged
(e.g.Text="{Binding MyText, UpdateSourceTrigger=PropertyChanged}"
)Then start the app and type quickly into the text box to observe the behaviour.
A quick test (https://github.com/Prolucid/Elmish.WPF/issues/40#issuecomment-408803285) shows that in Elmish.XamarinForms, the problem does not occur because the UI is blocked while the update function runs. While this avoids the feedback loop, blocking the UI is also not optimal.
The way I have solved this in my real app:
I have used a mix of Redux (similar to Elmish) and normal, but simple view models. All of the important, central state is modified in the update function and stored in the central state object. But each view still has a view model that acts as the binding target. The view model receives model updates whenever an update loop completes (the code that instantiates the view model subscribes to model updates and sets the updated model – or rather, for simpler testing, the necessary subset of it – to a property on the view model). But the important part in this discussion is that text input fields do not go through the central update loop. They are bound only to mutable properties on the view model (as in normal MVVM), and button states/validation messages/etc. (read-only properties on the view model) are then based on these mutable properties together with the central model state.
For example, a view model might look like this (excluding
INotifyPropertyChanged
implementation) (writing this from the top of my head, so please excuse any syntax errors):In spite of the mutability, this solution is still functional in nature. The view models are completely dumb and are in essence pure functions, whose inputs are their mutable fields (the main model, text input field values, a continually updated clock, etc.) and whose outputs are their read-only fields. They are, thus, fairly trivially testable.
(I know that was really quickly and roughly explained; let me know if more details are needed. I have considered presenting the solution in a blog post, but I have no immediate plans to do so unless requested.)
There are a few complications one might come across in real-life scenarios; for example, the VM might have to react to messages. For example, in the view model above, there is no way for a successful registration (done from another view/VM in the foreground, with the sign-in view in the background) to automatically fill the recently registered username. To provide this functionality, I have given the view models that need it a simple "update"-like function with the signature
'msg -> unit
. For example:The code that instantiated the VM (which already subscribes to state updates and sets the
Data
property) then subscribes to messages that are dispatched (a simple thing to do since I'm using a homegrown architecture) and passes them to theOnDispatch
function above.One could then say that overall, these VMs, in addition to being binding targets, play the role of a mini Redux store for localized state in a single part of the app.
Another complication I won't go into detail about are view models for elements of ListView et al. I like to avoid codebehind, but I needed a timer in order to update arbitrary elements of a ListView every second, and I found no better solution than to simply place the timer in the view codebehind, updating a
Time
property on the element's VM.In any case, having used this solution for a while, I'm fairly happy with it. It's not technically purely functional, but most of us can't afford to be idealists. It's practically purely functional, and the important part is that it's clear (once you understand it; my explanation here may have been insufficient) and easily testable.
What I’m wondering about then, and hope to start a fruitful discussion about, is the general problem I have described with the update feedback loop in Elmish/Redux architectures. The problem is partly a general performance problem of an immutable architecture, but is also more specific than that. Some questions to get started: