React 18 <StrictMode> tweaks and bug fixes (#788)

* Enable strict mode in storybook * Remove unnecessary requestAnimationFrame * Fix issues with droppable measuring in strict mode
clauderic · Jan 4, 2023 · da94c02 · da94c02
1 parent d5c4732
commit da94c02
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 43 deletions.
diff --git a/.changeset/mean-doors-whisper.md b/.changeset/mean-doors-whisper.md
@@ -0,0 +1,6 @@
+---
+"@dnd-kit/core": patch
+"@dnd-kit/sortable": patch
+---
+
+Bug fixes for React 18 Strict Mode
diff --git a/.storybook/main.js b/.storybook/main.js
@@ -3,6 +3,7 @@ const path = require('path');
 module.exports = {
   reactOptions: {
     legacyRootApi: false,
+    strictMode: true,
   },
   staticDirs: ['./assets'],
   stories: ['../stories/**/*.story.tsx'],

diff --git a/packages/core/src/hooks/useDraggable.ts b/packages/core/src/hooks/useDraggable.ts
@@ -56,8 +56,11 @@ export function useDraggable({
     draggableNodes,
     over,
   } = useContext(InternalContext);
-  const {role = defaultRole, roleDescription = 'draggable', tabIndex = 0} =
-    attributes ?? {};
+  const {
+    role = defaultRole,
+    roleDescription = 'draggable',
+    tabIndex = 0,
+  } = attributes ?? {};
   const isDragging = active?.id === id;
   const transform: Transform | null = useContext(
     isDragging ? ActiveDraggableContext : NullContext

diff --git a/packages/core/src/hooks/utilities/useDroppableMeasuring.ts b/packages/core/src/hooks/utilities/useDroppableMeasuring.ts
@@ -35,11 +35,7 @@ export function useDroppableMeasuring(
   containers: DroppableContainer[],
   {dragging, dependencies, config}: Arguments
 ) {
-  const [
-    containerIdsScheduledForMeasurement,
-    setContainerIdsScheduledForMeasurement,
-  ] = useState<UniqueIdentifier[] | null>(null);
-  const measuringScheduled = containerIdsScheduledForMeasurement != null;
+  const [queue, setQueue] = useState<UniqueIdentifier[] | null>(null);
   const {frequency, measure, strategy} = config;
   const containersRef = useRef(containers);
   const disabled = isDisabled();
@@ -50,9 +46,13 @@ export function useDroppableMeasuring(
         return;
       }
 
-      setContainerIdsScheduledForMeasurement((value) =>
-        value ? value.concat(ids) : ids
-      );
+      setQueue((value) => {
+        if (value === null) {
+          return ids;
+        }
+
+        return value.concat(ids.filter((id) => !value.includes(id)));
+      });
     },
     [disabledRef]
   );
@@ -63,13 +63,11 @@ export function useDroppableMeasuring(
         return defaultValue;
       }
 
-      const ids = containerIdsScheduledForMeasurement;
-
       if (
         !previousValue ||
         previousValue === defaultValue ||
         containersRef.current !== containers ||
-        ids != null
+        queue != null
       ) {
         const map: RectMap = new Map();
 
@@ -79,9 +77,9 @@ export function useDroppableMeasuring(
           }
 
           if (
-            ids &&
-            ids.length > 0 &&
-            !ids.includes(container.id) &&
+            queue &&
+            queue.length > 0 &&
+            !queue.includes(container.id) &&
             container.rect.current
           ) {
             // This container does not need to be re-measured
@@ -104,13 +102,7 @@ export function useDroppableMeasuring(
 
       return previousValue;
     },
-    [
-      containers,
-      containerIdsScheduledForMeasurement,
-      dragging,
-      disabled,
-      measure,
-    ]
+    [containers, queue, dragging, disabled, measure]
   );
 
   useEffect(() => {
@@ -123,17 +115,21 @@ export function useDroppableMeasuring(
         return;
       }
 
-      requestAnimationFrame(() => measureDroppableContainers());
+      measureDroppableContainers();
     },
     // eslint-disable-next-line react-hooks/exhaustive-deps
     [dragging, disabled]
   );
 
-  useEffect(() => {
-    if (measuringScheduled) {
-      setContainerIdsScheduledForMeasurement(null);
-    }
-  }, [measuringScheduled]);
+  useEffect(
+    () => {
+      if (queue && queue.length > 0) {
+        setQueue(null);
+      }
+    },
+    //eslint-disable-next-line react-hooks/exhaustive-deps
+    [JSON.stringify(queue)]
+  );
 
   useEffect(
     () => {
@@ -157,7 +153,7 @@ export function useDroppableMeasuring(
   return {
     droppableRects,
     measureDroppableContainers,
-    measuringScheduled,
+    measuringScheduled: queue != null,
   };
 
   function isDisabled() {

diff --git a/packages/sortable/src/components/SortableContext.tsx b/packages/sortable/src/components/SortableContext.tsx
@@ -56,7 +56,6 @@ export function SortableContext({
     droppableRects,
     over,
     measureDroppableContainers,
-    measuringScheduled,
   } = useDndContext();
   const containerId = useUniqueId(ID_PREFIX, id);
   const useDragOverlay = Boolean(dragOverlay.rect !== null);
@@ -77,16 +76,10 @@ export function SortableContext({
   const disabled = normalizeDisabled(disabledProp);
 
   useIsomorphicLayoutEffect(() => {
-    if (itemsHaveChanged && isDragging && !measuringScheduled) {
+    if (itemsHaveChanged && isDragging) {
       measureDroppableContainers(items);
     }
-  }, [
-    itemsHaveChanged,
-    items,
-    isDragging,
-    measureDroppableContainers,
-    measuringScheduled,
-  ]);
+  }, [itemsHaveChanged, items, isDragging, measureDroppableContainers]);
 
   useEffect(() => {
     previousItemsRef.current = items;

diff --git a/packages/sortable/src/hooks/useSortable.ts b/packages/sortable/src/hooks/useSortable.ts
@@ -71,7 +71,12 @@ export function useSortable({
     () => items.slice(items.indexOf(id)),
     [items, id]
   );
-  const {rect, node, isOver, setNodeRef: setDroppableNodeRef} = useDroppable({
+  const {
+    rect,
+    node,
+    isOver,
+    setNodeRef: setDroppableNodeRef,
+  } = useDroppable({
     id,
     data,
     disabled: disabled.droppable,

diff --git a/packages/sortable/src/hooks/utilities/useDerivedTransform.ts b/packages/sortable/src/hooks/utilities/useDerivedTransform.ts
@@ -48,9 +48,7 @@ export function useDerivedTransform({disabled, index, node, rect}: Arguments) {
 
   useEffect(() => {
     if (derivedTransform) {
-      requestAnimationFrame(() => {
-        setDerivedtransform(null);
-      });
+      setDerivedtransform(null);
     }
   }, [derivedTransform]);