1

I want to know how to not mutate state in the following scenario:

I'm using React Hooks. I have a TodoList that lists TodoItems. TodoItems can be clicked to run a function called toggle which toggles their completed property.

import React, { useState } from "react";
import TodoItem from "./TodoItem";

export default function TodoList() {
  const [todos, setTodos] = useState([
    { text: "Example", completed: false }
  ]);

  const toggle = index => {
    const newTodos = [...todos];
    newTodos[index].completed = !newTodos[index].completed; // Mutating state?
    setTodos(newTodos);
  };

  return (
    <>
      {todos.map((item, index) => (
        <TodoItem key={index} index={index} todo={item} toggle={toggle} />
      ))}
    </>
  );
}

I was told that this was mutating state:

You create a copy of the todos array, but you still mutate an item inside the array:

newTodos[index].completed = !newTodos[index].completed;

Spreading the array only creates a shallow copy of the array, but the original todo object is not copied and thus mutated.

So, what is the correct way to handle this without mutating state? I've tried passing in a prevTodos param, but then the state doesn't update:

const toggle = index => {
  setTodos(prevTodos => {
    prevTodos[index].completed = !prevTodos[index].completed;
    return prevTodos;
  });
};

Update: I've taken a look at the posts you've all recommended. This is a solution, although I'm wondering if there's a cleaner way to express it:

const toggle = index => {
  setTodos(
    todos.map((todo, i) =>
      i === index ? { ...todo, completed: !todo.completed } : todo
    )
  );
};

Update 2: Thanks everyone. I posted the solution below. StackOverflow won't let me accept it for 2 days, but I'll do it then. The accepted answer on the suggested post suggests the use of a third-party library, and it doesn't use React Hooks, so I don't consider this question a duplicate.

Em.
  • 175
  • 7
  • https://stackoverflow.com/questions/55987369/why-is-the-state-mutating-if-i-am-using-spread-operator – LoXatoR Feb 14 '20 at 16:39
  • Does this answer your question? [Whats the best way to update an object in an array in ReactJS?](https://stackoverflow.com/questions/28121272/whats-the-best-way-to-update-an-object-in-an-array-in-reactjs) – Emile Bergeron Feb 14 '20 at 16:40
  • `prevTodos[index].completed = /*...*/` mutates the previous state, which is equally bad. – Emile Bergeron Feb 14 '20 at 16:43
  • Thanks @EmileBergeron. I've taken a look at those posts and came up with a solution (updated my original post). I'm wondering if that's the cleanest way to express it. – Em. Feb 14 '20 at 17:12
  • Yes it works! Though you should either post it as an answer to your own question and accept it, or just close your question as a duplicate of the one I've linked. – Emile Bergeron Feb 14 '20 at 17:19

3 Answers3

1

That's right, the simplest way to avoid this problem is to avoid mutating data.

One way to do this is using a functional approach, specifically with a a non mutating array method like .map:

function TodoList() {
  const [todos, setTodos] = React.useState([{
    text: "Example",
    completed: true
  }]);

  const toggle = index => {
    const updatedTodos = todos.map((todo, i) => {
      if (i === index) {
        todo.completed = !todo.completed;
      }
      return todo;
    });
    setTodos(updatedTodos);
  };

  return (
    <React.Fragment > 
      {todos.map((item, index) => (
        <p
          key={item.text}
          onClick={() => toggle(index)}
          className={item.completed ? "completed" : ""}
        >
          {item.text}
        </p>
       ))}
    </React.Fragment>
  );
}

ReactDOM.render( < TodoList / > , document.getElementById('root'))
.completed {
  text-decoration: line-through;
}
<script src="https://unpkg.com/react@16/umd/react.production.min.js"></script>
<script src="https://unpkg.com/react-dom@16/umd/react-dom.production.min.js"></script>
<div id="root"></div>
Juan Marco
  • 3,081
  • 2
  • 28
  • 32
0

Found a solution. map will return a different array, and you can listen to the index parameter (in my snippet below, this is i) to listen to the current index and change the object you're targeting.

const toggle = index => {
  setTodos(
    todos.map((todo, i) =>
      i === index ? { ...todo, completed: !todo.completed } : todo
    )
  );
};

These threads helped me piece this together:

Thanks to everyone that helped.

Em.
  • 175
  • 7
-3
const toggle = (index) => {
  const oldTodo = todos[index];
  setTodos(oldState => {
   oldState.splice(index, 1, {...oldTodo, checked: !oldTodo.checked})

   return oldState
})
}
rnmalone
  • 1,020
  • 13
  • 25