-2

I am using React components which look like this (a simplified version of the components I used, below).

My question is: how to make the same but using this.setState? The below code works, but I am mutating the state directly, and I am receiving the following warning:

Do not mutate state directly. Use setState()

class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      playerState: [
        {
          name: 'Jack',
          hp: 30
        },{
          name: 'Alan',
          hp: 28
        }
      ],
    };
  }
  lowerPlayerHealth = (index) => () => {
    this.state.playerState[index].hp = this.state.playerState[index].hp - 1
    this.forceUpdate();
  }
  render() {
    return (
      <div className="App">
        <p>Player 1: {this.state.playerState[0].name}</p>
        <p>Health: {this.state.playerState[0].hp}</p>
        <button onClick={this.lowerPlayerHealth(0)}>Hit player 1</button>
        <p>Player 2: {this.state.playerState[1].name}</p>
        <p>Health: {this.state.playerState[1].hp}</p>
        <button onClick={this.lowerPlayerHealth(1)}>Hit player 2</button>
      </div>
    );
  }
}

When rendered, it looks like this:

enter image description here

luismartinezs
  • 199
  • 1
  • 2
  • 11
  • 3
    A simple read through the [setState() docs](https://reactjs.org/docs/react-component.html#setstate) would make this a trivial problem to solve yourself – charlietfl Oct 14 '18 at 20:08
  • @charlietfl I'll read over it – luismartinezs Oct 14 '18 at 20:10
  • if `this.state.playerState[index]` is passed as a prop into child component and this child component is `extends PureComponent` it will never re-render on change. It's just a first use-case when mutating state can completely break something in your app.there are a set of reasons following the docs and never mutating state. – skyboyer Oct 15 '18 at 06:23

2 Answers2

2

If you want to modify an existing value in the state, you should never get the value directly from the state and update the state object, but rather use the updater function in setState so you can guarantee the state values are the ones you need at the time of updating the state. This is just how React's state works and it's a very common React mistake.

From the official docs

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(updater, callback)), either of which are guaranteed to fire after the update has been applied. If you need to set the state based on the previous state, read about the updater argument below.

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.

The first argument is an updater function with the signature:

(state, props) => stateChange

state is a reference to the component state at the time the change is being applied. It should not be directly mutated. Instead, changes should be represented by building a new object based on the input from state and props.

Both state and props received by the updater function are guaranteed to be up-to-date. The output of the updater is shallowly merged with state.

So you must get the value exactly when you want to update the component inside the setState function using the first argument of the updater function.

lowerPlayerHealth = (index) => () => {
    // use setState rather than mutating the state directly
    this.setState((state, props) => {
    // state here is the current state. Use it to update the current value found in state and ensure that it will be set correctly
      return (state); // update state ensuring the correct values
    });
  }

Solution

To lower a value found in state:

class App extends React.Component {
  constructor(props){
    super(props);
    this.state = {
      playerState: [
        {
          name: 'Jack',
          hp: 30
        }, {
          name: 'Alan',
          hp: 28
        }
      ],
      };
  }

  lowerPlayerHealth = (index) => () => {
    this.setState((state, props) => {
      state.playerState[index].hp -=1; //update the current value found in state
      return (state); // update state ensuring the correct values
    });
  }

  render() {
    return (
      <div className="App">
        <p>Player 1: {this.state.playerState[0].name}</p>
        <p>Health: {this.state.playerState[0].hp}</p>
        <button onClick={this.lowerPlayerHealth(0)}>Hit player 1</button>
        <p>Player 2: {this.state.playerState[1].name}</p>
        <p>Health: {this.state.playerState[1].hp}</p>
        <button onClick={this.lowerPlayerHealth(1)}>Hit player 2</button>
      </div>
      );
  }
}
c-chavez
  • 7,237
  • 5
  • 35
  • 49
  • using React we should never mutate state. your code does it. – skyboyer Oct 16 '18 at 05:18
  • @skyboyer what do you mean with "we should never mutate the state", do you mean we should never use setState? I'm lost... – c-chavez Oct 16 '18 at 06:13
  • I mean our code with `setState` should work the way when you return new object each time when need to change single property deep nested. otherwise `PureComponent` will never re-render. Some details: https://daveceddia.com/why-not-modify-react-state-directly/ – skyboyer Oct 16 '18 at 06:17
  • @skyboyer actually you should not use PureComponent with objects with deep nested properties... or maybe I'm wrong. [about PureComponent and state](https://reactjs.org/docs/react-api.html#reactpurecomponent) states "Only extend PureComponent when you expect to have simple props and state". Could you please expand your idea so I can have a proper read on the matter and learn about this? thank you! – c-chavez Oct 16 '18 at 06:25
  • `PureComponent` actually works with complex structures/nested objects. but if for `Component` state mutation rarely can cause issues for `PureComponent` it will definitely break something - and since it's easier to control when props are plain values - it's suggested. nothing special behind actually. – skyboyer Oct 16 '18 at 06:42
  • 1
    @skyboyer I think we are off the track here... it is suggested that PureComponent should not use complex objects, but of course it will work if you do it properly. The main question was about mutating the state directly, and using setState and mutating the object within setState is the way to go and to solve this issue as stated in the answer's documentation. Please correct me if I'm wrong and feel free to update the answer if you feel it's doing something improper. It's always good to learn something new and to correct any mistakes. – c-chavez Oct 16 '18 at 06:56
  • no, it really works. I still believe it is not _React-way_ but it would be rather opinion-based than _no-no_ thing. I'd rather create a dedicated answer with my thoughts. – skyboyer Oct 19 '18 at 12:29
1

You've answered your own question: don't mutate state. Also, best practice suggests using the function version of setState.

Since playerState is an array, use Array.map to create a new array containing the same objects, replacing only the one you want to change:

lowerPlayerHealth = (indexToUpdate) => () => {
  this.setState(state => ({
    ...state,
    playerState: state.playerState.map(
      (item, index) => index === indexToUpdate
        ? {
          ...item,
          hp: item.hp - 1
        }
        : oldItem
    )
  }));
}

If you made playerState an object instead of an array, you can make it tidier:

lowerPlayerHealth = (indexToUpdate) => () => {
  this.setState(state => ({
    ...state,
    playerState: {
      ...state.playerState,
      [indexToUpdate]: {
        ...state.playerState[indexToUpdate],
        hp: state.playerState[idToindexToUpdatepdate].hp - 1
      }
    }
  }));
}
Alex McMillan
  • 17,096
  • 12
  • 55
  • 88
  • `this.state.playerState` should be an object not an array to allow `[index]:` approach. for array it should be another `.map()`-based pattern used. could you update your answer? – skyboyer Oct 15 '18 at 06:25
  • @skyboyer array-like indexing works for objects and arrays. I made you [an example](https://jsbin.com/nabivejuzo/1/edit?html,js,output) – Alex McMillan Oct 15 '18 at 10:38
  • but you are converting array into object. if you keep `playerState` as array you will get `unexpected token` error – skyboyer Oct 15 '18 at 10:53
  • @skyboyer you're right. How would you approach this keeping it as a single `this.setState` call but retaining the array? (please feel free to edit my answer) – Alex McMillan Oct 15 '18 at 21:52
  • done, check updated version – skyboyer Oct 16 '18 at 05:55