pom421
pom421

Reputation: 1964

setState is not updating state properly

Sorry, I really miss something with the transmission of state within props of sub components in React.

I have implemented a version of a todo list with 3 components.

There is a Form component and a ListTodo component. The state is stored only in the App component.

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

class App extends Component {

    constructor(props) {
        super(props);
        this.state = {
            tasks: ["un truc", "autre truc"]
        };
        this.addTask = this.addTask.bind(this);
    }

    addTask(task) {
        this.setState({
            tasks: this.state.tasks.push(task)
        })
    }

    render() {
        return (
            <div className="App">
                <Form onTaskAdded={ this.addTask }></Form>
                <ListTodo tasks={ this.state.tasks }></ListTodo>
            </div>
        );
    }
}

class Form extends Component {

    constructor(props) {
        super(props);
        this.state = {
            task: ""
        }

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

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

    addTask(event) {
        this.props.onTaskAdded(this.state.task);
        event.preventDefault();
    }

    render() {
        return (
            <form onSubmit={ this.addTask }>
                <input placeholder="À faire" onChange={ this.handleChange }></input>
                <input type="submit"></input>
            </form>
        )
    }
}

class ListTodo extends Component {

    render() {
        const tasks = this.props.tasks.map((t, i) => (
                <li key={i}>{t}</li>
            ))

        return (
            <ul>{tasks}</ul>
        )
    }
}

export default App;

The display is good at start so the ListTodo achieves to see the prop tasks. But after a form submission, I get an error on ListTodo.render :

TypeError: this.props.tasks.map is not a function

When I console.log the this.props.tasks, I don't get my array but the length of the array.

Do you know why?

Edit : Thanks for answers guys, you're right. I missed the behavior of Array.push.

But React seems still odd. If I let the mistaken code this.setState({ tasks: this.state.tasks.push(task) })

then a console.log(JSON.stringify(this.state)) displays :

{"tasks":["un truc","autre truc","aze"]}.

Very disturbing to not be able to trust a console.log...

Upvotes: 3

Views: 1658

Answers (4)

Joshua
Joshua

Reputation: 866

The problem is from Array.push return the number of elements in the array and not the updated array

addTask(task) {
  this.setState({
    tasks: this.state.tasks.push(task)
  })
}

To fix this you can push to state.tasks then setState with it later on:

addTask(task) {
  this.state.tasks.push(task);
  this.setState({
    tasks: this.state.tasks
  })
}

This way you set state.task to the updated array.

Upvotes: 0

Mayank Shukla
Mayank Shukla

Reputation: 104519

As per MDN DOC:

The push() method adds one or more elements to the end of an array and returns the new length of the array.

Array.push never returns the result array, it returns the number, so after adding the first task, this.state.tasks becomes a number, and it is throwing the error when you trying to run map on number.

You did the mistake here:

addTask(task) {
    this.setState({
        tasks: this.state.tasks.push(task)
    })
}

Write it like this:

addTask(task) {
    this.setState( prevState => ({
        tasks: [...prevState.tasks, task]
    }))
}

Another import thing here is, the new state will be depend on the previous state value, so instead of using this.state inside setState, use updater function.

Explanation about Edit part:

Two important things are happening there:

1- setState is async so just after setState we can expect the updated state value.

Check this for more details about async behaviour of setState.

2- Array.push always mutate the original array by pushing the item into that, so you are directly mutating the state value by this.state.tasks.push().


Check the DOC for more details about setState.

Check the MDN Doc for spread operator (...).

Check this snippet:

let a = [1,2,3,4];

let b = a.push(5);  //it will be a number

console.log('a = ', a);

console.log('b = ', b);

Upvotes: 7

Miguel P&#233;res
Miguel P&#233;res

Reputation: 640

As the other answers stated, the Array push method does not return the array. Just to complement the answers above, if you are using ES6, a nice and elegant way of doing this is using the spread operator (you can read more about it here)

this.setState({
    tasks: [...this.state.tasks, task]
})

It is essentially the same as using the concat method, but I think this has a nicer readability.

Upvotes: 1

Pier Paolo Ramon
Pier Paolo Ramon

Reputation: 2900

The problem is in how you add a task in the App’s state.

addTask(task) {
    this.setState({
        tasks: this.state.tasks.push(task)
    })
}

See, Array.prototype.push returns the length of the array after adding an element. What you really want is probably Array.prototype.concat.

addTask(task) {
    this.setState({
        tasks: this.state.tasks.concat([ task ])
    })
}

Also, thanks to @t-j-crowder pointers and as also reported by @mayank-shukla, you should use a different approach to mutate your state:

addTask(task) {
    this.setState(function (state) {
        return {
            tasks: state.tasks.concat([ task ])
        }
    });
}

Or using ES2015 Arrows, Object destructuring and Array spread:

addTask(task) {
    this.setState(({ tasks }) => ({
        tasks: [ ...tasks, task ]
    }));
}

Since Component.prototype.setState can be asynchronous, passing a function to it will guarantee the new state values depend on the right, current previous values.

This means that if two or more setState calls happen one after another you are this way sure that the result of the first one will be kept by applying the second.

Upvotes: 2

Related Questions