javascriptreactjsreact-hooks

Push method replaces the last value of the array


I'm trying to make a simple Todo list app. I'm trying to use the push method to add to the array in JavaScript. I have a button that adds a todo and an input to enter the todo. Then an if/else statement determines whether or not the value of the todo input is null or an empty string. If the todo input is not null or an empty string then the following code is executed:

      todos.push(uploadTodo)
      setTodo(todos)
      console.log(todos)

This is where my problem is. todos.push(uploadTodo) will only add one new item to my array. After that it replaces the last element in the array.

Here's my code:

import { useState } from 'react';
import Todo from './Todo'

function App() {
  let todos = ["Test", "testing"]
  const [todo, setTodo] = useState(todos)
  const [value, setValue] = useState(null)
 
  const getInput = (event) => {
    setValue(event.target.value)
  } 

  const pushTodo = (uploadTodo) => {
    if (value === null || value === "") {
      alert("Can't add an empty todo.")
    }
    else {
      todos.push(uploadTodo)
      setTodo(todos)
      console.log(todos)
    }
  }

  return (
    <>
      <h1>Todo List!</h1>
      <Todo></Todo>
      <button onClick={() => pushTodo(value)}>Add a todo</button><button>Clear todos</button><br></br>
      <input placeholder="Add a todo" onChange={getInput}></input>
    </>
  );
}

export default App;

I'm using React 18.2.0.


Solution

  • Every time your component re-renders, your local todos variable gets reset to its original value by this line:

    let todos = ["Test", "testing"]
    

    You then push the new value:

    todos.push(uploadTodo)
    

    Now todos is ["Test", "Testing", uploadTodo] and you update state:

    setTodo(todos)
    

    This triggers a re-render, and todos gets reinitialized:

    let todos = ["Test", "testing"]
    

    So each time you push a value, you're pushing into the original list again.

    The easiest solution is to move the todos value out of the component so it doesn't get reinitialized on every render:

    const todos = ["Test", "testing"]
    
    function App() {
      const [todo, setTodo] = useState(todos)
      const [value, setValue] = useState(null)
      // ...
    

    But I would also strongly recommend you update state with a new array instead of continually pushing into the original todos one.

    Calling a state update and passing in the same array that's already in state--even if the items in the array have changed--won't trigger a re-render, because React sees that the new array is the same array it already has, and this can (obviously) cause the app to behave in strange ways.

    Instead of todos.push(...) I'd recommend you create a new array each time by spreading the existing state and appending the new value:

    setTodo([...todo, uploadTodo])
    
    function App() {
      const [todo, setTodo] = useState(["Test", "testing"])
      const [value, setValue] = useState(null)
     
      const getInput = (event) => {
        setValue(event.target.value)
      } 
    
      const pushTodo = (uploadTodo) => {
        if (value === null || value === "") {
          alert("Can't add an empty todo.")
        }
        else {
          // create a new array with the exiting state plus the new value
          setTodo([...todo, uploadTodo])
        }
      }
    
      return (
        <>
          <h1>Todo List!</h1>
          <Todo></Todo>
          <button onClick={() => pushTodo(value)}>Add a todo</button><button>Clear todos</button><br></br>
          <input placeholder="Add a todo" onChange={getInput}></input>
        </>
      );
    }