Am I updating the React state correctly?

Question

In my React state, I have the state:

this.state = {
  users: [{
    id: 1,
    name: "john",
    age: 27
  }, {
    id: 2,
    name: "ed",
    age: 18
  }, {
    id: 3,
    name: "mel",
    age: 20
  }]
}

I am rendering the name correctly. When you click on the name, it should remove the name, which will need an onClick that takes in a function and that returns a removeUser function.

It is my understanding that you do not want to mutate the state in React, but return a new state. So, in my removeUser function, I did:

removeUser(index) {
  // Making a new copy of the array
  const users = [...this.state.people].splice(index, 1);
  this.setState({ users });
}

I bind my method with this.removeUser = this.removeUser.bind(this).

When I tested out my code, I am removing the users as expected. However, when I run my code against a test that my friend wrote, I got a failed test that said: Expected 1 to be 0

That message tells me that I must be mutating the state somehow, but I am not sure how. Am I returning a new array and updating the state correctly? Can someone explain to me how I should update my state correctly in this case?

Here is the full code:

class Group extends Component {
  constructor(props) {
    super(props);
    this.state = {
      users: [
        {id: 1, name: "john", age: 27}, 
        {id: 2, name: "ed", age: 18}, 
        {id: 3, name: "mel", age: 20}
      ]
    }

    this.removeUser = this.removeUser.bind(this);
  }

  removeUser(index) {
    const users = [...this.state.users].splice(index, 1);
    this.setState({ users });
  }

  render() {
    const list = this.state.users.map((user, i) => {
      return (
        <div onClick={() => this.removeUser(i)} key={i}>{user.name}</div>
      );
    });

    return (
      <div>
        {list}
      </div>
    );
  }
}

Answer 1

const users = [...this.state.users].splice(index, 1)

looks like it should be removing an item from a collection, which I suppose it technically is. The problem is that users doesn't contain the list of users you want to keep.

Instead, splice has modified your new array in-place and then returned the removed items:

splice docs

Return value

An array containing the deleted elements. If only one element is removed, an array of one element is returned. If no elements are removed, an empty array is returned.

Instead, if you'd like to use splice, create a new array:

const users = [...this.state.users]

and then splice the new array:

users.splice(index, 1)

---

You'll run into a similar issue if you need to sort an array.

The issue of modifying data in-place is generally frowned upon for react, in favor of immutable references. This is because React uses a lot of direct comparison of objects as a heuristic approach to speed things up. If your function modifies an existing object instance, React will assume the objects haven't changed.

The act of copying to a new object and then operating on the data comes with some tradeoffs. For removing a single item, it's negligible. For removing many items, you may be better served by an alternative method, such as multiple slice calls:

const users = [
  ...this.state.users.slice(0, index)
  ...this.state.users.slice(index + 1)
]

However this too is quite verbose.

Another approach is to use an immutable variant of splice:

// this quick example doesn't handle negative start indices
const splice = (start, deleteCount, ...items) => arr => {
  const output = []
  let i
  for (i = 0; i < start && i < arr.length; i++) {
    output.push(arr[i])
  }
  output.push(...items)
  for (i += deleteCount; i < arr.length; i++) {
    output.push(arr[i])
  }
  return output
}

const users = splice(index, 1)(this.state.users)

Answer 2

you could also change your onClick and pass it the user.id instead of the index. that would allow you to filter the results instead of needing to splice the array.

onClick={() => this.removeUser(user.id)}

removeUser(id) {
  const users = this.state.users.filter((user) => user.id !== id);
  this.setState({ users });
}

Answer 3

You should use slice() instead of splice() because splice() mutates the original array, but slice() returns a new array. Slice() is a pure function. Pure is better!!

removeUser(index) {
    const users = [
    ...this.state.users.slice(0, index),
    ...this.state.users.slice(index+1)
    ];
    this.setState({ users });
  }

Here is JS Fiddle