2

I need to create a reducer that toggles the state of done using the id or index of the todo

state = {
    todos: [
      {
        title: "eat rice",
        done: false,
        id: 1
      },
      {
        title: "go fishing",
        done: true,
        id: 2
      },
      {
        title: "drink coffee",
        done: false,
        id: 3
      }
    ]
  }

I tried this but it mutates the state, the payload being the index of the object in the array.

case "DONE":
      const todos = [...state.todos];
      todos[action.payload].done = !todos[action.payload].done;
      return {
        ...state,
        todos: todos
      };
Improve this question

3 Answers 3

Reset to default
3

You could use a map function instead. The function will generate a new array which you can use to replaces todos with.

case "DONE":
  const newTodos = state.todos.map((todo, index) => {
    // Based on your code, I assume action.payload is the index of the todo in the array of todos
    if (index === action.payload) {
      const newTodo = {...todo};
      todo.done = !todo.done;
      return todo;
    }

    return todo;
  });

  return {
    ...state,
    todos: newTodos,
  };

If you don't want to iterate over every todo, you could do something else such as using slice to create a copy of the array and then change the one value:

case "DONE":
  const newTodos = todos.slice();
  const updatedTodo = {...newTodos[action.payload]};
  updatedTodo.done = !updatedTodo.done;
  newTodos[action.payload] = updatedTodo;

  return {
    ...state,
    todos: newTodos,
  };
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

This example line is still mutating: newTodos[action.paylod].done = !newTodos[action.paylod].done;. See the Redux docs page on "Immutable Update Patterns" for an explanation, as well as Dave Ceddia's post The Complete Guide to Immutability in React and Redux.
Using redux-logger, it shows that both methods mutate the state. Any other way around it?
Hmm. I guess you got it working, but I understand the issue from the docs @markerikson linked. The solution would be to create a new object for the one being replaced: const newTodo = { ...todos[action.payload] }; newTodo[action.payload].done = !newTodo[action.payload].done; That should create a new copy of both the array and the individual object within it.
The code snippet in this answer is still incorrect. const newTodo = todo; todo.done = !todo.done; is making one of the mistakes referenced in the docs page - it's simply another variable pointing to the same object in memory, and still mutating it. I'll edit the answer to fix the problem.
0

Found the answer. Thanks for the contributions.

case "DONE":
      const newTodos = state.todos.map((todo, index) => {
        if (index === action.payload) {
          const newTodo = { ...todo };
          newTodo.done = !newTodo.done;
          return newTodo;
        }

        return todo;
      });

      return {
        ...state,
        todos: newTodos
      };
Improve this answer

1 Comment

Hah cool, we came up with pretty much the same thing. See my answer for why stackoverflow.com/a/52600197/271442
0

Using the spread operator or map will create a new array but will not automatically clone the contained objects, as JavaScript follows "pass by reference". You'd have to clone the object as well. So maybe something like

case "DONE":
  const todos = state.todos.map((todo, index) => {
    const newTodo = {...todo};
    if (action.payload === index) {
      newTodo.done = !todo.done;
    }
    return newTodo;
  });

  return {
    ...state,
    todos,
  };

Of course you could also use a clone utility or something like Immutable.js.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.