Avoid using callback in setState when referencing the previous state

Question

I still get this error but I don't understand how it is possible in my code:

export default class GenericTable extends React.PureComponent {
  constructor(props) {
    super(props);

    this.state = {
      otherStuff: '',
      ...props, // rows are received on props
      sort: {
        col: 0,
        asc: true,
      },
    };
  }

  onSortChange = i => {
    const tempRows = this.state.rows;
    const tempSort = this.state.sort; // this is the line where it says about that error

    const newRows = [];
    if (tempSort.asc) {
      newRows = tempRows.sort(function(a, b) {
        if (a.cells[i] < b.cells[i]) {
          return -1;
        }
        if (a.cells[i] > b.cells[i]) {
          return 1;
        }
        return 0;
      });
    } else {
      newRows = tempRows.sort(function(a, b) {
        if (a.cells[i] > b.cells[i]) {
          return -1;
        }
        if (a.cells[i] < b.cells[i]) {
          return 1;
        }
        return 0;
      });
    }
    tempSort.col = [i];
    tempSort.asc = !tempSort.asc;
    this.setState({ rows: newRows, sort: tempSort });
  };
...
}

So, it is where tempSort is declared. How should it be changed in order to work?

I don't see the similarity with the described on eslint's page.

Answer 1

You'll need to use this form of setState which accepts a function called with the previous state so logic inside the function operates on the same state. In the end it should return the newly computed state.

import React from 'react';

export default class MyComponent extends React.PureComponent {
    constructor(props) {
        super(props);

        this.state = {
            otherStuff: '',
            ...props, // rows are received on props
            sort: {
                col: 0,
                asc: true
            }
        };
    }

    getNewState(prevState, i) {
        const tempSort = prevState.sort;
        const tempRows = this.state.rows;
        let newRows = [];

        if (tempSort.asc) {
            newRows = tempRows.sort(function(a, b) {
                if (a.cells[i] < b.cells[i]) {
                    return -1;
                }
                if (a.cells[i] > b.cells[i]) {
                    return 1;
                }
                return 0;
            });
        } else {
            newRows = tempRows.sort(function(a, b) {
                if (a.cells[i] > b.cells[i]) {
                    return -1;
                }
                if (a.cells[i] < b.cells[i]) {
                    return 1;
                }
                return 0;
            });
        }
        tempSort.col = [i];
        tempSort.asc = !tempSort.asc;
        return { rows: newRows, sort: tempSort };
    }

    onSortChange = i => {
        this.setState(prevState => {
            return this.getNewState(prevState, i);
        });
    };

    render() {
        return <p>test</p>;
    }
}

Tested in on my VS Code with the rule in question added to my .eslintrc.js. But nevertheless, keep in mind that putting your state computation logic in the version with the prevState param doesn't prevent you from introducing other issues (as I mentioned in my previous version of the answer, not creating new objects, but mutating existing ones, where the first was expected. I removed that part of the answer as it actually doesn't solve that particular issue, but I suggest you check it again in the edit history).

Answer 2

The React Main Concepts - 5. State and Lifecycle lists rules to follow:

Using State Correctly

There are three things you should know about setState().

Do Not Modify State Directly

For example, this will not re-render a component:

// Wrong
this.state.comment = 'Hello';

Instead, use setState():

// Correct
this.setState({comment: 'Hello'});

The only place where you can assign this.state is the constructor.

State Updates May Be Asynchronous

React may batch multiple setState() calls into a single update for performance.

Because this.props and this.state may be updated asynchronously, you should not rely on their values for calculating the next state.

For example, this code may fail to update the counter:

// Wrong
this.setState({
  counter: this.state.counter + this.props.increment,
});

To fix it, use a second form of setState() that accepts a function rather than an object. That function will receive the previous state as the first argument, and the props at the time the update is applied as the second argument:

// Correct
this.setState((state, props) => ({
  counter: state.counter + props.increment
}));

