Trigger this.emit( 'change' ); after dispatch ends

[mjangda]

I'm using Fluxxor + rackt/react-router and running into the "Cannot dispatch an action (...) while another action (...) is being dispatched" error.

Here's what I'm doing:

• A Login route/component displays a login form
• onSubmit triggers the "login" action creator.
• The "login" action creator interacts (async) with our API to validate the submission. When the call completes, dispatch the "LOGIN_SUCCESS" action (assume no failures for simplicity)
• AuthStore is listening for this action and updates its data and triggers an this.emit( 'change' );
• The Login route/component is using StoreWatchMixin( 'AuthStore' ) to listen for updates and uses react-router to transitionTo( 'dashboard' )
• During its setup, the "dashboard" route/component triggers an action.
• The "Cannot dispatch an action..." error is thrown.

The problem is that the emit call triggers the transition and results in the "dashboard" component set up while the dispatcher thinks that the "LOGIN_SUCCESS" action is still being fired.

Is there a way to defer the emit call to after the action dispatch is complete? Or should I be thinking of another approach to this? (I could do a setTimeout but that feels fragile.)

[BinaryMuse]

Unfortunately, this problem seems to pop up fairly often when components dispatch actions in their componentWill/DidMount hooks, especially when routes change in response to an action, as the dispatch loop hasn't completed by the time the component mounts.

As you've said, one quick fix is to wrap some portion of the process in a setTimeout. Depending on your setup, you might experiment with making either the transitionTo() call or the second action dispatch (the one that happens in your dashboard component) async in this manner. While I normally recommend against this practice, as it defeats the purpose of the assertion, the practice falls down a bit in this situation. I would probably not make the emit call async, as that completely sidesteps the assertion in all cases, when in fact you only want to sidestep it in the routing/mounting case.

Another pattern I've been using more often when a component needs to get initial data from a flux store is to fetch the data from the store instead of firing an action when the component mounts. If the store doesn't have the data cached/ready, it can fetch it asynchronously and then dispatch an action when it's available. Here's a basic example: https://gist.github.com/BinaryMuse/3b0a4ba166ac81840cab This allows you to remove the dispatch when the component mounts.

[paulstatezny]

Thanks for your work on Fluxxor, @BinaryMuse! I came here to report the same problem and noticed this GitHub issue. A few days ago at React.js Conference I spoke with Michael Jackson of rackt/react-router about it. His take on this (as I recall) was that actions are not meant to be long-lived in this manner.

I'm sure there's a reason Fluxxor assumes the role of preventing multiple actions, but there is an obvious conflict here with an extremely common use case. (Save data on a page with an action, route to new page as a result, perform new action when the next page loads.)

Using setTimeout or async routing is just putting a bandaid on the problem. Would you be open to a modification of Fluxxor's behavior? I'd be glad to try my hand at it and submit a PR.

[paulstatezny]

I made a barebones React/React-Router/Fluxxor app to recreate the error -- without luck. It didn't make actual API calls, but I simulated them using promises. Interesting that it didn't work.

@BinaryMuse, would you consider allowing the dispatcher to be agnostic about multiple dispatches? I made a hacky proof-of-concept patch that isolates dispatch calls from each other by instantiating a new dispatcher each time. (Patch here.)

In an actual PR, I would make the dispatcher stateless. It would instantiate a separate "action handler" object on dispatch. The action handler would hold the state related its action. (And most of the code from the dispatcher would be moved there.) Are you against this idea? And if so, can I ask why?

Thanks for considering!

@mjangda, can you confirm your issue goes away after applying the patch? (It does on my project.)

[BinaryMuse]

There are a few things at play here.

First, not allowing cascading dispatches isn't a technical limitation; the decision to add that restriction was purposeful, based on the flux design pattern, and is even listed in the "Key Properties" on the "What is Flux" page. The restriction helps promote good data flow design by forcing you to think about your actions in terms of domain concepts rather than rote instructions your stores are following. Long story short, I'm not in favor of simply removing the cascading dispatch restriction and calling it a day.

