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

Cannot dispatch in the middle of a dispatch #71

Closed
jongbeau opened this issue Mar 11, 2015 · 29 comments
Closed

Cannot dispatch in the middle of a dispatch #71

jongbeau opened this issue Mar 11, 2015 · 29 comments

Comments

@jongbeau
Copy link

From a Login react component, I have a login action that makes an API request and on successful response, it updates a SessionStore. The Login component sees the value of the session store updated through it's props passed down from the parent component, and in shouldComponentUpdate, it does a transitionTo (using react-router) to an account page. The account page then calls an action to update the SessionStore in ComponentWillMount.

I'm getting this error message:
Uncaught Error: Invariant Violation: Dispatch.dispatch(...): Cannot dispatch in the middle of a dispatch.

I believe it's due to the account page trying to update the SessionStore as the result of the SessionStore just getting updated from a previous action. Any suggestions on how to architect this differently?

NOTE: things work when using react-router's HashLocation, but not with the HistoryLocation.

@goatslacker
Copy link
Owner

If you can, move the action out of the store listener's path. I've seen this happen with transitionTo often, it's probably not a good idea to transitionTo immediately in a store listener. Try setting the transitionTo in a setTimeout/setImmediate so it happens on next tick.

@mshick
Copy link

mshick commented Mar 14, 2015

I've dealt with similar problems using Flux & react-router together. Building off of this document React Router in a Flux App, and ideas similar to @goatslacker I put together a scheme like this gist

It gives me a consistent interface for invoking the router, and removes the need to think about when a setTimeout/setImmediate is necessary. After working with it a bit I feel that router state transitions should always happen in a new execution loop since it's a unique type of app activity, occurring in a separate dispatcher.

You'll also see in that gist a mixin I use for sending optional actions on the willTransitionTo/willTransitionFom static hooks, which can be used to, say, do an async data fetch when transitioning to a new route, or setting flags when leaving a route. This is a little like that fetchData method in the react-router async examples.

@deser
Copy link

deser commented Apr 8, 2015

Sorry guys but I think that dispatcher should query and fire in turn actions if they are fired simultaneously. Am I right? If so could you please improve dispatcher of alt. Thanks.

@troutowicz
Copy link
Contributor

The dispatcher handles actions synchronously. If an action is fired inside another action's context, the dispatcher will throw an error. As others have said before, if you cannot move the nested action elsewhere, use setTimeout to fire the nested action in a new context.

FYI, there is no need to open new issues (#136) just to bump another.

@deser
Copy link

deser commented Apr 8, 2015

Guys from facebook say that this solution not bad: https://monosnap.com/image/VfYSlclKucvgSkudGJnliEkn19upZu

P.S. setTimeout is not silver bulletin. I can't use setTimeout elsewhere

@deser
Copy link

deser commented Apr 8, 2015

By the way sorry for opening new issue

@troutowicz
Copy link
Contributor

Apologies, I thought you were the OP of this issue. In your scenario, an action queue would be a good approach. But that would not be part of the dispatcher, the dispatcher will always handle one action at a time. It could possibly be made into a util for alt though.

Like fisherwebdev wrote, before firing the actions for each query result, check the dispatcher's isDispatching method. If the dispatcher is free, fire the next action. :)

@deser
Copy link

deser commented Apr 8, 2015

:) I think in this case code will be a little unreadable.
Can you provide me some details in which way I can modify alt dispatcher?
Thanks

@goatslacker
Copy link
Owner

Do you have anything in mind in terms of implementation details?

My opinion on the matter: in lieu of implementing a custom dispatcher I defer over to Facebook's dispatcher, if they implement a queue then Alt will have it.

@deser
Copy link

deser commented Apr 9, 2015

Yep, in flux i've made such dispatcher:
https://monosnap.com/image/XM8PJLB9nsHtfNCYbAjv2HjfpIilh2

@jdlehman
Copy link
Collaborator

jdlehman commented Apr 9, 2015

Cool. You can also effectively do the same thing with MyActions.someAction.defer() in alt, which boils down to wrapping the action in a setTimeout like your dispatcher implementation.

@deser
Copy link

deser commented Apr 9, 2015

Your solution is not always suitable because in some cases I need this action to be deffered in some cases I don't need. That's why i've made processing of such situations centralized in dispatcher.

@jdlehman
Copy link
Collaborator

jdlehman commented Apr 9, 2015

Well right, you don't always use defer. My example is not intended to be used for each action call, you just need it if you call an action inside of another action (meaning the dispatcher is dispatching already).

@deser
Copy link

deser commented Apr 9, 2015

And exactly this situation is complex to catch :). One action can be triggered from view1 another from view2.

@goatslacker
Copy link
Owner

👍

@mhuggins
Copy link

mhuggins commented Jul 5, 2015

@deser Thanks for sharing! Small FYI, it looks like in newer versions of the Flux dispatcher, you can now just call this.isDispatching() instead of this.$Dispatcher._isDispatching. 😄

@marklevi
Copy link

Thanks, the setTimeout worked for transitions.

@aksonov
Copy link

aksonov commented Aug 29, 2015

So is usage of setTimeout OK? Or maybe is it wrong "Flux" way when i do "login" fetch action to do login, then get success result, change login store, and then react component calls action to go to next screen?

@mull
Copy link
Collaborator

mull commented Aug 31, 2015

@aksonov setTimeout (or deferring) is generally not desirable but sometimes unavoidable. I follow the "try not to use it" approach. ;)

@aksonov
Copy link

aksonov commented Aug 31, 2015

So what is correct way for typical login scenario?

Pavel.

31 авг. 2015, в 15:50, Emil Ahlbäck [email protected] написал(а):

@aksonov setTimeout (or deferring) is generally not desirable but sometimes unavoidable. I follow the "try not to use it" approach. ;)


Reply to this email directly or view it on GitHub.

@mull
Copy link
Collaborator

mull commented Aug 31, 2015

@aksonov this is one of the times I use defer/setTimeout.

@mib32
Copy link

mib32 commented Sep 25, 2015

What would you recommend to do in my case:
PhotoUploader component is mounting within the dispatching of some higher level action (Modal mounting). I want to alter the PhotoUploaderStore state (clear it, so uploader would display no image) during componentWillMount of PhotoUploader. And it throws cannot dispatch in a middle of dispatch.

How could i avoid deferring in that case?

Sorry if such a question is inappropriate here.

@goatslacker
Copy link
Owner

On photouploaderstore listen to that higher level action that mounts all the things and then clear the store then.

Actions can do various things they don't have to be 1:1.

@mib32
Copy link

mib32 commented Sep 27, 2015

@goatslacker thanks Josh.

@jamesfzhang
Copy link

What's an appropriate timeout to set, 50ms? 100ms? I imagine it must get affected by system resources?

@jdlehman
Copy link
Collaborator

@jamesfzhang You can set it with 0ms. All you want is for it to be in the next event loop.

@jamesfzhang
Copy link

Thanks @jdlehman

@mib32
Copy link

mib32 commented Jan 16, 2018

@goatslacker What if I want to make a chain of actions, in a waterfall way?

  • I can't use promises in the component because it breaks flux
  • I don't want to get into a defer trap

@CrazyPython
Copy link

@mib32 I'm not exactly sure what your case is, please clarify.

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