Skip to content
New issue

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

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

Already on GitHub? # to your account

Race condition in simple example app #538

Closed
OStrekalovsky opened this issue Aug 2, 2019 · 7 comments
Closed

Race condition in simple example app #538

OStrekalovsky opened this issue Aug 2, 2019 · 7 comments

Comments

@OStrekalovsky
Copy link

OStrekalovsky commented Aug 2, 2019

In "Simple example" app we can see regular example of binding input value of the DOM input element with app state and reading "proposed value" in on-change event from target.value.
https://github.com/Day8/re-frame/blob/ce4c580dfc24e120b932f4aebf000a85b426543e/examples/simple/src/simple/core.cljs#L72
https://github.com/Day8/re-frame/blob/ce4c580dfc24e120b932f4aebf000a85b426543e/examples/simple/src/simple/core.cljs#L73
But applying this pattern can leads to inconsistence in UI when a user types faster, then app state syncs with DOM input element. Is is not only about UI hangs but about stale app state reads: typing a string in input a user can finished with string which is subsequence of the original string.
Just try to add the following code (an argument value change is probably necessary for different machines) to event handler which updates app state:
(reduce * 1(range 5000000))

It can leads to very bad UX: imagine that a user fills in a field with a new password without seeing its content, and due to this problem some kind of mess is going to the server :)

This is because target.value mutates by user (when it types charecters) and by re-frame at the same time. The problem was previously discussed in #39 and proposed solution was to use dispatch-sync instead dispatch in on-change event.
Another way to solve the problem is to use reagent atom for reading "local" component state and use subscribptions and event handlers only for global sync, but this violates the whole harmony of the state work approach described in this library and complicates the code even for the most primitive cases. Or maybe we should avoid using on-change event at all and starts handle on-key-pressed event instead, but it is not easy at all.

To be honest, I'm confused.
Any suggestions on how to solve this problem are welcome. Perhaps this issue should be covered at least in the FAQ section.

@jrad87
Copy link

jrad87 commented May 4, 2020

Another way to solve the problem is to use reagent atom for reading "local" component state and use subscribptions and event handlers only for global sync, but this violates the whole harmony of the state work approach described in this library and complicates the code even for the most primitive cases. Or maybe we should avoid using on-change event at all and starts handle on-key-pressed event instead, but it is not easy at all.

There is actually nothing wrong with this approach in many situations. This tutorial: https://purelyfunctional.tv/guide/state-in-re-frame/ broadly covers the nuances of various strategies for managing state. For forms, it is very sensible to have a component local reagent atom store any change to the form inputs, and also have a button that could dispatch the global sync event.

@OStrekalovsky
Copy link
Author

For forms, it is very sensible to have a component local reagent atom store any change to the form inputs, and also have a button that could dispatch the global sync event.

Maybe, but splitting the state (component local and global) can leads to inconsistencies and very messy code. I was hoping that in case of the simplest and most frequent work patterns good examples will be provided where everything should work reliably.

@mike-thompson-day8
Copy link
Contributor

mike-thompson-day8 commented May 4, 2020

I would suggest you look at a library like re-com. Some of its components have local state and it works without problems or messyness.

@OStrekalovsky
Copy link
Author

OStrekalovsky commented May 4, 2020

I would suggest you look at a library like re-com. Some of its components have local state and it works without problems or messyness.

Thanks for reply. Should we replace hand-written components from example app by re-com components as soon as only an expert (e.g authors of re-com) can writes them correctly? Should we highlight the problem of race conditions and how to avoid them in docs?

@mike-thompson-day8
Copy link
Contributor

You requested good examples, yes? I believe they were supplied. You expressed concern about how code might get messy, and I believe the example shows how to avoid it.

As far as I can tell your final request is that we add an FAQ entry, yes?

@OStrekalovsky
Copy link
Author

OStrekalovsky commented May 4, 2020

As far as I can tell your final request is that we add an FAQ entry, yes?

Yes, it would be great. And fix example app.

@mike-thompson-day8
Copy link
Contributor

The next time I push FAQs there will be a new one covering this subject. The text currently looks like this ...

Question

My input field is laggy. When the user types quickly, it is dropping characters.

I have implemented it like this:

  [:input.input
          {:type "text"
           :value @(rf/subscribe [::subs/username])
           :on-change #(rf/dispatch [::events/change-username (-> % .-target .-value)])
           }]

Answer

That on-change handler is being called after the user types every character. If the user is typing very quickly, then the following race condition can occur:

  1. The user types a new character taking the field from state A to state B (B has one new, extra character in it, compared to state A)
  2. The change event for state B is dispatched by on-change. And that event is queued for processing.
  3. But before that event can be processed, the browser schedules an animation frame.
  4. In that animation frame, the component is rerendered
  5. But during that render the subscribe will deliver state A
  6. That means the text in the box will revert from state A to state B
  7. Now, if nothing happened till the next animation frame, the situation would resolve itself correctly. Because state B would be rendered.
  8. BUT if the user immediately types another character, the state dispatched by on-change will be State A + the-new-character. The extra character
    which caused A -> B is now lost.

Bottom line: with high-speed typing, characters can get dropped.

There are three solutions:

  1. don't use on-change, and instead, use on-blur which is only called when the user has done all their fast typing, and they leave the field.
  2. if you have to use on-change then switch to use dispatch-sync in on-change, instead of dispatch. The event will not be placed on the queue. It will be handled immediately. Synchronously.
  3. use a component from something like re-com because it has been engineered not to have this problem. Or copy the (local state) technique it uses.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants