Search code examples
javascriptreactjsreact-reduxtypeerrorredux-toolkit

TypeError: Cannot read properties of undefined (reading 'id') only when more than 1 item in array


I can't figure out why I get this error when there is more than 1 item on the todos - which only seems to happen when I click save changes. I am using React Redux to make a todo list.

What is wrong with my code?

App.js

import React from "react";
import "./App.css";
import "bootstrap/dist/css/bootstrap.min.css";
import AddTodo from "./components/AddTodo";
import TodoItem from "./components/TodoItem";
import { useSelector } from "react-redux";

function App() {
  const todos = useSelector((state) => state.todos);

  return (
    <div className="App">
      <AddTodo />
      <div className="todoList">
        {todos && todos.map((todo) => (
          <TodoItem key={todo.id} todo={todo} />
        ))}
      </div>
    </div>
  );
}

export default App;

Reducer (slice.js)

import { createSlice } from "@reduxjs/toolkit";

const todoSlice = createSlice({
  name: "todos",
  initialState: [
    {
      id: 1,
      item: "Example1",
      completed: false,
    },
    {
      id: 2,
      item: "Example2",
      completed: false,
    },
  ],
  reducers: {
    addTodo: (state, action) => {
      state.push(action.payload);
    },

    editTodo: (state, action) => {
      return state.map((todo) => {
        if (todo.id === action.payload.id) {
          return { ...todo, item: action.payload.item };
        }
      });
    },

    deleteTodo: (state, action) => {
      return state.filter((todo) => todo.id !== action.payload);
    },

    completeTodo: (state, action) => {
      return state.map((todo) => {
        if (todo.id === action.payload) {
          return { ...todo, completed: !todo.completed };
        }
        return todo;
      });
    },
  },
});

export const { addTodo, selectTodo, editTodo, deleteTodo, completeTodo } =
  todoSlice.actions;

export default todoSlice.reducer;

TodoItem.js

import React, { useState } from "react";
import { useDispatch } from "react-redux";
import { editTodo, deleteTodo, completeTodo, selectTodo } from "../store/slice";
import Button from "react-bootstrap/Button";
import Modal from "react-bootstrap/Modal";
import Form from "react-bootstrap/Form";

export default function TodoItem({ todo }) {
  const dispatch = useDispatch();
  const [showEdit, setShowEdit] = useState(false);
  const [editText, setEditText] = useState(todo.text);

  function handleDeleteTodo() {
    dispatch(deleteTodo(todo.id));
  };
  
  function handleCompleteTodo() {
    dispatch(completeTodo(todo.id));
  };

  function handleEditSave(){ 
      dispatch(editTodo({
        id: todo.id,
        item: editText,
      }));
      console.log(editText);
      setShowEdit(false);
    }
  
  return (
    <div>
      <h4
        style={{
          textDecoration: todo.completed ? "line-through" : "none",
          color: todo.completed ? "grey" : "black",
        }}
      >
        {todo?.item}
      </h4>
      <Button variant="success" onClick={handleDeleteTodo}>
        Delete
      </Button>

      <Button variant="success" onClick={handleCompleteTodo}>
        {todo?.completed ? "Incomplete" : "Complete"}
      </Button>

      <Button
        variant="primary"
        onClick={() => {
          setShowEdit(true);
        }}
      >
        Edit
      </Button>

      <Modal show={showEdit} onHide={() => setShowEdit(false)}>
        <Modal.Header closeButton>
          <Modal.Title>Edit</Modal.Title>
        </Modal.Header>
        <Modal.Body>
          <Form.Label> Edit To-do </Form.Label>
          <Form.Control
            type="text"
            value={editText ?? ''}
            placeholder={todo.item}
            onChange={(e) => setEditText(e.target.value)}
          />
        </Modal.Body>
        <Modal.Footer>
          <Button
            variant="primary"
            onClick={handleEditSave}
          >
            Save Changes
          </Button>
          <Button variant="secondary" onClick={() => setShowEdit(false)}>
            Close
          </Button>
        </Modal.Footer>
      </Modal>
    </div>
  );
}

AddTodo.js

import React, { useState } from "react";
import { useSelector, useDispatch } from "react-redux";
import { addTodo } from "../store/slice";

export default function AddTodo() {
  const [text, setText] = useState("");
  const todos = useSelector((state) => state.todos);
  const dispatch = useDispatch();

  function handleAddTodo(todo) {
    if (text === "") {
      alert("Input is Empty");
    } else {
      dispatch(
        addTodo({
          id: Date.now(),
          item: todo,
          completed: false,
        })
      );
      console.log({ todos });
      setText("");
    }
  }

  return (
    <div>
      <input
        type="text"
        value={text}
        onChange={(e) => setText(e.target.value)}
      />
      <button onClick={() => handleAddTodo(text)}>Add Todo</button>
    </div>
  );
}

I have tried changing my reducer, but nothing changed. I find it odd that it will only come up with the error if I have more than 1 things on the todos.


Solution

  • The editTodo reducer case returns undefined for all todo array elements that are not the one you are editing.

    editTodo: (state, action) => {
      return state.map((todo) => {
        if (todo.id === action.payload.id) {
          return { ...todo, item: action.payload.item };
        }
        // <-- Missing todo elements that are not modified!!
      });
    },
    

    When mapping over an array you need to explicitly return a value for each array index iterated over, otherwise you implicitly return undefined.

    editTodo: (state, action) => {
      return state.map((todo) => {
        if (todo.id === action.payload.id) {
          return { ...todo, item: action.payload.item };
        }
        return todo;
      });
    },
    

    or

    editTodo: (state, action) => {
      return state.map((todo) => todo.id === action.payload.id
        ? { ...todo, item: action.payload.item }
        : todo;
      });
    },
    

    Or alternatively you could use a mutable update and let Immer.js (used under the hood) do the shallow copying for you. Search for the todo array item and if it exists mutate it directly. Note here that you do mutate the state and do not return anything.

    editTodo: (state, action) => {
      const { payload } = action;
      const todo = state.find(todo => todo.id === payload.id);
    
      if (todo) {
        todo.item = payload.item;
      }
    },