Pending state updates may be confusing

[sophiebits]

If you have

componentWillReceiveProps: function() {
    this.setState({x: this.state.x + 1});
    this.setState({x: this.state.x + 1});
}

then x will only be incremented once because the new state will just be stored in _pendingState until receiveProps finishes. After my refactoring in #115, this will never increment twice, which is probably confusing.

Maybe this isn't a bug. We can fix this behavior by instead updating this.state immediately (but still updating the UI later). Right now I can't think of any situations where having this.state out of sync with the UI is a problem.

@vjeux suggested I open an issue, so here I am. Probably easiest for me to fix while updating #115 though.

[jordwalke]

It might be best to think of setState as being enqueueStateUpdate. With that understanding, much of that confusion seems to go away. I have been pondering the exact same thing as you - should we let you "read" the state updates immediately after you write them, even if the UI has not been synchronized with it? At first I thought we should let you read the update immediately. But then I realized something: Not allowing reads of state after enqueueing the updates has an interesting effect. It encourage you to aggregate the entire update payload into a single object and execute a single setState. Since mutations are the hardest things in the world to understand, it makes sense to encourage squeezing them into small, centralized parts of your code. There's a theory that it's better to have all the crap in one pile, as opposed to spreading it out all over your code. Depending on how you feel about mutations, that stance might apply here. I haven't thought enough about this to feel strongly in either way, but I thought I'd share my recent thoughts - I've gone back and forth on this one.

[sophiebits]

Agree that isolating mutations to one place is good, but it sounds like you're suggesting making state-setting more awkward just to discourage it, which doesn't make sense to me.

Will continue to think about this as I finish up the setState batching. My gut feeling right now is that updating this.state immediately may simplify our code as well as being less confusing to the user – I'll see if that turns out to be the case.

[sophiebits]

Unfortunately this looks hard to change due to the arguments that the component lifecycle methods take.

[zpao]

Any more concerns about this? I'm guessing this is still an issue, though I'm not sure how often people are hitting it.

I think another option would be to simply restrict you to a single setState call at certain points in the lifecycle. We'd count in __DEV__, have invariants as needed, and then reset at the right times. Does that make sense?

[sophiebits]

Until we hear more, I think it's fine to close this out. I haven't heard of anyone actually getting confused.

[jordwalke]

One issue with updating this.state, even though you haven't flushed it accordingly, is that your system (React) is not consistent with itself. So for example, if you have a ref="myThing", and it accepts props that should be influenced by your state, this.refs.myThing.answerQuestion() returns an answer that does not take into account your state, but this.answerMyOwnQuestion() does take into account the change in this.state. I think we talked about adding a setStateSync() method for the times when you really want it to flush - though you don't get the performance benefit of your diff.

[sebmarkbage]

If you include closures in your render function as callbacks you can have similar problems. Likewise refs can be inconsistent, but also props if your parent doesn't flush down to you before your callback is invoked.

Deferred reconciliation (batching) puts the whole system in an inconsistent state and to do this correctly you really have to reconcile anything that you may have dependencies on during your side-effectful operation.

It turns out that this is a much more complex issue that this.state alone.

[dustingetz]

I think I have made progress on the double setState problem using a cursor-based approach. http://jsfiddle.net/dustingetz/SFGU5/