We used an arrow function above, but it also works with regular functions:

// Correct
this.setState(function(state, props) {
  return {
    counter: state.counter + props.increment
  };
});

State Updates are Merged

When you call setState(), React merges the object you provide into the current state.

For example, your state may contain several independent variables:

constructor(props) {
  super(props);
  this.state = {
    posts: [],
    comments: []
  };
}

Then you can update them independently with separate setState() calls:

componentDidMount() {
  fetchPosts().then(response => {
    this.setState({
      posts: response.posts
    });
  });

  fetchComments().then(response => {
    this.setState({
      comments: response.comments
    });
  });
}

The merging is shallow, so this.setState({comments}) leaves this.state.posts intact, but completely replaces this.state.comments.

With your provided code snippet you break two of these rules. Let's first handle the one that causes the JSLint warning:

Prevent using this.state within a this.setState (react/no-access-state-in-setstate)

You get this warning because of:

State Updates May Be Asynchronous ... Because this.props and this.state may be updated asynchronously, you should not rely on their values for calculating the next state.

Both newRows and tempSort are based upon the previous state. The JSLint warning is saying that you should use the callback variation of setState().

onSortChange = i => {
  this.setState(({ rows: tempRows, sort: tempSort }) => {
    // sort
    return { rows: newRows, sort: tempSort };
  });
};

With the warning handled let's have a look at another issue within the snippet. sort() sorts the elements in-place, meaning that the original is modified. Similarly tempSort.col = [i] and tempSort.asc = !tempSort.asc also both modify the current state. Assigning an object to a new variable does not create a copy. Both variables will simply refer to the same object.

const object_1 = { a: 1 };
const object_2 = object_1;

object_2.a = 2;

console.log("object_1", object_1);
console.log("object_2", object_2);

Make sure you copy an object before mutating an object or using in-place methods.

 const tempRows = [...this.state.rows];
 const tempSort = {...this.state.sort};

Both create a shallow copy of the array/object before assigning it to a variable.

With the above being said a solution might look something like this:

/* Comparator factories. sort[true] is ascending, sort[false] is descending.
 * This factory accepts a function that is called for both a and b,
 * comparing their return values.
 *
 *     items.sort(sort[true](item => item.a))
 *
 * Sorts an array of items based on their "a" property in ascending order.
 */
const sort = {
   true: (fn) => (a, b) => (a = fn(a), b = fn(b), -(a < b) || +(a > b)),
  // flip a, b arguments for descending order
  false: (fn) => (a, b) => sort[true](fn)(b, a),
};

class GenericTable extends React.Component {
  constructor(props) {
    super(props);
    this.state = { ordered: props.data };
  }
    
  handler(i) {
    // use a callback because the new state depends on the old state
    this.setState(({ordered, i: iPrevious, asc}) => {
      asc = i != iPrevious || !asc;
      return { 
        i, asc,
        // copy `ordered` before using the in-place `sort` method
        ordered: [...ordered].sort(sort[asc](row => row[i]))
      };
    });
  }

  render() {
    return (
      <table>
        <thead>
          <tr>{this.props.headers.map((header, i) => (
            <th key={i} onClick={() => this.handler(i)}>{header}</th>
          ))}</tr>
        </thead>
        <tbody>
          {this.state.ordered.map((row, i) => (
            <tr key={i}>
              {row.map((cell, i) => <td key={i}>{cell}</td>)}
            </tr>
          ))}
        </tbody>
      </table>
    );
  }
}

ReactDOM.render(
  <GenericTable
    headers={["a", "b", "c"]}
    data={[[1, 5, 10], [6, 11, 2], [12, 3, 7]]}
  />,
  document.querySelector("#root")
);

th{cursor: pointer}th,td{border:1px solid black}

<script src="https://unpkg.com/react@17/umd/react.development.js"></script>
<script src="https://unpkg.com/react-dom@17/umd/react-dom.development.js"></script>
<div id="root"></div>