perf: use listener cleanups and proper hook dependencies

Author: itsjavi

packages/dragswag/src/react/drag-overlay.tsx

@@ -7,6 +7,15 @@ type DragOverlayContextType = {
   setDragElementPosition: (position: { top: number; left: number }) => void
 }
 
+// Create context with safe default values
+const defaultContextValue: DragOverlayContextType = {
+  dragElement: null,
+  setDragElement: () => {},
+  setDragElementPosition: () => {},
+}
+
+const DragOverlayContext = createContext<DragOverlayContextType>(defaultContextValue)
+
 interface DragOverlayProviderProps {
   children: React.ReactNode
   style?: React.CSSProperties
@@ -43,8 +52,6 @@ function DragOverlay({ ref, style, children: dragElement, ...props }: React.Comp
   )
 }
 
-const DragOverlayContext = createContext<DragOverlayContextType | null>(null)
-
 export function DragOverlayProvider({ children, ...rest }: DragOverlayProviderProps) {
   const [dragElement, setDragElement] = useState<ReactElement | null>(null)
   const dragWrapperRef = useRef<HTMLDivElement>(null)
@@ -65,17 +72,11 @@ export function DragOverlayProvider({ children, ...rest }: DragOverlayProviderPr
   )
 }
 
-const noContext: DragOverlayContextType = {
-  dragElement: null,
-  setDragElement: () => {},
-  setDragElementPosition: () => {},
-}
-
 export function useDragOverlayElement(): DragOverlayContextType {
   const context = useContext(DragOverlayContext)
   if (!context) {
     console.warn('useDragOverlayElement must be used within a DragOverlayProvider')
-    return noContext
+    return defaultContextValue
   }
   return context
 }

packages/dragswag/src/react/use-draggable.tsx

@@ -19,6 +19,8 @@ export function useDraggable(config: DraggableConfig) {
     data: null as any,
     currentDragComponent: null as React.ReactElement | null,
     elementDimensions: { width: 0, height: 0 },
+    componentCleanup: null as (() => void) | null,
+    dragComponentCleanup: null as (() => void) | null,
   })
 
   refs.current.config = config
@@ -127,18 +129,36 @@ export function useDraggable(config: DraggableConfig) {
     plugins: config.plugins,
   }
 
-  const dragSource = useMemo(() => createDragSource(trueConfig), [])
+  // Create dragSource only once but update its config when it changes
+  const dragSource = useMemo(() => {
+    const source = createDragSource(trueConfig)
+    return source
+  }, [])
 
