Do I have a stale closure in my React app using react-dnd for drag and drop?

Question

I am building a drag-n-drop application that lets users build up a tree of items that they've dragged onto a list.

When dragging items onto a list, there are two possible actions:

• The item is dropped onto the list and added
• The item is dropped onto an existing item and is added as a child of that item

There is a bug when adding child items. I have reproduced the bug in CodeSandbox here: https://codesandbox.io/s/react-dnd-possible-stale-closure-2mpjju?file=/src/index.tsx**

Steps to reproduce the bug:

• Open the CodeSandbox link
• Drag 3 or more items onto the list
• Drag an item and drop it onto the first or second item
• You will see it successfully adds the item as a child, but the root items below it get removed

I will explain what I think is happening below, but here's an overview of the code:

A list of draggable items:

export function DraggableItems() {
  const componentTypes = ["Car", "Truck", "Boat"];

  return (
    <div>
      <h4>These can be dragged:</h4>

      {componentTypes.map((x) => (
        <DraggableItem itemType={x} />
      ))}
    </div>
  );
}

function DraggableItem({ itemType }: DraggableItemProps) {
  const [, drag] = useDrag(() => ({
    type: "Item",
    item: { itemType: itemType }
  }));

  return <div ref={drag}>{itemType}</div>;
}

...these items can be dropped on two places: the DropPane and the previously DroppedItems.

DropPane:

export function DropPane() {
  const [items, setItems] = useState<Array<Item>>([]);

  const [, drop] = useDrop(
    () => ({
      accept: "Item",
      drop: (droppedObj: any, monitor: any) => {
        if (monitor.didDrop()) {
          console.log("Drop already processed by nested dropzone");
        } else {
          const newItem = makeRandomItem(droppedObj.itemType);
          setItems([...items, newItem]);
        }
      }
    }),
    [items]
  );

  const deleteItem = (idToDelete: number) => {
    // a recursive function which filters out the item with idToDelete
    const deleteItemRecursively = (list: Array<Item>) => {
      const filteredList = list.filter((x) => x.id !== idToDelete);

      if (filteredList.length < list.length) {
        return filteredList;
      } else {
        return list.map((x) => {
          x.children = deleteItemRecursively(x.children);

          return x;
        });
      }
    };

    // the recursive function is called initially with the items state object
    const listWithTargetDeleted = deleteItemRecursively(items);
    setItems(listWithTargetDeleted);
  };

  const addItemAsChild = (child: Item, targetParent: Item) => {
    // same as the delete function, this recursive function finds the correct
    // parent and adds the child item to its list of children
    const addItemAsChildRecursively = (list: Array<Item>) => {
      return list.map((x) => {
        if (x.id === targetParent.id) {
          x.children.push(child);
          return x;
        } else {
          x.children = addItemAsChildRecursively(x.children);
          return x;
        }
      });
    };

    // it's called initially with the items state object
    const reportComponentsWithChildAdded = addItemAsChildRecursively(items);
    setItems(reportComponentsWithChildAdded);
  };

  return (
    <div ref={drop} style={{ border: "1px solid black", marginTop: "2em" }}>
      <h4>You can any items anywhere here to add them to the list:</h4>

      {items.length === 0 || (
        <DroppedItemList
          items={items}
          onDeleteClicked={deleteItem}
          addItemAsChild={addItemAsChild}
          indentation={0}
        />
      )}
    </div>
  );
}

It's the addItemAsChild function that I believe may be causing the error, but I am not sure since if there is a stale closure - i.e. the items list is getting wrapped and passed into the DroppedItemList to be called - I would think it would happen for the deleteItem function, but that method works fine.

To elaborate, if I add 5 items to the list, then add a breakpoint in addItemsAsChild and drop an item on the #1 in the list (to add it as a child), the items state object only has one item in it (it should have 5 since there are 5 items on screen). If I drop an item onto the 2nd item in the list, the item state object has 2 items in it instead of 5... and so on. It seems that the item state gets closed within addItemsAsChild when that item is rendered, but this is only happening for addItemsAsChild and not for the delete?

I cannot figure out why this is happening and several fixes have failed. Can anyone help? Alternative approaches are welcome if you think I'm doing something wrong.

Answer 1

Just figured this out after many wasted hours. react-dnd really need to improve their documentation as this is not an adequate explanation of what the useDrop() hook needs:

depsA dependency array used for memoization. This behaves like the built-in useMemoReact hook. The default value is an empty array for function spec, and an array containing the spec for an object spec.

The translation is that any state objects that will be modified by any callback within useDrop needs to be referenced in the dependency array.

In my DropPane I have a list of components and they appear in the dep array for the useDrop at that level, but in the DroppedItem I have another useDrop.

The solution is the prop-drill the items array all the way down to the DroppedItem component and add the items array as a dependency. I will update the CodeSandbox just for future reference.