From fe13ce95bd7d214baab6cd5deb7aa9ae1208f509 Mon Sep 17 00:00:00 2001
From: Conrad Chan <conradchan@box.com>
Date: Wed, 16 Dec 2020 15:47:43 -0800
Subject: [PATCH] fix(region): Memoize callback props passed to PointerCapture
 (#656)

* fix(region): Memoize callback props passed to PointerCapture

* chore: trimming back useCallback usage

* chore: pr comments

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 src/drawing/DrawingCreator.tsx | 29 +++++++++++--------
 src/region/RegionCreator.tsx   | 52 ++++++++++++++++++----------------
 2 files changed, 45 insertions(+), 36 deletions(-)

diff --git a/src/drawing/DrawingCreator.tsx b/src/drawing/DrawingCreator.tsx
index 70b589bfc..fd8683c44 100644
--- a/src/drawing/DrawingCreator.tsx
+++ b/src/drawing/DrawingCreator.tsx
@@ -65,7 +65,8 @@ export default function DrawingCreator({ className, onStart, onStop, stroke = de
         capturedPointsRef.current = [{ x: x1, y: y1 }];
         drawingDirtyRef.current = true;
     };
-    const stopDraw = (): void => {
+
+    const stopDraw = React.useCallback((): void => {
         const adjustedPoints = getPoints();
 
         // If there is only one point in the points array, it is likely the user clicked
@@ -83,19 +84,23 @@ export default function DrawingCreator({ className, onStart, onStop, stroke = de
             paths: [{ points: adjustedPoints }],
             stroke,
         });
-    };
-    const updateDraw = (x: number, y: number): void => {
-        const [x2, y2] = getPosition(x, y);
-        const { current: points } = capturedPointsRef;
+    }, [onStop, setDrawingStatus, stroke]);
 
-        points.push({ x: x2, y: y2 });
-        drawingDirtyRef.current = true;
+    const updateDraw = React.useCallback(
+        (x: number, y: number): void => {
+            const [x2, y2] = getPosition(x, y);
+            const { current: points } = capturedPointsRef;
 
-        if (drawingStatus !== DrawingStatus.drawing) {
-            setDrawingStatus(DrawingStatus.drawing);
-            onStart();
-        }
-    };
+            points.push({ x: x2, y: y2 });
+            drawingDirtyRef.current = true;
+
+            if (drawingStatus !== DrawingStatus.drawing) {
+                setDrawingStatus(DrawingStatus.drawing);
+                onStart();
+            }
+        },
+        [drawingStatus, onStart, setDrawingStatus],
+    );
 
     // Event Handlers
     const renderStep = (callback: () => void): void => {
diff --git a/src/region/RegionCreator.tsx b/src/region/RegionCreator.tsx
index 6f3fd1d98..26ab51285 100644
--- a/src/region/RegionCreator.tsx
+++ b/src/region/RegionCreator.tsx
@@ -19,6 +19,9 @@ const MIN_X = 0; // Minimum region x position must be positive
 const MIN_Y = 0; // Minimum region y position must be positive
 const MIN_SIZE = 10; // Minimum region size must be large enough to be clickable
 
+const isValid = (x1: number, y1: number, x2: number, y2: number): boolean =>
+    Math.abs(x2 - x1) >= MIN_SIZE || Math.abs(y2 - y1) >= MIN_SIZE;
+
 export default function RegionCreator({ className, onAbort, onStart, onStop }: Props): JSX.Element {
     const [drawingStatus, setDrawingStatus] = React.useState<DrawingStatus>(DrawingStatus.init);
     const [isHovering, setIsHovering] = React.useState<boolean>(false);
@@ -95,7 +98,8 @@ export default function RegionCreator({ className, onAbort, onStart, onStop }: P
         positionY2Ref.current = null;
         regionDirtyRef.current = true;
     };
-    const stopDraw = (): void => {
+
+    const stopDraw = React.useCallback((): void => {
         const shape = getShape();
 
         setDrawingStatus(DrawingStatus.init);
@@ -111,31 +115,31 @@ export default function RegionCreator({ className, onAbort, onStart, onStop }: P
         } else {
             onAbort();
         }
-    };
-
-    const isValid = (x1: number, y1: number, x2: number, y2: number): boolean =>
-        Math.abs(x2 - x1) >= MIN_SIZE || Math.abs(y2 - y1) >= MIN_SIZE;
-
-    const updateDraw = (x: number, y: number): void => {
-        const [x2, y2] = getPosition(x, y);
-        const { current: x1 } = positionX1Ref;
-        const { current: y1 } = positionY1Ref;
-        const { current: prevX2 } = positionX2Ref;
-
-        // Suppress the creation of a small region if the intention of the user is to click on the document
-        if (prevX2 === null && !isValid(x1 ?? 0, y1 ?? 0, x2, y2)) {
-            return;
-        }
+    }, [onAbort, onStop, setDrawingStatus]);
+
+    const updateDraw = React.useCallback(
+        (x: number, y: number): void => {
+            const [x2, y2] = getPosition(x, y);
+            const { current: x1 } = positionX1Ref;
+            const { current: y1 } = positionY1Ref;
+            const { current: prevX2 } = positionX2Ref;
+
+            // Suppress the creation of a small region if the intention of the user is to click on the document
+            if (prevX2 === null && !isValid(x1 ?? 0, y1 ?? 0, x2, y2)) {
+                return;
+            }
 
-        positionX2Ref.current = x2;
-        positionY2Ref.current = y2;
-        regionDirtyRef.current = true;
+            positionX2Ref.current = x2;
+            positionY2Ref.current = y2;
+            regionDirtyRef.current = true;
 
-        if (drawingStatus !== DrawingStatus.drawing) {
-            setDrawingStatus(DrawingStatus.drawing);
-            onStart();
-        }
-    };
+            if (drawingStatus !== DrawingStatus.drawing) {
+                setDrawingStatus(DrawingStatus.drawing);
+                onStart();
+            }
+        },
+        [drawingStatus, onStart, setDrawingStatus],
+    );
 
     // Event Handlers
     const handleMouseOut = (): void => {