(Past discussion is also here: #629)

[dustingetz]

If anyone is paying attention, here is an improved cursor example that I consider idiomatic, no double setState problem, and uses === to implement shouldComponentUpdate
https://github.com/dustingetz/wingspan-cursor/blob/master/examples/helloworld/webapp/js/Page.js

[azoerb]

I just ran into this problem as well.

setAlarmTime: function(time) {
  this.setState({ alarmTime: time });
  this.checkAlarm();
},
checkAlarm: function() {
  this.setState({
    alarmSet: this.state.alarmTime > 0 && this.state.elapsedTime < this.state.alarmTime
  });
} ...

When calling setAlarmTime, I assumed that this.state.alarmTime would have been updated before the call to checkAlarm. It seems a bit ridiculous to have to manually track the actual state of alarmTime in order to be able to correctly call checkAlarm, and this is really making me worry about how often my "state" is actually correct now. Batch the state updates behind the scenes for rendering, sure, but don't make me have to keep track of what state is up-to-date!

Please fix this!

EDIT: So I can get around it in this case by placing the call to this.checkAlarm in the callback on setState, but there are other cases where the callback workaround isn't as simple / clean, and it just still seems counter-intuitive to have to do this.

[nambrot]

I'm also currently having the issue where I'm not really sure what this.state refers too. I agree that this.state should ideally refer to the latest state, regardless of whether it's visible in the UI.

[azoerb]

Edit: also check out Douglas' answer on this StackOverflow question as that may be enough to solve your problems without this workaround.

@nambrot if you're looking for a quick fix, what I ended up doing is creating a Utility function which wraps React.createClass and creates a separate version of the state that will always be up-to-date:

var Utils = new function() {
  this.createClass = function(object) {
    return React.createClass(
      $.extend(
        object,
        {
          _state: object.getInitialState ? object.getInitialState() : {},
          _setState: function(newState) {
            $.extend(this._state, newState);
            this.setState(newState);
          }
        }
      )
    );
  }
}

then just change your React.createClass(obj) calls to Utils.createClass(obj) and replace all calls to this.state with this._state and this.setState with this._setState. It's not super optimal, but unfortunately I was getting errors when trying to do a direct swap of setState with my new function.

You will also lose the generated displayName property (when using jsx they transform var anotherComponent = React.createClass({ into var anotherComponent = React.createClass({displayName: 'anotherComponent'),
so you'll just have to define it yourself if you want the correct tag names to show up in the react tools.

Disclaimer: this isn't super well tested, so let me know if you have any problems with it.

[abergs]

I ran into the same problem as @azoerb

[th0r]

+1, the same problem, as @azoerb
My current hack is to update this.state directly and call this.forceUpdate later. Can it break something?

[zpao]

@th0r forceUpdate will bypass shouldComponentUpdate

[RoyalIcing]

Could there just be a queuing function like changeState (or queueStateChange) that takes a closure that is queued? Your closure then returns the changed state to be merged. The closure could even be passed the current state as an argument possibly.

[briandipalma]

The other workaround is to move your state out of React and into flux stores. As long as you keep state of out React then you shouldn't have this problem.

[ptomasroos]

@briandipalma Can you explore this?
And just have everything passed through props and set no state within the react view?

I thought a common convention was to setState when flux stores change otherwise there will be a hard time getting the change to render from the event handler?

[briandipalma]

@ptomasroos Yes and the setState call is passed the state provided by the store, the React component doesn't calculate itself based on this.state.

[ptomasroos]

@briandipalma Good then we agree! Just wanted to be explicit. 👍

[aldendaniels]

DISCLAIMER: I'm new to React.

If I understand correctly, React's setState() method's only advantage over directly modifying component state and calling forceUpdate() is that setState() lets the developer avoid unnecessary re-renders by overloading shouldComponentUpdate().

For shouldComponentUpdate() to be effective, however, we need to use immutable data structures (e.g. ClosureScript + OM, React Immutability Helpers, ImmutableJS, etc.). Without immutability, shouldComponentUpdate() can't detect changes.

In fact, we often CAN'T use setState() when using mutable data. You can't remove an item from a list using setState() unless you've shallow cloned the whole list (think immutability).

Takeaways

1. If you're using immutable data structures, then use setState() and enjoy the confidence that this.state and this.refs are always in sync.
2. If you're using mutable data structures, modify this.state directly and call forceUpdate(). You'll have to live with this.state and this.refs being out of sync. On the bright side, you'll get to write VanillaJS update code and won't run into the confusion detailed in this issue.

Again - I'm new to React. Please clarify if I've got something wrong.

[leebyron]

@aldendaniels I think you've mixed two concepts.

You're 100% correct about PureRenderMixin's implementation of shouldComponentUpdate(). It is only safe if your props and state use immutable values (including string, number, bool).

However independent of any shouldComponentUpdate implementation, setState is still perfectly safe to use, even if you're using mutable data in your props and state.

In fact, you should never mutate this.state directly, as that is considered immutable.

[aldendaniels]

Hi @leebyron - thanks for reaching out.

you should never mutate this.state directly, as that is considered immutable

I'm a little dubious: from the React docs - "React lets you use whatever style of data management you want, including mutation". I also see that in Facebook's official TodoMVC Flux Example, state is not being treated as immutable.

Using immutable state in JavaScript without using a 3rd party library like those listed in my previous post quickly becomes slow and tedious when dealing with large, nested data sets.

If mutable state really is anti-pattern, then I'm all ears - but I'm also concerned by the invasiveness of the requirement.

[gaearon]

Wrote some thoughts about why React works this way: #11527 (comment)