Do I need to use PrevState even if I spread the state into a variable?

Question

I am testing some code to try and understand the race condition regarding the use of setState().

my code can be found here

my code below:

import React from "react";

export default class App extends React.Component {
  state = {
    id: "",
    ids: [{ id: 7 }, { id: 14 }]
  };

  // here is where I create the id numbers
  uniqueIdCreatorHandler = incrementAmount => {
    let ids = [...this.state.ids];
    let highestId = 0;
    if (ids.length > 0) {
      highestId = ids
        .map(value => {
          return value.id;
        })
        .reduce((a, b) => {
          return Math.max(a, b);
        });
    }
    let newId = highestId + incrementAmount;
    ids.push({ id: newId });
    this.setState({ ids: ids });
  };

  idDeleterHanlder = currentIndex => {
    let ids = this.state.ids;
    ids.splice(currentIndex, 1);
    this.setState({ ids: ids });
  };
  //below is when I test performing the function twice, in order to figure if the result would be a race condition
  double = (firstIncrementAmount, secondIncrementAmount) => {
    this.uniqueIdCreatorHandler(firstIncrementAmount);
    this.uniqueIdCreatorHandler(secondIncrementAmount);
  };

  render() {
    let ids = this.state.ids.map((id, index) => {
      return (
        <p onClick={() => this.idDeleterHanlder(index)} key={id.id}>
          id:{id.id}
        </p>
      );
    });

    return (
      <div className="App">
        <button onClick={() => this.uniqueIdCreatorHandler(1)}>
          Push new id
        </button>
        <button onClick={() => this.double(1, 2)}>Add some Ids</button>
        <p>all ids below:</p>
        {ids}
      </div>
    );
  }
}

when invoking the double function on the second button only the secondIncrementAmount works. You can test it by changing the argument values on the call made on the onClick method.

I think that I should somehow use prevState on this.setState in order to fix this.

How could I avoid this issue here? This matter started at CodeReview but I did not realize how could I fix this.

There is also a recommendation to spread the mapped ids into Math.max and I could not figure out how and Why to do it. Isn't the creation of the new array by mapping the spreaded key values safe enough?

Answer 1

This isn't the most elegant solution but you can pass a callback to setState(see https://reactjs.org/docs/react-component.html#setstate).

If you modify uniqueIdCreatorHandler like this:

uniqueIdCreatorHandler = (incrementAmount, next) => {
  let ids = [...this.state.ids];
  let highestId = 0;
  if (ids.length > 0) {
    highestId = ids
      .map(value => {
        return value.id;
      })
      .reduce((a, b) => {
         return Math.max(a, b);
    });
  }
  let newId = highestId + incrementAmount;
  ids.push({ id: newId });
  this.setState({ ids: ids }, next); //next will be called once the setState is finished
};

You can call it inside double like this.

double = (firstIncrementAmount, secondIncrementAmount) => {
  this.uniqueIdCreatorHandler(
    firstIncrementAmount,
    () => this.uniqueIdCreatorHandler(secondIncrementAmount)
  );
};

Answer 2

.splice and .push mutate the array. Thus the current state then does not match the currently rendered version anymore. Instead, use .slice (or .filter) and [...old, new] for immutable stateupdates:

 deleteId = index => {
  this.setState(({ ids }) => ({ ids: ids.filter((id, i) => i !== index) }));
 };

 uniqueIdCreatorHandler = increment => {
   const highest = Math.max(0, ...this.state.ids.map(it => it.id ));    
   this.setState(({ ids }) => ({ ids: [...ids, { id: highest + increment }] }));
 };

Answer 3

setState can be asynchronous, batching up multiple changes and then applying them all at once. So when you spread the state you might be spreading an old version of the state and throwing out a change that should have happened.

The function version of setState avoids this. React guarantees that you will be passed in the most recent state, even if there's some other state update that you didn't know about. And then you can product the new state based on that.

There is also a recommendation to spread the mapped ids into Math.max and I could not figure out how and Why to do it

That's just to simplify the code for finding the max. Math.max can be passed an abitrary number of arguments, rather than just two at a time, so you don't need to use reduce to get the maximum of an array.

uniqueIdCreatorHandler = incrementAmount => {
  this.setState(prevState => {
    let ids = [...prevState.ids];
    let highestId = Math.max(...ids.map(value => value.id));
    let newId = highestId + incrementAmount;
    ids.push({ id: newId });
    this.setState({ ids: ids });    
  });
};