-
-
Notifications
You must be signed in to change notification settings - Fork 691
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
re-rendering ALL draggable items on drag start and such #1071
Comments
Same issue here. Anytime I drag start the entire page rerenders. This is a problem on pages with 100+ items, the page freezes for 1s+ |
@jonahallibone a workaround for this issue could be creating a Or you can separate the draggable item into 2 components: don't get me wrong, I still think that the right thing to do is not to re-render all of them |
@alissaVrk thanks! that helped a bit! |
@alissaVrk a significant amount of performance-based open issues are both directly and indirectly related to this (e.g., #994, #943, #898, ...). A PR would be highly appreciated! |
@tomasmenezes I will try to create a first draft for next week |
@tomasmenezes PR ready, only for active.
|
@tomasmenezes PR for drappable and sortable is ready All items will still re-render if the passed items to the sortable context change, happens mostly on drop, probably fixable as well, but I had to stop at some point :) tested with the example in #994, seems to solve the issue. |
@alissaVrk Looks awesome! Will also test it out later this week. I think API changes are welcome to deal with issues as big as this one. I think most people here would encourage you to keep on going 😄 |
@tomasmenezes, If this PR will get enough attention to actually get merged, maybe I will :) |
@alissaVrk great work! i hope @clauderic will eventually continue to work on this lib. unfortunately there has been no activity for several months |
@tomasmenezes @kyeo76 Thank you for the feedback :) |
@alissaVrk Thanks for working on this! I'm brand new to dnd-kit, but this is the first thing I noticed - that all draggable and droppable components are re-rendering on start, over and end. |
any updates on this? |
Any updates on this? I am facing performance issues with |
@sophia-kravchenko @sheminanto doesn't seem like it :( |
I am facing this same issue, thanks @alissaVrk for the workaround and @clauderic for this amazing library! |
I've used I was curious if anyone has tried it (also, there's a few other libraries that do something similar)? |
Great inspiration, I haven’t tried, but I also know that https://github.com/dai-shi/use-context-selector can achieve too. I will try it later today, thanks for the solution. |
You can if you use const clonedChild = useMemo(
() =>
cloneElement(children, {
isDragging,
}),
[children, id, isDragging]
)
return (
<div
ref={setNodeRef}
style={style}
{...attributes}
{...listeners}
>
{clonedChild}
</div>
) |
it seems that all draggable components will re-render when one of them is dragged, it happens because of the use of context in
useDraggable
and the way that the context exposes the values, for example, active (which definitely changes when you start dragging).also need to change from exposing the active element on the context to isDragging or myActiveInfo so that components that aren't active won't get a new value that they don't need (with my solution it will be useIsDraggable or useMyActiveInfo)
there is a way around it. the idea is to expose functions instead of values on the context value so it won't re-render all components that use it when something that is irrelevant to them changes.
I wrote a blog post on the subject.
it probably happens in many other cases.. like a change in
over
If you like the idea I can try and prepare a PR
the problem is also that you expose active to the user of
useDraggable
and if we do the change I propose non-active components won't have that information - it will change the API a bitThe text was updated successfully, but these errors were encountered: