4
votes

I'm developing a simple 'to do list' react app (new to React.js). I have adding items to a list working but deleting items raises a question. Within my parent react component, i have the following code:

import ToDoEntries from './to_do_entries.jsx';

class ToDoList extends React.Component {
  constructor(props) {
    super(props);
    this.state = { list: [] }
    this.add = this.addItem.bind(this);
    this.removeItem = this.removeItem.bind(this);
  }

  addItem(e) { //removed to avoid tl:dr }

  render() {
    return(
      <form onSubmit={this.add}>
        <input placeholder='Enter item' type='text' ref={(el) => {this._input = el;} }/>
        <button>Add</button>
      </form>

      <ToDoEntries entries={this.state.list}
        removeCallback={this.removeItem}
      />
    );
  }

}

My to_do_entries.jsx component:

class ToDoEntries extends React.Component {
  constructor(props) {
    super(props);
  }

  renderItems() {
    const { entries, removeCallback } = this.props;

    function createTasks(item) {
      return <li key={item.key}>{item.text}</li>
    }

    var listItems = entries.map(function(item) {
      return(<li onClick={removeCallback} key={item.key}>{item.text}</li>)
    })

    return listItems;
  }

  render() {
    var todoEntries = this.renderItems();

    return(
      <ul>
        {todoEntries}
      </ul>
    );
  }
}

export default ToDoEntries;

Running this code bring:

Warning: setState(…): Cannot update during an existing state transition

Question:

why does to_do_entries.jsx's render immediately execute the callback when an item gets added i.e:

var listItems = entries.map(function(item) {
  return(<li onClick={removeCallback(id)} key={item.key}>{item.text}</li>)
})

However, adding .bind(null, id) to removeCallback ie. <li onClick={removeCallback.bind(null, id)} /> does not?

3
So if I understand correctly you are asking why onClick={removeCallback(id)} is firing/calling removeCallback immediately and why onClick={removeCallback.bind(null, id)} does not fire/call removeCallback immediately?John
Exactly yes. Javascript's this context and function callbacks i.e functionName() vs function has been mind-boggling me for some time now. @Johnsoups

3 Answers

4
votes

Problem is in this part:

onClick={removeCallback(id)}

We need to pass a function to onClick, not the value. When we use () with functionName, that means you are calling that method and assigning the result of that to onClick, that will create a infinite loop if you do setState in removeCallback, because of this cycle:

render ->  removeCallback()  ->  setState ->
  ^                                         |
  |                                         |
  |                                         |
   -----------------------------------------

That's why you are getting the error.

Check the snippet for difference between abc and abc():

function abc(){
   return 'Hello';
}

console.log('without () = ', abc);     //will return the function
 
console.log('with () = ', abc());      //will return the function result (value)

Why it is working with onClick={removeCallback.bind(null, id)}?

Because bind will create a new function, and assign that function to click event, here removeCallback will get called when you click on any item not automatically.

As per MDN Doc:

The bind() function creates a new bound function (BF). A BF is an exotic function object (a term from ECMAScript 2015) that wraps the original function object. Calling a BF generally results in the execution of its wrapped function.

Check the React DOC: Handling events in JSX.

Check this answer for more details about bind: Use of the JavaScript 'bind' method

2
votes

I would advise against that, and use a similar approach to the example I have written for you. Render a list of todo's that is bind-ed to state and then pass in the relevant information back up to your parent component to remove the item. In this case, I use the index of the todo to splice the array so that it removes it.

Your current onClick is invoked immediately when each todo <li> is rendered because it's simply a function call which is causing the problem. .bind solves this problem because it will create a new function when you click on the element which is why the function doesn't invoke immediately.

However, this is generally considered bad practice because every time the component it'll create this function again and again and again. Multiple this by the amount of todo's on the screen and you'll be losing performance. It's a small issue, but my example shows how to solve this problem. https://codepen.io/w7sang/pen/VWNLJp

// App
class App extends React.Component{
  constructor(props) {
    super(props);
    this.state = { list: [] }
    this.add = this.addItem.bind(this);
    this.removeItem = this.removeItem.bind(this);
  }
  addItem(e) { 
    e.preventDefault();
    this.setState({
      list: [ 
        ...this.state.list, 
        {
          key: Math.random(1,10000),
          text: this._input.value
        }
      ]
    })
  }
  removeItem(payload){
    this.setState({
      list: [ 
        ...this.state.list.slice(0, payload.index),
        ...this.state.list.slice(payload.index + 1)
      ]
    })
  }
  render() {
    return(
      <div>
        <form onSubmit={this.add}>
          <input placeholder='Enter item' type='text' ref={(el) => {this._input = el;} }/>
          <button>Add</button>
        </form>
        <ToDoEntries entries={this.state.list} removeItem={this.removeItem} />
      </div>
    );
  }
}

// TodoEntries [STATELESS]
const ToDoEntries = ({ entries, removeItem } ) => {
  return(
    <ul>
      { entries.map((item, index) => {
        return(<Todo key={item.key} index={index} item={item} removeItem={removeItem} />)
      }) }
    </ul>
  );
}

// Todo
class Todo extends React.Component {
  constructor(props){
    super(props);
    this.state = {};
    this.remove = this.remove.bind(this);
  }
  remove() {
    const { index, removeItem } = this.props;
    removeItem({
      index
    });
  }
  render() {
    return <li onClick={this.remove}>{this.props.item.text}</li>
  }
}

ReactDOM.render(<App />,document.getElementById('app'));
<div id="app"></div>
2
votes

why does to_do_entries.jsx's render immediately execute the callback?

Well, when your mapping over the list of todo's each <li/> is invoking the removeCallback function instead of assigning it to the onClick.

So current code

<li onClick={removeCallback(id)} </li>

is equivalent to:

var result = removeCallback(id);
<li onClick={result} </li>

You have correctly pointed out that using bind will work. This is due to the behavior which makes it so useful in these situations.

See the mdn docs for more info, but I'll quote the important part here:

bind ... creates and returns a new function that, when called...

In your case, when using bind, and giving that to the onClick you are creating a new function that will be called when the click event is actually fired, not when the element is being rendered.

Another way of looking at removeCallback.bind(null, id) is its like this:

var newFunc = () => {
  return removeCallback(id);
}
<li onClick={newFunc} </li>