Why doesn't React update my state correctly?-CodePudding

I am displaying my state, which is an array, by iterating through it with the map function. Additionally, I have a button that reverses the array on click.

import React, { useState, useEffect } from "react";
import PropTypes from "prop-types";
import "./App.css";

const App = (props) => {
  const [reverse, setReverse] = useState(false);

  const [arr, setArr] = useState([]);

  useEffect(() => {
    let newArr = props.arr;

    newArr = newArr.reverse();

    setArr(newArr);
  }, [reverse, props.arr]);

  return (
    <div className="App">
      <button
        onClick={() => {
          setReverse(!reverse);
        }}
      >
        Reverse
      </button>
      {arr.map((e, i) => {
        return (
          <div key={i}>
            <p>{e}</p>
          </div>
        );
      })}
    </div>
  );
};

App.propTypes = {
  arr: PropTypes.array,
};

export default App;

I think it is fairly obvious what I want to do. But it doesn't work for me and I don't know why. I have to click twice to accomplish the first reversion and the weird case emerges in which the array that is rendered and the one shown in the component state by the React dev tools in Chrome do not match.

I cannot explain this behaviour. I am starting to think that it has something to do with the fact that I'm getting the array from the props, but I don't really know. Any ideas?

CodePudding user response：

There are several improvements to be made. Some are bugs, and others are just anti-patterns.

The first thing to note is that reverse mutates the original array. You should never mutate state or props in React.

The second is an anti-pattern involving copying props into state that do not need to be.

The React docs say that state is for values that change over time. It says that a value is not state if it can be derived from props.

Can you compute it based on any other state or props in your component? If so, it isn’t state.

That is what we have in this case.

Instead of copying the prop arr into state and reversing it there, we can just use the prop and a temporary variable to accomplish the same goal with less code.

const {useState, useEffect} = React;

const App = (props) => {
  const [reverse, setReverse] = useState(false);
  
  let array = [...props.arr]; // New reference so we will not mutate props
  if (reverse) {
    array.reverse();
  }

  return (
    <div className="App">
      <button
        onClick={() => {
          setReverse(!reverse);
        }}
      >
        Reverse
      </button>
      {array.map((e, i) => {
        return (
          <div key={i}>
            <p>{e}</p>
          </div>
        );
      })}
    </div>
  );
};

ReactDOM.render(<App arr={[1,2,3]} />, document.getElementById('root'));

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.4/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.4/umd/react-dom.production.min.js"></script>
<div id="root"></div>

CodePudding user response：

That's because the useEffect() hook runs on the mount of your component and reverses your array although you haven't clicked the reverse button.

Try adding logic to avoid the first reversal that is happening on the mount.

This would probably work for you,

  useEffect(() => {
      if(arr.length>0){
         let newArr = props.arr;
         newArr = newArr.reverse();
         setArr(newArr);
      }
  }, [reverse, props.arr]);

CodePudding user response：

The main mistake is that you are assigning your props to the newArr variable, not arr, so each time it is not the new state of arr that is reversed, but the original array of props. Also, when changing the props, it is better to add one more useEffect. I have provided an example of the correct code below I apologize if there are mistakes, I do not speak English well

  useEffect(() => {
    setArr(props.arr);
  }, [props.arr]);

  useEffect(() => {
    if (!arr.length) return;

    let newArr = arr.length ? [...arr] : [...props.arr];
    newArr = [...newArr.reverse()];
    setArr(newArr);
  }, [reverse]);