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

:dispatch-later causes error #14

Open
mike-thompson-day8 opened this issue Dec 26, 2017 · 1 comment
Open

:dispatch-later causes error #14

mike-thompson-day8 opened this issue Dec 26, 2017 · 1 comment

Comments

@mike-thompson-day8
Copy link
Contributor

See this re-frame issue:
day8/re-frame#441 (comment)

Do we want to do anything about this? What would we do about it?

@mike-thompson-day8 mike-thompson-day8 changed the title :dispatch-later :dispatch-later causes error Dec 26, 2017
@samroberton
Copy link
Contributor

The code in re-frame/interop.cljc already attempts to handle this scenario -- the chosen semantics (per the docstring for set-timeout!) are to ignore the ms value given to :dispatch-later and queue the dispatched event up immediately by using next-tick. Something about that implementation is not working correctly.

What's confusing is that the :dispatch and :dispatch-n FX handlers work without encountering this error.

... and I think that's lead me to a likely explanation of the problem, since really, the use of next-tick is the only difference between :dispatch-later, which doesn't work, and :dispatch, which does.

next-tick, in the JVM implementation, uses bound-fn to capture the state of all dynamic variables as at the time that the :dispatch-later effect is processed (this is (a) generally the correct thing to do when moving a computation onto a different thread, and (b) specifically necessarily to make sure that the dynamic variables that re-frame-test uses still work).

This use of bound-fn will thus capture the current value of re-frame.events/*handling*. In theory, that should be fine: it will capture exactly the same value that the :dispatch and :dispatch-n FX handlers will see, so you'd expect everything to work correctly, no?

But, there's an invisible coupling between the FSM in re-frame.router and the behaviour of re-frame.events/handle: when the FSM it sees an :add-event trigger, the FSM makes its decisions about whether an event is currently being handled based on its fsm-state instance field (which is not captured by bound-fn), not based on re-frame.events/*handling*. If the FSM state is :idle, then the event will immediately be processed (via run-next-tick, which will again ensure that *handling* retains its value at the time that :dispatch-later was processed), whereas if the FSM state is :scheduled or :running, the event will be adding to the queue but not immediately processed.

So I think what's happening is that in the :dispatch-later scenario, *handling* and fsm-state are effectively giving different answers to the same question ("am I currently handling an event?") because *handling* has been captured by bound-fn in next-tick.

Does that explanation make sense?

If I am correct there, then fixing it is likely to be somewhat involved. I think ideally it would involve getting rid of re-frame.events/*handling* entirely, and making dispatch-sync go through the event-queue as well (but synchronously, obviously), so that there's only on piece of state which represents the work currently being done.

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

No branches or pull requests

2 participants