The problem, as mentioned above, is that this restriction causes exceptions when one action causes a new component to be mounted, and that new component immediately fires another action in componentDid/WillMount. For what it's worth, I'm starting to feel like this is an antipattern. It's just another form of cascading dispatches; a button click dispatches ActionA, which causes a new component to mount, and that component's willMount dispatches ActionB, which causes a new component to mount, which dispatches ActionC... all of the sudden your application state is wildly different because of all the cascading actions, and this is the hard-to-debug scenario that the only-one-dispatch-at-a-time rule is supposed to solve. In fact, it's probably even more insidious because the dispatch ordering is implicitly defined by the order of new components mounting throughout the application.

That said, I also understand that this is a common pattern (and even implicitly recommend it in one of the examples on the site), so I think I'd like to support it if we can. The real problem here, and what makes it hard to debug, is that React renders immediately sometimes but not others. Take the following example (see it working at this JSFiddle):

var App = React.createClass({
  render: function() {
    console.log("Rendering");
    return (
      <div>
        <p>Value: {this.props.value}</p>
        <button onClick={this.sync}>Increment Synchronously</button>
        <button onClick={this.async}>Increment Asynchronously</button>
        <p>
          When incrementing synchronously, rendering happens on the same tick,
          but after the increment function has fully finished. When incrementing
          asynchronously, rendering happens immediately on the same tick,
          before the increment function has finised.
        </p>
      </div>
    );
  },

  sync: function() {
    this.props.increment();
  },

  async: function() {
    setTimeout(function() {
      this.props.increment();
    }.bind(this));
  }
});

var value = 0;

var increment = function() {
  console.group("Increment");
  setTimeout(function() {
    console.log("Next tick");
    console.groupEnd("Increment");
  });

  console.log("Starting increment");
  value++;
  render();
  console.log("Ended increment");
};

var render = function() {
  React.render(<App value={value} increment={increment} />, document.body);
};

render();

When you click the left button, the console logging ordering is:

Starting increment
Ended increment
Rendering
Next tick

However, clicking the right button, the order is:

Starting increment
Rendering
Ended increment
Next tick

This is the problem, I think—state changes caused by event handlers allow the handlers to fully process before React re-renders, allowing the dispatch loop to finish. However, actions that are dispatched asynchronously cause an re-render immediately, and Fluxxor throws since the dispatch loop hasn't completed.

So far, I've been unable to tell if this is on purpose or just a bug or side effect of the implementation used in React. I'll keep digging.

[paulstatezny]

@BinaryMuse Thanks for your quick response, for explaining the conflict, and for looking into a solution!

[BinaryMuse]

Okay, thanks to monastic-panic in IRC for accidentally providing a potential solution here.

Assuming you're using the latest version of React, please try the following code directly after you instantiate your Flux object and let me know if it helps:

// assuming a var `flux` containing your Flux instance...
var oldDispatch = flux.dispatcher.dispatch.bind(flux.dispatcher);
flux.dispatcher.dispatch = function(action) {
  React.addons.batchedUpdates(function() {
    oldDispatch(action);
  });
};

[paulstatezny]

Apologies for the delay. Give me another day or 2 on this. We're not using the latest React, so not sure if that is necessary yet.

[BinaryMuse]

@paulstatezny No worries. I think React.addons.batchedUpdates was exposed in React v0.12.0, but if you're using Browserify/webpack for React, you might be able to require it with var batchedUpdates = require("react/lib/ReactUpdates").batchedUpdates.

[paulstatezny]

@BinaryMuse One of my colleagues tried out your solution with batchedUpdates. Works like a charm! Is this making its way into Fluxxor?

[BryceHayden]

So I tried your solution @BinaryMuse but I couldn't get it to work...I'm not sure I'm implement it correctly. I"m sorry to post this on fluxxor forum as I'm not using your repo, but I was hoping you wouldn't mind explaining a little more. I explained my issue in more detail on stack overflow...if you feel up for helping me out then I'd appreciate it, if not I understand. here's the link to my post:

http://stackoverflow.com/questions/28759728/react-flux-react-router-dispatch-error-possible-batchupdates-solution

[jonaswindey]

I can confirm adding the batchedUpdates code after creating the flux instance is working.

Does this mean it's ok (in the flux flow) to dispatch actions while another one is dispatched?

[ruswerner]

@BinaryMuse When is this branch scheduled to land in master and npm? Thanks.

[BinaryMuse]

@ruswerner This has been published in v1.6.0