React. Delete specific row from the table

Question

I have a component that is a table. Each row of this table is also component.

    class FormulaBuilder extends Component {
        constructor(props) {
            super(props);
            this.state = {
                rows: [{}]
            }
        }
handleAddRow = () => {
        const item = {};

        this.setState({
            rows: [...this.state.rows, item]
        });
    };

    handleRemoveSpecificRow = (idx) => {
        const rows = [...this.state.rows]
        rows.splice(idx, 1)
        this.setState({ rows })
    }

     render() {
            return (
               {
                   this.state.rows.map((item, idx) => {
                        return (
                           <React.Fragment key={idx}>
                                 <ConcoctionRow
                                  removeSpecificRow={(idx) =>this.handleRemoveSpecificRow(idx)}
                                  id={idx} />
                            </React.Fragment>);
                })
             });
            }
    }

In the child component there is a button. When clicked, the event from the parent component is called:

class ConcoctionRow extends Component {
    constructor(props) {
        super(props);
    }

  handleRemoveSpecificRow = () => {
        this.props.removeSpecificRow(this.props.id);
    }
}

The properties passed the index of the array. But only the last line is always deleted not specific.

Where is my bad? P.S. I am new in JS.

removeSpecificRow is called with the id of the row, however you are assuming it to be index when you call rows.splice in handleRemoveSpecificRow. Are you sure that index and id match exactly? — Pavan Bahuguni, May 24 '19 at 07:05
so in short, your item should have an unique id that doesn't change over time, so you can pass that value to the remove function, something like this `removeSpecificRow={() =>this.handleRemoveSpecificRow(item.uid)}` — Duc Hong, May 24 '19 at 07:24

Chris Ngo · Answer 1 · 2019-05-24T07:41:54.860

A couple of things, you want to avoid using .splice() to update your arrays in components. Often times this actually ends up mutating your original state instead of creating a new one. A direct violation of React concepts.

Likewise lets try some stuff out on the console:

const arr = [1, 2, 3] <-- this is your state
const newArr = arr  <-- you created a reference of your state. This does not actually create a new copy.

Now if you splice the newArr

newArr.splice(0, 1) <-- now newArr = [2, 3]

Well guess what, you also mutated your original state.

arr <-- is now also [2, 3]

A common misconception in JavaScript is that when you create a new variable that equals an existing variable, you expect that it actually creates a new copy.

let cat = {id: 1, name: "bunny"}
let myCat = cat

This is not actually the case, instead of explicitly creating a new copy, your new variable points to the same reference of the original object it is derived from. If I did something like:

myCat.age = 2  <-- Now myCat has a new property of age.
myCat <-- {id: 2, name: "bunny", age: 2}

BUT, because these two variables point to the same reference. You also mutate the original cat object as well

cat <-- {id: 2, name: "bunny", age: 2}

Use array.filter() instead to create a completely new array.

Here's an example with your code as well as a sandbox for reference: https://codesandbox.io/s/heuristic-nobel-6ece5

import React from "react";
import ConcoctionRow from "./ConcoctionRow";

class FormulaBuilder extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      rows: [{}, {}, {}]
    };
  }
  handleAddRow = () => {
    const item = {};

    this.setState({
      rows: [...this.state.rows, item]
    });
  };

  handleRemoveSpecificRow = idx => {
    const { rows } = this.state;

    const updatedRows = rows.filter((row, index) => {
      return index !== idx;
    });

    this.setState({
      rows: updatedRows
    });
  };
  render() {
    return (
      <div>
        {this.state.rows.map((item, idx) => {
          return (
            <React.Fragment key={idx}>
              <ConcoctionRow
                removeSpecificRow={this.handleRemoveSpecificRow}
                id={idx}
              />
            </React.Fragment>
          );
        })}
      </div>
    );
  }
}

export default FormulaBuilder;

Fair point about avoiding `splice`, but in OP it is used fine, since they avoid creating a reference by using spread: `const rows = [...this.state.rows]` creates a new `rows` array that contains values from `state.rows`. — pawel, May 24 '19 at 07:49
@pawel, you know what, you're absolutely right. Running OP's code in my sandbox, I actually don't see the problem he has mentioned. — Chris Ngo, May 24 '19 at 07:58

score 2 · Answer 2 · answered May 24 '19 at 08:43

I show the pattern I would use for this case. I recommend to use id instead of array index for items. filter array function is immutable (it creates a new array, not mutates the previous one), so ok to use in set state. The functional form of setState is also a good stuff.

const Row = ({ onClick, children, id }) => (
  <li>{children} <button onClick={() => onClick(id)}>Delete</button></li>
)

class App extends React.Component {
  state = {
    list: [
      {id: 1, label: 'foo' },
      {id: 2, label: 'bar' }
    ]
  }
  
  handleDelete = id => {
    this.setState(prevState => ({
      list: prevState.list.filter(row => (
        row.id !== id
      ))
    }))
  }
  
  render(){
    const { list } = this.state;
    
    return (
      <ul>
        {list.map(({ id, label }) => (
          <Row id={id} onClick={this.handleDelete}>{label}</Row>
        ))}
      </ul>
    )
  }
}

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

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.6.3/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.6.3/umd/react-dom.production.min.js"></script>
<div id="root"></div>

React. Delete specific row from the table

2 Answers2