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

Docs: Clarification of setState() behavior #9329

Merged
merged 5 commits into from
Apr 13, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions docs/docs/reference-react-component.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,34 +230,53 @@ componentWillUnmount()
### `setState()`

```javascript
setState(nextState, callback)
setState(updater, [callback])
```

Performs a shallow merge of nextState into current state. This is the primary method you use to trigger UI updates from event handlers and server request callbacks.
`setState()` is an asynchronous method which enqueues changes to be made to the state during the next update cycle. This is the primary method you use to trigger UI updates in response to UI event handlers, server responses, etc...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably get away with removing UI here

trigger updates in response to event handlers, server responses, etc...


The first argument can be an object (containing zero or more keys to update) or a function (of state and props) that returns an object containing keys to update.
`setState()` does not immediately mutate `this.state` but creates a pending state transition. Accessing `this.state` after calling this method can potentially return the previous state, rather than the state after enqueued updates have been applied. This is a common source of bugs in React applications.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing this.state after calling this method can potentially return the previous state

It could also return a future state that hasn't flushed yet, or a state that was aborted completely.

This is a common source of bugs in React applications.

Source? This seems overly alarmist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this from mentoring students & product teams who are making the switch to React. I saw two examples yesterday. See setState Gate. Further evidence is available by searching for setState on Stack Overflow (there are MANY more examples of confused setState users on Stack Overflow).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

potentially return the previous state

That bit is just a slight rephrase of the current doc, which says "potentially return the existing value".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe

setState() does not immediately mutate this.state. Instead, React mutates this.state when it applies the enqueued update, and this may happen later than the setState() call. This makes reading this.state right after calling setState() a potential pitfall.

?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better, but I'm concerned that it implies that this.state is a source of truth. You can mostly get away with this assumption in sync mode, with the exception that you describe here. But this won't be the case with async.

What I'd like to write is

You should only read this.state (or this.props) inside a lifecycle method or render. It's not safe in an event handler, promise handler, timeout callback, or any place in the stack above React, where you should use this.setState((state, props) => stateUpdate) instead.

but this might be too prescriptive. Because again, this isn't really an issue in sync mode, only with async. And we'll likely have to come up with a new API for async mode, anyway, at which point this setState doc will be moot.

Maybe this is a good way to split the difference:

setState() does not always immediately update the component. It may batch or defer the update until later. This makes reading this.state right after calling setState() a potential pitfall. Instead, use componentDidUpdate or a setState callback (setState(update, callback)), either of which are guaranteed to fire after the update has been applied.


Here is the simple object usage:
There is no guarantee of synchronous operation of calls to `setState`.

`setState()` will always lead to a re-render unless `shouldComponentUpdate()` returns `false`. If mutable objects are being used and conditional rendering logic cannot be implemented in `shouldComponentUpdate()`, calling `setState()` only when the new state differs from the previous state will avoid unnecessary re-renders.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part feels a little too opaque for beginners

If mutable objects are being used and conditional rendering logic cannot be implemented in shouldComponentUpdate()

What does conditional rendering logic have to do with shouldComponentUpdate and setState?

calling setState() only when the new state differs from the previous state will avoid unnecessary re-renders.

Don't you only get the new state by calling setState in the first place? I get what you mean, but it's slightly ambiguous.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't write this part. It's already in the docs. Maybe cleanup of that wording could happen in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the diff made it look like you did. It would be nice to simplify this here or in another PR 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. I wasn't sure what to do with it, so I left it alone for now. =)


The first argument is an updater function with the signature:

```javascript
this.setState({mykey: 'my new value'});
(prevState, props) => nextState
```

It's also possible to pass a function with the signature `function(state, props) => newState`. This enqueues an atomic update that consults the previous value of state and props before setting any values. For instance, suppose we wanted to increment a value in state by `props.step`:
`prevState` is a reference to the previous state. It should not be directly mutated. Instead, changes should be represented by building a `nextState` based on the input from `prevState` and `props`. For instance, suppose we wanted to increment a value in state by `props.step`:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

building a nextState

Maybe change to

build a new state object

As it's unclear what nextState is if you're not already familiar with setState callback signature

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. =)


```javascript
this.setState((prevState, props) => {
return {myInteger: prevState.myInteger + props.step};
});
```

The second parameter is an optional callback function that will be executed once `setState` is completed and the component is re-rendered. Generally we recommend using `componentDidUpdate()` for such logic instead.
The second parameter to `setState()` is an optional callback function that will be executed once `setState` is completed and the component is re-rendered. Generally we recommend using `componentDidUpdate()` for such logic instead.

You may optionally pass an object as the first argument to `setState()` instead of a function:

```javascript
setState(stateChange, callback)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use an object literal instead so it's a little more familiar? Maybe use the same example above that updates myInteger

setState({ myInteger: this.state.myInteger  + this.props.step })

That could also serve as an example of a situation where the setState callback is preferred, as this.state.myInteger may not be totally up-to-date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended to show the signature, not an example, but you make a good point that an example would be helpful.

```

This performs a shallow merge of `stateChange` into the new state. This form of `setState()` is also asynchronous, and multiple calls during the same cycle may be batched together, which will result in the equivalent of:

`setState()` does not immediately mutate `this.state` but creates a pending state transition. Accessing `this.state` after calling this method can potentially return the existing value.
```javaScript
Object.assign(
previousState,
stateChange1,
stateChange2,
...
)
```

There is no guarantee of synchronous operation of calls to `setState` and calls may be batched for performance gains.
Subsequent calls may override values from previous calls. If the next state depends on the previous state, we recommend using the updater function form, instead.

`setState()` will always lead to a re-render unless `shouldComponentUpdate()` returns `false`. If mutable objects are being used and conditional rendering logic cannot be implemented in `shouldComponentUpdate()`, calling `setState()` only when the new state differs from the previous state will avoid unnecessary re-renders.
For more detail, see the [State and Lifecycle guide](/react/docs/state-and-lifecycle.html).

* * *

Expand Down