clauderic / dnd-kit

The modern, lightweight, performant, accessible and extensible drag & drop toolkit for React.
http://dndkit.com
MIT License
13.16k stars 653 forks source link

useSortable re-renders all items even with just clicking one item, affecting performance #1379

Open wdire opened 7 months ago

wdire commented 7 months ago

I noticed a lag while moving a sortable item on mobile when having around 50 sortable items. I've already noticed that when even only clicking a item it does render every other items, idk why. So I moved everything beside useSortable to ContentCardMemo that is wrapped with React.memo, that memo component doesn't re-render every time and I thought its ok now. I've made sure that useSortable is the one causing all re-renders, the container that maps all ContentCards is not the one re-rendering.

It might have improved with memo but I'm having performance issues on mobile with a merely 50 items. First I thought my way of storing/updating items might have causing the issue, storing it in a array of objects, when order changes it recreates whole items with arrayMove. But considering container not being re-rendered and just touching a sortable item causes total 200 re-renders, the issue is about useSortable itself.

Though on desktop it's not as bad as mobile, it still shows lag, performance issue when the list grows more.

Is there a way to fix this?

Versions: "@dnd-kit/core": "^6.1.0", "@dnd-kit/modifiers": "^7.0.0", "@dnd-kit/sortable": "^8.0.0", // also tried with 7.0.2, same "@dnd-kit/utilities": "^3.2.2",

Code at issue:

// ContentCard.tsx

const ContentCard = memo(function ContentCard({content}: Props) {
  const redirectSourcePage = useAppSelector((state) => state.list.redirectSourcePage);

  const {setNodeRef, attributes, listeners, transform, transition, isDragging} = useSortable({
    id: content.id,
    data: {
      type: "Content",
    },
    disabled: redirectSourcePage,
  });

  // This runs 4 times each. If there are 50 items, 200 times even with just touching to any item.
  console.log("card", content.id);

  return (
    <div
      ref={setNodeRef}
      style={{
        transition,
        transform: CSS.Translate.toString(transform),
      }}
      {...attributes}
      {...listeners}
      data-contentcard="true"
    >
      <ContentCardMemo
        content={content}
        redirectSourcePage={redirectSourcePage}
        isDragging={isDragging}
      />
    </div>
  );
});

export default ContentCard;
// index.tsx

...
const sensors = useSensors(
  useSensor(PointerSensor, {
    activationConstraint: {
      distance: 10,
    },
  }),
  useSensor(TouchSensor, {
    activationConstraint: {
      distance: 10,
    },
  }),
);
...
<DndContext
  id="main-dnd"
  sensors={sensors}
  onDragStart={handleDragStart}
  onDragEnd={handleDragEnd}
  onDragMove={handleDragMove}
  onDragCancel={() => console.log("onDragCancel")}
  autoScroll={false}
  modifiers={[restrictToWindowEdges]}
  collisionDetection={pointerWithin}
>
  ...
  <SortableContext items={contentIds}>{contentsMemo}</SortableContext>
  ...
</DndContext>
lukjaki commented 7 months ago

Encountered this but got solved by wrapping whole component in memo() where the second argument is the function that checks if component should rerender.

wdire commented 7 months ago

For people having similar usage and problem. In addition to using memo I made performance manageable with removing items array changes in onDragMove, depending on DragOverlay except when item changes between Sortable Containers. And used debounce on onDragMove. It cost a bit smoothness while dragging between containers tho.

igorbrasileiro commented 7 months ago

I'm facing the same issue @wdire , can you share the final idea of your code, to inspire me?

wdire commented 6 months ago

@igorbrasileiro in addition to the code I shared above with memo, making sure container doesn't re-render etc. I've added debounce like this on DndContext: onDragOver={debounce(handleDragMove, 75)}.

Going from my example, even though parent of ContentCard doesn't re-render, useSortable itself re-renders while dragging and affects the performance, as I've interpreted. So adding debounce to onDragOver is like dropping down the FPS of drag movement a bit, 75 value felt like balanced between performance and user experience for me.

DakotaWray2 commented 6 months ago

@wdire personally I experienced issues with keeping track of items jumping over containers in a kanban layout while testing your debounce recommendation. For example jumping from container column 1 -> container column 3.

dinkinflickaa commented 1 month ago

This happens when DndContext updates the activeSensor and activatorEvent states on a mouse down event. These state changes cause rerenders in all consumers of DndContext. Consumers of useSortable re-render too because SortableContext depends on DndContext.

1494 should fix this perf bottleneck.