Delete Function in To Do App Returning as an Undefined error in React

Question

So I am working on a todo list app where I can add tasks to the body of the page, and delete tasks that are complete. I'm working on the last part of the app, making the delete buttons functional. But my delete button function, the onRemove function, is returning an error on undefined. I have no idea why it is doing this, as the code looks good to me. Perhaps your eyes can catch what I'm missing? I have two other components for this app, in case you need reference.

My App component is where all of my logic is handled, including the onRemove function:

import React, { Component } from 'react';
import './App.css';
import Navigation from './components/Navigation';
import ListInput from './components/ListInput';
import ListName from './components/ListName';
import Item from './components/Item';
import ItemList from './components/ItemList';

class App extends Component {
  constructor() {
    super();
      this.state = {
        items: [{id: 1, text:"item 1"}, {id:2, text:"item 2"}]
      };    
  }




  addItem = (value) => {
    this.setState(state => {
      const { items } = state;
      const uuidv1 = require('uuid/v1');
      const newId = uuidv1();
      const newItem = {
        text: value, id: newId
      };


    onRemove = (id) => {
        this.setState(state => {
          const { items } = state;
          const filterdItems = 
          items.filter(items => items.id !== id);
          return { items: filterdItems };
        })
      };


      return {
        items: [ items, newItem]
      };
    }
    )
  };



  render() {
    const { items } = this.state;
    return (
      <div className="App">
       <Navigation />
       <ListName />
       <ListInput onSubmit={this.addItem} />
        <Item />
        <ItemList items={ items } 
        onRemove={ this.onRemove } />
      </div>
    );
  }
}

export default App;

This is my Item component

import React from 'react';

const Item = ({ text, onRemove }) =>{
    return (
        <div>
            <button 
            onClick={onRemove}>X

            </button>
            <ul>{text}</ul>
        </div>
    )}
export default Item;

And this is my ItemList component:

import React from 'react';
import Item from './Item';

const ItemList = ({ items, onRemove }) => {
    return (
        <div>
            {items.map(items => <Item key={items.id}
            text={items.text}
            onRemove={() => onRemove(items.id)} />
            )}
        </div>
    )
}

export default ItemList;

Please let me know if you need the code for any other components or if you have any other questions. Thank you in advance for your help!!!

Answer 1

On your App component, your onRemove method should be on the class level, not inside your addItem method. Something like this:

addItem = (value) => {
  this.setState(state => {
    const { items } = state;
    const uuidv1 = require('uuid/v1');
    const newId = uuidv1();
    const newItem = {
      text: value, id: newId
    };

    return {
      items: [ items, newItem]
    };
  })
}

onRemove = (id) => {
  this.setState(state => {
    const { items } = state;
    const filterdItems = items.filter(item => item.id !== id);
    return { items: filterdItems };
  });
}

Alternatively, your onRemove method could be more efficient, you don't need to run through the whole array if you know it is consisted of unique objects, you can stop percorring the array when you find the desired element, and then, remove it. Like so:

onRemove = (id) => {
  const items = [...this.state.items];
  const index = items.findIndex(item => item.id === id);

  items.splice(index, 1);

  this.setState({ items });
}

Answer 2

Your onRemove method is defined in your addItem method. Is this a huge typo? :) Apart from this problem, your code seems working for removal. You can see this in the snippet. Just move your method to the right place. Also, probably your adding method is not handling your state right.

return {
    items: [ items, newItem]
};

If you do this you will get an array including your items array plus the new item. You probably want to do this:

return {
    items: [ ...items, newItem]
};

class App extends React.Component {
  constructor() {
    super();
    this.state = {
      items: [ { id: 1, text: "item 1" }, { id: 2, text: "item 2" } ],
    };
  }

  addItem = ( value ) => {
    this.setState( ( state ) => {
      const { items } = state;
      // const uuidv1 = require( "uuid/v1" );
      // const newId = uuidv1();
      const newItem = {
        text: value,
        // id: newId,
      };

      return {
        items: [ items, newItem ],
      };
    } );
  };

  onRemove = ( id ) => {
    this.setState( ( state ) => {
      const { items } = state;
      const filterdItems = items.filter( items => items.id !== id );
      return { items: filterdItems };
    } );
  };

  render() {
    const { items } = this.state;
    return (
      <div className="App">
        {/* <ListInput onSubmit={this.addItem} /> */}
        <Item />
        <ItemList items={items} onRemove={this.onRemove} />
      </div>
    );
  }
}

const Item = ( { text, onRemove } ) => (
  <div>
    <button onClick={onRemove}>X</button>
    <ul>{text}</ul>
  </div>
);

const ItemList = ( { items, onRemove } ) => (
  <div>
    {items.map( items => (
      <Item
        key={items.id}
        text={items.text}
        onRemove={() => onRemove( items.id )}
      />
    ) )}
  </div>
);

ReactDOM.render( <App />, document.getElementById( "root" ) );

<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="root"></div>

Update after comments

Three dots in ...items is spread syntax. It is a very useful tool spreading arrays or objects. In your app, you want to add a new item to your items array.

[ items, newItem ];

As you can easily see, if you do this you will get something like this:

[ [ {...}, {...} ], {yourNewItem} ]

So, you will end up with an array, including your items array and your new item. So, to fix this you will use spread syntax and spread your array. Sprads mean here it picks the objects in your array, so you will get all the objects without an array.

[ ...items, newItem ];

So, here you are spreading the items, then adding a new one. A new array wraps all of them. This is what your items look like in the first place:

[ {..}, {..}, {yourNewITem} ]

You can see the difference in the snippet below.

const state = {
  items: [{ id: 1, text: "item 1" }, { id: 2, text: "item 2" }]
};

const { items } = state;
const newItem = {
  text: "foo",
  id: 3,
};

const newItems = [ items, newItem ];
const fixedNewItems = [ ...items, newItem ];

console.log( newItems );
console.log( fixedNewItems );