2

I'm new to React and I'm following an online tutorial. The instructor presented the code below as part of a Todo App. It is the logic for adding a new todo item (object) to the state.

var TodoApp = React.createClass({
  getInitialState: function() {
    return {
      todos: {}
    };
  },
  addTodo: function(todo) { 
    var timestamp = (new Date()).getTime();
    this.state.todos[`todo-${timestamp}`] = todo;
    this.setState({
      todos: this.state.todos
    });
  }
});

1. In this case, is it a good practice to assign the todo object to the state before calling this.setState()? (This SO question gives some related information.)
2. Wouldn't it be better to spread the todos as below?
3. Alternatively, is there a better way to update state in this setup?

var TodoApp = React.createClass({
  getInitialState: function() {
    return {
      todos: {}
    };
  },
  addTodo: function(todo) { 
    var timestamp = (new Date()).getTime();
    this.setState({
      todos: {...this.state.todos, [`todo-${timestamp}`]: todo}
    });
  }
});
Community
  • 1
  • 1
Piotr Berebecki
  • 7,428
  • 4
  • 33
  • 42
  • 1
    in first example you have mutated the state. You should not mutate the state, because of bugs and unpredictable beheviour. Second example looks good – Kokovin Vladislav Sep 10 '16 at 07:56

1 Answers1

1
  1. No, see example below & documentation.

    this.state.todos = ...
     // Someone modifies todo's state at this point
     this.setState({
       todos: this.state.todos // this would not do what you expected anymore
     });
    

NEVER mutate this.state directly, as calling setState() afterwards may replace the mutation you made

React's documentation

  1. Absolutely, that's the React way. This is a perfect solution.
  2. Spreading is good enough.
Lyubomir
  • 19,615
  • 6
  • 55
  • 69