1

Simple todo list. I want to add a delete function but getting error:

proxyConsole.js:56 Warning: setState(...): Cannot update during an existing state transition (such as within render or another component's constructor). Render methods should be a pure function of props and state; constructor side-effects are an anti-pattern, but can be moved to componentWillMount.

I might meesed with the binding as I try to get a grasp of it.

class App extends Component { constructor(props) { super(props); this.onDelete = this.onDelete.bind(this); this.state = { todos: ['wash up', 'eat some cheese', 'take a nap'], }; } render() { var todos = this.state.todos; todos = todos.map(function(item, index){ return( <TodoItem item={item} key={index} onDelete={this.onDelete}/> ) }.bind(this)); return ( <div className="App"> <ul> {todos} </ul> </div> ); } onDelete(item){ var updatedTodos = this.state.todos.filter(function(val, index){ return item !== val; }); this.setState({ todos:updatedTodos }); } } class TodoItem extends Component { constructor(props) { super(props); this.handleDelete = this.handleDelete(this); } render(){ return( <li> <div className="todo-item"> <span className="item-name">{this.props.item}</span> <span className="item-delete" onClick={this.handleDelete}> x</span> </div> </li> ); } handleDelete(){ this.props.onDelete(this.props.item); } } 
3
  • 1
    Is there a reason you have the onDelete function outside both classes that your defining? Commented Aug 22, 2017 at 16:13
  • formatting issue. It is within App just after render() Commented Aug 22, 2017 at 16:17
  • 1
    ah you know what it might be is this: this.handleDelete = this.handleDelete(this); Change that to this.handleDelete = this.handleDelete.bind(this); I think its immediately executing that method when the TodoItem class is constructed. Commented Aug 22, 2017 at 16:24

1 Answer 1

2

I think you are invoking handleDelete handler in child component's constructor. It should be :

this.handleDelete = this.handleDelete.bind(this); 

class App extends React.Component { constructor(props) { super(props); this.onDelete = this.onDelete.bind(this); this.state = { todos: ["wash up", "eat some cheese", "take a nap"] }; } render() { var todos = this.state.todos; todos = todos.map( function(item, index) { return <TodoItem item={item} key={index} onDelete={this.onDelete} />; }.bind(this) ); return ( <div className="App"> <ul> {todos} </ul> </div> ); } onDelete(item) { var updatedTodos = this.state.todos.filter(function(val, index) { return item !== val; }); this.setState({ todos: updatedTodos }); } } class TodoItem extends React.Component { constructor(props) { super(props); this.handleDelete = this.handleDelete.bind(this); } render() { return ( <li> <div className="todo-item"> <span className="item-name">{this.props.item}</span> <span className="item-delete" onClick={this.handleDelete}> x</span> </div> </li> ); } handleDelete() { this.props.onDelete(this.props.item); } } ReactDOM.render(<App />, document.getElementById("app"));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react.min.js"></script> <script src="https://cdnjs.cloudflare.com/ajax/libs/react/15.1.0/react-dom.min.js"></script> <div id="app"/>

Sign up to request clarification or add additional context in comments.

2 Comments

This is good. Personally, I think its nicer to create a new function, name it something like "renderToDoList()" and put the logic for mapping the todos in there, instead of doing it inside the render method. Inside of the jsx body, you would call, {this.renderToDoList()}, if you did it that way. Good stuff though, it works.
@DanielZuzevich, Yes, we can do it in that way too. I just wanted to highlight the issue of OP. :-). So i didn't make much changes in the original code.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.