How to Do It Without Creating a Stateless Component

I need to know how to update the state list using an inline function, and not a stateless component. I’m almost there and Id like someone to show me that’s possible.

It’s the Shopping List code - there is a video in FFC Yt channel about it (link:React Basics - Beau teaches JavaScript - YouTube).

So, here’s my code so far. Below render, I need only to update listItems so that the item clicked on will be deleted from the list. The author used a stateless functional component to do it. Id like to do it inline.


class ShoppingList extends React.Component{
  constructor(){
    super();
    this.state = {list:["potatoes", "eggs", "curly braces", "onions"]};
  }

addItem(){
  var item = document.getElementById("listItem").value;
  document.getElementById("listItem").value = "";
  var newList = this.state.list.slice();
  newList.push(item);
  this.setState({list:newList});
}
onCLicky(index){
  var newList = this.state.list.slice();
  newList.splice(index,1);
  this.setState({list:newList});
}
  
  render(){
    var listItems = [];
    this.state.list.forEach((item,i)=>{listItems.push(()=>{<li onClick={this.onClicky(i)}>{item}</li>})})
    return (
      <div>
      <h1>Cheap Shopping List for {this.props.user}</h1>
        <input type="text" id="listItem" placeholder="Shot Here"/>
        <button type="button" onClick={()=> this.addItem()}>1 More</button>
      <ul>
        {listItems}
      </ul>
      </div>
    );
  }
}

ReactDOM.render(
 <ShoppingList user="Caio Sa"/>,
document.getElementById('container')
);

And here’s his solution:

function ListItem(props) {
  return (
    <li onClick={props.onClick}>
      {props.item}
    </li>
  );
}

class ShoppingList extends React.Component {
  constructor() {
    super();
    this.state = {
      list: ['unicycle', 'juggling clubs', 'stilts']
    };
  }
  
  addItem() {
    var item = document.getElementById("listItem").value;
    document.getElementById("listItem").value = "";
    var newList = this.state.list.slice();
    newList.push(item);
    this.setState({list: newList});
  }  
  
  onClick(index) {
    var newList = this.state.list.slice();
    newList.splice(index, 1);
    this.setState({list: newList});
  }
  
  render() {
    var listItems = [];
    this.state.list.forEach((item, i) => {
      listItems.push(<ListItem item={item} onClick={() => this.onClick(i)} />)
    });
  
    return (
      <div className="shopping-list">
        <h1>Shopping List for {this.props.name}</h1>
        <input type="text" id="listItem" placeholder="Add item"/>
        <button type="button" onClick={() => this.addItem()}>Add</button>
        <ul>
          {listItems}
        </ul>
      </div>
    );
  }
}

ReactDOM.render(
  <ShoppingList name="Beau" />,
  document.getElementById('container')
);

Thanks a lot! You all are amazin’!

OK, there are some issues here.

    var listItems = [];
    this.state.list.forEach((item,i)=>{listItems.push(()=>{<li onClick={this.onClicky(i)}>{item}</li>})})

Why are we pushing on a function that returns a component - it should just be the component:

listItems.push(<li onClick={this.onClicky(i)}>{item}</li>)

This will give our render an array or components. It doesn’t know how to handle an array of functions that return components.

The next issue for me is this:

    var listItems = [];
    this.state.list.forEach((item,i)=>{listItems.push(()=>{<li onClick={this.onClicky(i)}>{item}</li>})})

First of all, why use forEach here? Using map would make a lot more sense since you are creating a new array from an old array. So, this should be:

  const listItems = this.state.list.map((item, i) => <li onClick={this.onClicky(i)}>{item}</li>)

That makes more semantic sense. Use the most specific prototype method you can.

The next issue is the error message:

iframeConsoleRunner-d8236034cc3508e70b0763f2575a8bb5850f9aea541206ce56704c013047d712.js:1 TypeError: this.onClicky is not a function
// …

So, that isn’t very clear since we know that this.Clicky is in fact a function. The issue is that the return value is not a function. You are passing it in as onClick={this.onClicky(i)}. So you are invoking that method each time so you are actually passing in the return value of that method, which returns nothing, so you are passing in undefined. This is a common problem in JS in general and comes up a lot in React. There are a couple of ways to deal with it, but the easiest is to just wrap it in an anonymous function:

<li onClick={() =>  this.onClicky(i)}>

Now it’s passing an uninvoked anonymous function that when called will invoke our onClicky method.

It should be pointed out that if you didn’t need to pass a parameter, this wouldn’t have been an issue, e.g., <li onClick={ this.onClicky }> - that is perfectly legit since we are passing in the uninvoked function, the function reference. I’m not saying that there is anything wrong with passing in a parameter, just trying to make a distinction.

With that, things start to work.

My last question would be “why?” This is the opposite direction you should be moving. The point should be to break things into manageable components, not to combine them into one mega-component. You are defeating one of the best features of React.

1 Like

In your code:

The onClick attribute takes a function that it can run when the element is clicked. You aren’t passing it a function, you’re calling your function and then passing the output (which is undefined since the function onClicky() has no return statement in its declaration).

Awesome insights! Thanks for being so helpful!
The code now is:

   this.state.list.map((item,i)=>{listItems.push(<li onClick={()=>this.onClicky(i)}>{item}</li>)})

But when I click on the list, the list items dont disappear. And CodePen doesnt identify any error. I cant identify whats the issue then.

And about the why Id like to it: I think it was too much to create another component if that could be fit inline with clarity.

Sure, there are situations where this makes sense, but that can also be dangerous logic. You don’t loose anything by putting it in a different component/file - it’s just an extra file. But when it gets compiled it (I assume) becomes the same thing. Learn and use the strength of React components.

Awesome! Thanks for the clarification.

So, how would my clicky function look like with a return?

onCLicky(index){
  var newList = this.state.list.slice();
  newList.splice(index,1);
  return (this.setState({list:newList});); 
}

or

  onCLicky(index){
  var newList = this.state.list.slice();
  newList.splice(index,1);
  return {this.setState({list:newList});}; 
}

I got a bit confused how to define the return in a JSX function.

No, the point is to use map instead of and not to use push. You should read up on the map method - it comes in handy in React a lot. The point is that it returns the new array that you want. Please take a look at the code that I suggested and understand how it is different.

The other problem is here:

onCLicky(index) {

That should be a lowercase “l”, not “L”.

He wasn’t saying that the method needs to return something, just pointing out that it doesn’t return anything and that was what you were passing to the onClick attribute. Some methods do return something, but there is no need for this one to return anything.

Sorry if that was unclear – you shouldn’t be returning anything from the function. The only problem was that instead of passing in a function to onClick, you were calling the function and then passing in the (undefined) result. The easy way to pass a function that needs a parameter is to wrap it in an anonymous function expression:

<li onClick={() => onClicky(i)} />

instead of

<li onClick={onClicky(i)} />

See the difference?

Also, regarding this:

Breaking down your app into more components isn’t just adding more files – it actually improves efficiency in many cases! When you call the render() method of a component that uses other sub-components, it doesn’t immediately render all of the sub-components. Instead, it just remembers what props you gave it and calls it a day. That way, if the outer component gets new props or state and is re-rendered, but still gives the same props to the sub-component, React can skip calling render() for the sub-component, saving time.

If instead you don’t use sub-components, but just spell everything out in the outer component’s render() function, React has no choice but to re-render everything whenever anything small changes.

Breaking down components into sub-components is a strongly recommended design pattern in React.

1 Like

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.