-  dragSource.setConfig(trueConfig)
+  // Update config when it changes
+  useEffect(() => {
+    dragSource.setConfig(trueConfig)
+  }, [
+    dragSource,
+    config.disabled,
+    config.kind,
+    config.data,
+    config.shouldDrag,
+    config.mouseConfig,
+    config.plugins,
+    config.onDragStart,
+    config.onDragMove,
+    config.onDragEnd,
+  ])
 
   const componentRef = useCallback(
     (element: HTMLElement | null) => {
       const current = refs.current
 
       if (element) {
         current.element = element
-
-        dragSource.listen(element)
+        // Store the cleanup function
+        current.componentCleanup = dragSource.listen(element)
       }
 
       const ref = current.originalRef
@@ -154,8 +174,11 @@ export function useDraggable(config: DraggableConfig) {
 
   const dragComponentRef = useCallback(
     (element: HTMLElement | null) => {
+      const current = refs.current
+
       if (element) {
-        dragSource.listen(element)
+        // Store the cleanup function
+        current.dragComponentCleanup = dragSource.listen(element)
       }
     },
     [dragSource],
@@ -218,6 +241,19 @@ export function useDraggable(config: DraggableConfig) {
     }
   }, [isDragging, setDragElement])
 
+  // Clean up all listeners when component unmounts
+  useEffect(() => {
+    return () => {
+      const current = refs.current
+      if (current.componentCleanup) {
+        current.componentCleanup()
+      }
+      if (current.dragComponentCleanup) {
+        current.dragComponentCleanup()
+      }
+    }
+  }, [])
+
   return {
     draggable,
     isDragging,

packages/dragswag/src/react/use-droppable.tsx

@@ -1,4 +1,4 @@
-import React, { useCallback, useMemo, useRef, useState } from 'react'
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
 import { type DragSourceType, type DropTargetConfig, createDropTarget } from '../core'
 import type { DroppableConfig, Kind } from './types'
 import { getDropTargets } from './utils'
@@ -12,11 +12,20 @@ type HoveredData = {
 
 export function useDroppable(config: DroppableConfig) {
   const [hovered, setHovered] = useState<HoveredData | null>(null)
+  const cleanupRef = useRef<(() => void) | null>(null)
+  const previousAcceptsRef = useRef(config.accepts)
 
   let { accepts } = config
 
   const trueAccepts = Array.isArray(accepts) || typeof accepts === 'function' ? accepts : [accepts]
 
+  // Detect if accepts has changed
+  const acceptsChanged = useMemo(() => {
+    const hasChanged = previousAcceptsRef.current !== accepts
+    previousAcceptsRef.current = accepts
+    return hasChanged
+  }, [accepts])
+
   const trueConfig: DropTargetConfig<any> = {
     disabled: config.disabled,
     accepts: trueAccepts as unknown as DragSourceType<any>[],
@@ -74,35 +83,65 @@ export function useDroppable(config: DroppableConfig) {
     },
   }
 
+  // Create dropTarget only once
   const dropTarget = useMemo(() => createDropTarget(trueConfig), [])
 
-  dropTarget.setConfig(trueConfig)
+  // Update config when it changes
+  useEffect(() => {
+    dropTarget.setConfig(trueConfig)
+  }, [
+    dropTarget,
+    config.disabled,
+    config.data,
+    config.onDragIn,
+    config.onDragOut,
+    config.onDragMove,
+    config.onDrop,
+    acceptsChanged, // Use the memoized value that indicates if accepts changed
+  ])
 
   const originalRef = useRef(null as any)
 
-  const dropComponentRef = useCallback((element: HTMLElement | null) => {
-    if (element) {
-      dropTarget.listen(element)
-    }
+  const dropComponentRef = useCallback(
+    (element: HTMLElement | null) => {
+      if (element) {
+        // Store the cleanup function
+        cleanupRef.current = dropTarget.listen(element)
+      }
 
-    const ref = originalRef.current
+      const ref = originalRef.current
 
-    if (typeof ref === 'function') {
-      ref(element)
-    } else if (ref && ref.hasOwnProperty('current')) {
-      ref.current = element
-    }
-  }, [])
+      if (typeof ref === 'function') {
+        ref(element)
+      } else if (ref && ref.hasOwnProperty('current')) {
+        ref.current = element
+      }
+    },
+    [dropTarget],
+  )
 
-  const droppable = useCallback((child: React.ReactElement<React.ComponentPropsWithRef<any>> | null) => {
-    if (!child) {
-      return null
-    }
+  const droppable = useCallback(
+    (child: React.ReactElement<React.ComponentPropsWithRef<any>> | null) => {
+      if (!child) {
+        return null
+      }
 
-    // @ts-ignore React 16-19+ refs compatibility.
-    originalRef.current = child.props?.ref ?? child.ref
+      // @ts-ignore React 16-19+ refs compatibility.
+      originalRef.current = child.props?.ref ?? child.ref
 
-    return React.cloneElement(child, { ref: dropComponentRef })
+      return React.cloneElement(child, { ref: dropComponentRef })
+    },
+    [dropComponentRef],
+  )
+
+  // Clean up all listeners when component unmounts
+  useEffect(() => {
+    return () => {
+      if (cleanupRef.current) {
+        cleanupRef.current()
+        cleanupRef.current = null
+      }
+    }
   }, [])
 
   return {