Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wdire opened this issue Apr 1, 2024 · 5 comments

Comments

@wdire
Copy link

wdire commented Apr 1, 2024

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
Copy link

lukjaki commented Apr 9, 2024

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
Copy link
Author

wdire commented Apr 10, 2024

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
Copy link

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

@wdire
Copy link
Author

wdire commented Apr 23, 2024

@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
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants