[FIXED] Why isn't the todo being deleted?

Issue

I am working through a tutorial for a course I'm taking. The lab I'm working on walks through creating a to-do app. I'm on step 3, which asks us to create a button that deletes a task. I feel ridiculous, because I know I can figure it out but...well, I haven't yet! I will post the code to see if there are any initial issues, and then update with the methods I've already tried. Any help is greatly appreciated. Thank you!

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

const App = () => {
  const [todos, setTodos] = useState([]);
  const [todo, setTodo] = useState("");

  const handleSubmit = (e) => {
      e.preventDefault();

      const newTodo = {
          id: new Date().getTime(),
          text: todo.trim(),
          completed: false,
      };

      if (newTodo.text.length > 0) {

          setTodos([...todos].concat(newTodo));
          setTodo("");

      } else {

          alert("Enter Valid Task");
          setTodo("");
      }
  }

  const deleteTodo = (id) => {
      let updatedTodos = [...todos].filter((todo) => todo.id !== id);
      setTodos(updatedTodos);
  }

  const button = <button onClick={() => deleteTodo(todo.id)}>Delete</button>

return (
    <div>
        <h1>To-do List</h1>
            <form onSubmit={handleSubmit}>
                <input
                    type="text"
                    onChange={(e) => setTodo(e.target.value)}
                    placeholder="Add a new task"
                    value={todo}
                />
                <button type="submit">Add Todo</button>
            </form>
            {todos.map((todo) => <div>ID: {todo.id} Task: {todo.text} {button}</div>)}
    </div>
    );
};

export default App;

I didn't just copy and paste, so it's possible that I messed something up while typing. I'm expecting the deleteTodo() function to accept a todo.id and filter the list of todos, excluding the one I want to delete. I'm thinking that the issue may be cause by the way I've created the button? Again, I'm not sure why I can't figure it out. TIA.

EDIT: Okay, it works now! Thank you all so much for explaining this. For anyone else that comes across this problem, here's where I mis-stepped:

const button = <button onClick={() => deleteTodo(todo.id)}Delete<button>

@Nicholas Tower's explanation was very clear--creating this outside of .map(...)causes deleteTodo to get the todo state, not the not the todo I want it to delete from the todos array. @Lars Vonk, @0stone0, and @Sudip Shrestha all said this as well. @Sudip Shrestha and @pilchard also helped correct the deleteTodo function. Again, I really appreciate all the help. The code works now. I'll show the updates so people having a similar issue can compare:

import React from "react";
import "./App.css";

const App = () => {
  const [todos, setTodos] = React.useState([]);
  const [todo, setTodo] = React.useState("");

  const handleSubmit = (e) => {
      e.preventDefault();

      const newTodo = {
          id: new Date().getTime(),
          text: todo.trim(),
          completed: false,
      };

      if (newTodo.text.length > 0) {

          setTodos(todos.concat(newTodo));
          setTodo("");

      } else {

        alert("Enter a valid task");
        setTodo("");
      }
  }

  // update the state using setState, rathar than mutating it directly @Sudip Shrestha
  const deleteTodo = id => {
      setTodos(prevState => {
          return prevState.filter(todo => todo.id !== id)
      });
  };

// line 51: button placed inside .map(), as per many suggestions below.
return (
    <>
        <h1>Todo List</h1>
            <form onSubmit={handleSubmit}>
                <input
                    type="text"
                    onChange={(e) => setTodo(e.target.value)}
                    placeholder="Add a new task..."
                    value={todo}
                />
            </form>
            {todos.map((todo) => 
                <div>
                    ID: {todo.id} Task: {todo.text}
                    <button onClick={() => deleteTodo(todo.id)}>Delete</button>
                </div>)}
    </>
    );
};

export default App;

Solution

const button = <button onClick={() => deleteTodo(todo.id)}>Delete</button>

You're creating this button element just once, and the todo variable it refers to is the todo state, which is a string (usually an empty string). Since todo is a string, todo.id is undefined, and deleteTodo can't do anything with that.

You need to create separate buttons for each item, so you should move this code down into your .map:

{todos.map((todo) => (
  <div>
    ID: {todo.id} Task: {todo.text}
    <button onClick={() => deleteTodo(todo.id)}>Delete</button>
  </div>
))}

Now each item has its own button, with its own onClick function. And in those functions, todo is the item of the array.

Answered By - Nicholas Tower
Answer Checked By - - Jay B. (ReactFix Admin)