TypeError: unshift is not a function

Question

I have todos array of objects passed as props to a TodoList component. I'm trying to add a new object to the array of objects. I created a method called addTodo which adds the first todo's object & when I tried to add the second todo it fails with the error: TypeError: this.state.todos.unshift is not a function.

Two questions:

• Why it fails on the second try & works on the first try?
• how can I fix it?

This is my code:

import React, { Component } from 'react';
import './TodoListStyles.css'

class TodoList extends Component {
  constructor(props){
    super(props);
    
    this.state = { 
      todos: this.props.todos,
      value: ''
    }

    this.handleChange = this.handleChange.bind(this);
  }

  addTodo(e) {
    e.preventDefault();
    this.setState({
      todos: this.state.todos.unshift({
        id: this.state.todos.length + 1,
        title: this.state.value,
        completed: true
      }),
      value: ''
    });

  }

  handleChange(e) {
    this.setState({ value: e.target.value });
  }

  render() {
    
    return (
      <div>
        <h1>Todos:</h1>
        <form onSubmit={(e) => this.addTodo(e)}>
          <input type='text' value={this.state.value} onChange={this.handleChange} />
        </form>
     
        <ul className='todo-list'>
          { 
            this.props.todos.map(
              todo => 
              <React.Fragment key={todo.id}>
                <li className={`todo-item ${todo.completed ? 'todo-item-completed' : ''}`}>{ todo.title }</li>
                <input type='checkbox' checked={todo.completed} />
              </React.Fragment>
            ) 
          }
        </ul>
      </div>
    )
  }
}

export default TodoList;

EDIT:

I refactored my code based on the explanation answer below to use concat() method instead. I got rid of the error, but it doesn't add the object to the array. This is my new code:

addTodo(e) {
    e.preventDefault();
    
    const newItem = {
      id: this.state.todos.length + 1,
      title: this.state.value,
      completed: true
    };

    this.setState((prevState) => {
      prevState.todos.concat(newItem)
    })
    
    console.log(this.state.todos)
  }```

Answer 1

Array#unshift is an in-place function that mutates the array it's called upon. unshift returns the length of the updated array. The reason it fails after adding an item is because you've replaced what should be an array with a number. Number.map is not a thing so the program crashes.

The golden rule of React is "never mutate state" because it confuses React's diffing algorithm as it attempts to figure out what to rerender, with unpredictable results. Make a copy if you need to use a mutating function like unshift.

Try Array#concat instead of unshift here. concat is non-mutating and returns a copy of the array it's called on, so there will be no problems with React and it's safe to use inline directly in the object you're passing to setState.

A second problem is reading state values inside of the setState call, like this.state.todos.length. Use the callback version of setState that accepts the previous state as a parameter:

this.setState(prevState => ({
  todos: [{
    id: prevState.todos.length + 1,
    title: prevState.value,
    completed: true
  }].concat(prevState.todos)
}));

Spread syntax works in place of concat here. The code below also fixes a bug where this.props.map was used instead of this.state.map and doesn't use an <input> as a child element of a <ul>.

class TodoList extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      todos: this.props.todos,
      value: ""
    };
    this.handleChange = this.handleChange.bind(this);
  }

  addTodo(e) {
    e.preventDefault();
    this.setState(prevState => ({
      todos: [{
        id: prevState.todos.length + 1,
        title: prevState.value,
        completed: true
      }, ...prevState.todos]
    }));
  }

  handleChange(e) {
    this.setState({value: e.target.value});
  }

  render() {    
    return (
      <div>
        <h1>Todos:</h1>
        <form onSubmit={(e) => this.addTodo(e)}>
          <input type='text' 
                 value={this.state.value} 
                 onChange={this.handleChange} />
        </form>
        <ul className='todo-list'>
          { 
            this.state.todos.map(todo =>              
              <React.Fragment key={todo.id}>
                <li className={
                  `todo-item ${todo.completed 
                   ? 'todo-item-completed' 
                   : ''}`
                }>
                  { todo.title }
                  <input
                    readOnly
                    type='checkbox'
                    checked={todo.completed}
                  />
                </li>
              </React.Fragment>
            ) 
          }
        </ul>
      </div>
    )
  }
}

ReactDOM.createRoot(document.querySelector("#app"))
  .render(<TodoList todos={[]} />);

<script crossorigin src="https://unpkg.com/react@18/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@18/umd/react-dom.development.js"></script>
<div id="app"></div>

Using function components and the state hook moves pieces of state into distinct variables, simplifying state management in most use cases.

As a side note, prevState.todos.length + 1 may be a poor way to generate ids if you introduce the option to delete todos. I could add a todo and make the length 1, add a second todo to make the length 2. Then, if I delete the first item with id 1 and create a new item, I have two items with id set to 2. A monotonically increasing number or UUID might be better.