How to splice an array within the state of a React Component

Question

I am making a timesheet app. The index.js renders the table. The rows of the table are read from the children array. The children array reads the state to keep itself updated. AddRow() is working fine. DeleteRow() has some problem. I intend to fetch the parentNode id from the event, lookup the state's array, splice it and let children update itself - which in turn updates the render. So when a row is deleted, it is spliced from the state's array, and that in turn is updated in the children array, and that in turn is rendered. DeleteRow runs everything except the array.splice or indexOf part. I even tried a for loop, but it doesn't work. Is my approach correct?

index.js    
import React from "react";
import ReactDOM from "react-dom";
import HeaderAndFirstTableRow from "./headerandfirstrow";
import TableRow from "./tablerow";
import "./styles.css";

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = { noOfRows: [] };
    this.addRow = this.addRow.bind(this);
    this.deleteRow = this.deleteRow.bind(this);
    this.find = this.find.bind(this);
  }
  addRow() {
    let arr = this.state.noOfRows;
    let arrLength = arr.length;
    arr.push(arrLength + 1);
    this.setState({
      noOfRows: arr
    });
  }
  find(arr, id) {
    for (let i = 0; i < arr.length; i++) {
      if (arr[i] === id) {
        return i;
      }
    }
  }
  deleteRow(event) {
    let arr = this.state.noOfRows;
    let id = event.target.parentNode.id;
    let ix = arr.findIndex(id);
    arr.splice(ix, 1);
    this.setState({ noOfRows: arr });
  }
  render() {
    let children = [];
    for (let i = 0; i < this.state.noOfRows.length; i++) {
      children.push(
        <TableRow id={this.state.noOfRows.length} delete={this.deleteRow} />
      );
    }
    return (
      <HeaderAndFirstTableRow click={this.addRow}>
        {children}
      </HeaderAndFirstTableRow>
    );
  }
}
const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

headerandfirstrow.js

import React from "react";

import "./styles.css";

export default function HeaderAndFirstTableRow(props) {
  function handler() {
    props.click();
  }
  return (
    <table>
      <tr>
        <th>Feature</th>
        <th>Phase</th>
        <th>Comments</th>
        <th>Monday</th>
        <th>Tuesday</th>
        <th>Wednesday</th>
        <th>Thursday</th>
        <th>Friday</th>
        <th>Saturday</th>
        <th>Sunday</th>
        <th onClick={handler}>+</th>
      </tr>

      {props.children}
    </table>
  );
}

tablerow.js
import React from "react";
import "./styles.css";
export default function TableRow(props) {
  function del(e) {
    props.delete(e);
  }
  return (
    <tr id={props.id}>
      <td />
      <td />
      <td />
      <td />
      <td />
      <td />
      <td />
      <td />
      <td />
      <td />
      <td onClick={del}>-</td>
    </tr>
  );
}

shouldn't it be `.indexOf ` instead of `.findIndex` ? – Freddy Jun 01 '19 at 06:38 — Freddy, Jun 01 '19 at 06:38
splice mutates original array, you should not mutate state – Code Maniac Jun 01 '19 at 06:41 — Code Maniac, Jun 01 '19 at 06:41

score 1 · Accepted Answer · answered Jun 01 '19 at 06:43

1

splice mutates original array, you should not mutate state,

You can change to this

deleteRow(event) {
    let arr = this.state.noOfRows;
    let id = event.target.parentNode.id;
    let filteredArray = arr.filter(val=> val !== id)
    this.setState({ noOfRows: filteredArray });
}

answered Jun 01 '19 at 06:43

Code Maniac

37,143
5
39
60

This code is not working for me. Probably you might have the time to jsFiddle it? – Raju Jun 01 '19 at 11:08
@Raju [here](https://react-6hfnww.stackblitz.io), keep in mind in your add row you're adding numbers as `arr.length` so if your remove one row and than add one row you will have duplicated numbers as id, your indexOf is returning `-1` always as you have id as string not as number, so you need to parse it to number – Code Maniac Jun 01 '19 at 11:34
Good stuff. Your observation was spot on and also about the need for parseInt to get the indexOf. Thanks. – Raju Jun 02 '19 at 04:23
Would you suggest a strategy to capture time entered in the elements? – Raju Jun 06 '19 at 09:37
@Raju better write a new question with requirements, it hard to answer a question which divert from it's original state, [read this](https://meta.stackexchange.com/questions/43478/exit-strategies-for-chameleon-questions) – Code Maniac Jun 06 '19 at 16:51

score 0 · Answer 2 · answered Jun 01 '19 at 06:44

0

Use indexOf instead of findIndex. Additionally, always copy objects instead of mutating them directly into the state.

Here is working Fiddle: https://stackblitz.com/edit/react-6hfnww

answered Jun 01 '19 at 06:44

Saurabh Nemade

1,522
2
11
20

score 0 · Answer 3 · answered Jun 01 '19 at 06:44

Splicing this.state.noOfRows array mutates its elements in place.

In deleteRows, this is setting the same array which is now mutated back in the component state.

Instead use Array.prototype.slice to get chunks of the array without the item and compose a new array from that.

  deleteRow(event) {
    let arr = this.state.noOfRows;
    let id = event.target.parentNode.id;
    let ix = arr.findIndex(id);
    const noOfRows = [...arr.slice(0, ix), ...arr.slice(ix+1)] ;
    this.setState({ noOfRows });
  }

How to splice an array within the state of a React Component

3 Answers3