3

I'm trying to remove an item from an error but it's not working like expected.

Im using state:

  const [actions, setActions] = useState([
    {
      action: "",
      key: ""
    }
  ]);

I have a button to add actions:

    <IconButton
      icon="add"
      bgColor="white"
      iconColor="darkGray"
      onClick={() =>
        setActions([
          ...actions,
          {
            action: "",
            key: ""
          }
        ])
      }
    />

Each row has a delete and I'm trying to use the row index to delete the item in the actions array:

      <IconButton
        disabled={actions.length === 1}
        icon="dash"
        iconColor="red"
        onClick={() => {
          console.log(index);
          setActions(actions => {
            return [...actions.splice(index, 1)];
          });
        }}
      />

https://codesandbox.io/s/actions-selector-n9xb4

Batman
  • 5,563
  • 18
  • 79
  • 155
  • 1
    [`splice`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice) mutates the original array and returns the **deleted** items. I don't think you have the right array method... – Brian Thompson Mar 19 '20 at 20:29
  • 1
    Does this answer your question? [Is this the correct way to delete an item using redux?](https://stackoverflow.com/questions/34582678/is-this-the-correct-way-to-delete-an-item-using-redux) – XCS Mar 19 '20 at 20:31

2 Answers2

11

Try using filter. It does not modify the existing array and can be used like this:

setActions(prevActions => (
  // Filter out the item with the matching index
  prevActions.filter((value, i) => i !== index)
));
Brian Thompson
  • 13,263
  • 4
  • 23
  • 43
  • Isn't filter slower than slice? As it has to go through all elements and make a comparison – XCS Mar 19 '20 at 20:33
  • Yeah, sorry, changed my comment. – XCS Mar 19 '20 at 20:33
  • 1
    `!==` would be better. Should encourage best practices for OP, right? ;) – Andrew Mar 19 '20 at 20:35
  • 1
    I think you are right about performance, but I don't think `slice` returns what they want. It returns the "extracted" items. – Brian Thompson Mar 19 '20 at 20:35
  • 1
    @XCS You would have to slice in both directions. It would be [...actions.slice(0, i), ...actions.slice(i + 1)]. It would be up to the compiler as to which is faster. After a certain point, you're splitting hairs. Just write what's clearer – Andrew Mar 19 '20 at 20:37
  • I would have to agree with @Andrew, I'm always in favor of readability over small optimizations. – Brian Thompson Mar 19 '20 at 20:37
  • But with filter you create a new anonymous function each time. – XCS Mar 19 '20 at 20:40
  • 1
    If you really want to go to that level of micro-optimization you can easily declare it in a function `filterActions` and provide it to filter. But I really don't think the performance gain justifies the loss in clarity of the code. – Brian Thompson Mar 19 '20 at 20:44
  • Yeah, that's true. You still have to make sure you don't pass a new callback to your component each time, as it will break the memoization/pure components. – XCS Mar 19 '20 at 20:44
  • This isn't work. Try it in the codepen: https://codesandbox.io/s/actions-selector-n9xb4. I used letters in the key input to track what was deleted. – Batman Mar 19 '20 at 21:13
  • That is unrelated to the problem in this question. You're using array indexes as the `key`'s for mapped elements. This is [advised against by react](https://reactjs.org/docs/lists-and-keys.html#keys). The array changes, which means the indexes shift. Then react thinks the wrong element has changed. – Brian Thompson Mar 19 '20 at 21:18
  • Well if I'm adding empty elements dynamically there's nothing I can use as an ID – Batman Mar 19 '20 at 21:22
  • Ok, but that is a different problem. It has nothing to do with the current question or the current answers. This solution works. It doesn't produce the desired output for you because of something unrelated. – Brian Thompson Mar 19 '20 at 21:24
  • If you manually add 3 objects to your array with different values, console log it every render, and remove one with my provided function, you will see that the correct object gets deleted in the console log. This answer is correct. – Brian Thompson Mar 19 '20 at 21:28
  • The answer is correct, but it's usually nice to point potential issues with his current implementation. Also, the question is a duplicate. And a benchmark, just for reference: https://jsperf.com/filter-vs-slice-2 – XCS Mar 19 '20 at 21:38
  • I never disagreed that slice is faster. I'm just saying it's clearer code. And I'd also say it's easier to adapt if they discover, like in this case, that they need to use something other than an index to uniquely identify an entry. – Brian Thompson Mar 19 '20 at 22:30
  • And the only reason I didn't point out the key thing before was because I never noticed it. It wasn't included in the question, just the codepen. So I never saw it until I had to prove my answer worked. – Brian Thompson Mar 19 '20 at 22:31
1
  <IconButton
    disabled={actions.length === 1}
    icon="dash"
    iconColor="red"
    onClick={() => {
      setActions(actions.filter((item, i) => i !== index));
    }}
  />

I tested it in your Codesandbox and it worked

Luis Montoya
  • 3,117
  • 1
  • 17
  • 26
  • It doesn't track what gets deleted correctly. So I put a,b,c in the key input, I deleted b and c was removed. – Batman Mar 19 '20 at 21:15
  • it does track, check: https://codesandbox.io/s/actions-selector-wgxdp, probably you just need to connect the ActionDropdown with the actions state and you should be fine. – Luis Montoya Mar 19 '20 at 22:11