DOM still shows items which are have already been deleted from state in React using hooks

Question

I am using react-dnd (Drag and Drop) to drop in items into a div which acts as a canvas for holding the items. Since I was having trouble with dnd not updating the reference of the canvas state, I had to use an useRef hook to manually update the same.

const [canvas, setCanvas] = useState([])
const canvasRef = useRef(null);

//Function to update the referrence
const _setCanvas = (Canvas) => {
    canvasRef.current = Canvas;
    setCanvas(Canvas);
};


const [{ isOver }, drop] = useDrop(() => ({
    accept: "formElement",
    drop: (item) => {
        addElementToCanvas(item.id, generateUUID());
    },
    collect: (monitor) => ({
        isOver: !!monitor.isOver(),
    }),
}));

//Function to add element on to the canvas
const addElementToCanvas = (id, uuid) => {
    const formElement = formElements.find((element) => id === element.id);
    //Add up the necessary details required for the element
    formElement.question = "Type your question here";
    if (!formElement) {
        return;
    }
    if (canvasRef.current === null) {
        canvasRef.current = [];
    }
    const formAreaElements = [...canvasRef.current, { ...formElement, uuid: uuid }];        
    _setCanvas(formAreaElements);
    setfilled(true);
};
const removeElementfromCanvas = (formElement) => {
    const updatedForm = canvas.filter((element) => element.uuid != formElement.uuid)
    _setCanvas(updatedForm);

};

Adding items to the canvas works fine. However while deleting a particular item by its uuid, the state is updated with the rest of the items. But on the DOM, the last item which has been added gets removed, and not the one which has already been deleted from the state.

<div
  className="bg-gray-800 w-full h-screen z-1   overflow-y-auto text-white "
  ref={drop}
>
  {filled &&
    canvasRef.current.map((element, idx) => {
      return (
        <li key={idx} className="mt-8 cursor-default  w-full ">
          <FormElementHolder
            questionNo={idx + 1}
            element={element}
            removeElement={() => removeElementfromCanvas(element)}
            canvasRef={canvasRef.current}
            _setCanvas={_setCanvas}
          />
        </li>
      );
    })}
</div>;

Thanks in advance!

`JSON.parse(JSON.stringify(x))` is basically never a good way to make a deep copy of anything. It's slow (making a pass through JSON text) and lossy (any `undefined` value, or function, or anything else that doesn't have a JSON representation). See [this question's answers](https://stackoverflow.com/questions/122102/what-is-the-most-efficient-way-to-deep-clone-an-object-in-javascript) for how to do a deep copy if you need one, in particular the ones about `structuredClone`. — T.J. Crowder, Feb 01 '22 at 17:28
But when adding/removing objects from the array, there's no reason to do a deep copy of the array at all, just a shallow one. You only need to copy things you change (the array), not things you don't change (the other elements in it). You can replace `const formAreaElements = JSON.parse(JSON.stringify(canvasRef.current)); const newElement = { ...formElement, uuid: uuid }; formAreaElements.push(newElement);` with `const formAreaElements = [...canvasRef.current, { ...formElement, uuid: uuid }];`, for instance. — T.J. Crowder, Feb 01 '22 at 17:31
In the scope of the code shown, I can't see any reason for `canvasRef` being a ref. It looks like it should be state. — T.J. Crowder, Feb 01 '22 at 17:33
The reason why I'm using `canvasRef` as a ref is that react dnd is not updating the reference of the previous state ie `canvas` and so upon adding elements of similar id, then they all get the same UUID as per the latest one. — Sounav Saha, Feb 01 '22 at 17:46
Don't use the array element index as React key, ***especially*** if you are mutating the underlying data, i.e. adding/removing from the middle, sorting, etc... — Drew Reese, Feb 01 '22 at 17:46

David · Answer 1 · 2022-02-09T08:51:19.570

Your element already has an id, it's better to use that instead.
Generally it's a bad practice to use indexes as keys and in this instance it is causing an error.
Index changes when you delete an item and therefore the dnd library can’t recall proper references.
This article is great one to check : https://dev.to/shiv1998/why-not-to-use-index-as-key-in-react-lists-practical-example-3e66

<div
  className="bg-gray-800 w-full h-screen z-1   overflow-y-auto text-white"
  ref={drop}
>
 {filled &&
 canvasRef.current.map((element, idx) => {
  return (
    <li key={element.id} className="mt-8 cursor-default  w-full ">
      <FormElementHolder
        questionNo={idx + 1}
        element={element}
        removeElement={() => removeElementfromCanvas(element)}
        canvasRef={canvasRef.current}
        _setCanvas={_setCanvas}
      />
    </li>
  );
})}

;

T.J. Crowder · Answer 2 · 2022-02-01T17:47:14.240

0

You can't reliably use indexes as keys when you're modifying the array like that. Details in this article linked from the React documentation. The problem is that on re-rendering, React thinks it can keep/reuse the DOM element for the previous thing at that index when you give it something new with that same index as its key.

You seem to have an id on the elements, so use the id as the key.

edited Feb 01 '22 at 17:47

answered Feb 01 '22 at 17:25

T.J. Crowder

1,031,962
187
1,923
1,875

Thanks a lot! I used the uuid as the key when mapping them out. It solved the problem. – Sounav Saha Feb 02 '22 at 05:42

DOM still shows items which are have already been deleted from state in React using hooks

2 Answers2