From 7d26a76c26e71e28cb7d0397cefd2c5193e6ec70 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Thu, 25 Jan 2024 18:46:57 -0600
Subject: [PATCH] useDebouncedValue cleanup

#1747
---
 .../components/src/modal/DebouncedModal.tsx   |  2 +-
 .../src/useCheckIfExistsValue.ts              |  5 ++++-
 .../useDebouncedViewportSelectionFilter.ts    |  2 +-
 packages/react-hooks/src/useDebouncedValue.ts | 20 +++++++++++--------
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/packages/components/src/modal/DebouncedModal.tsx b/packages/components/src/modal/DebouncedModal.tsx
index da8fec67e9..290926a0f5 100644
--- a/packages/components/src/modal/DebouncedModal.tsx
+++ b/packages/components/src/modal/DebouncedModal.tsx
@@ -28,7 +28,7 @@ function DebouncedModal({
   debounceMs,
   isOpen = false,
 }: DebouncedModalProps): JSX.Element {
-  const debouncedIsOpen = useDebouncedValue(isOpen, debounceMs);
+  const { value: debouncedIsOpen } = useDebouncedValue(isOpen, debounceMs);
 
   return (
     <>
diff --git a/packages/jsapi-components/src/useCheckIfExistsValue.ts b/packages/jsapi-components/src/useCheckIfExistsValue.ts
index 5b52015486..078df50f80 100644
--- a/packages/jsapi-components/src/useCheckIfExistsValue.ts
+++ b/packages/jsapi-components/src/useCheckIfExistsValue.ts
@@ -36,7 +36,10 @@ export function useCheckIfExistsValue(
   const tableUtils = useTableUtils();
 
   const [valueTrimmed, setValueTrimmed] = useState('');
-  const valueTrimmedDebounced = useDebouncedValue(valueTrimmed, debounceMs);
+  const { value: valueTrimmedDebounced } = useDebouncedValue(
+    valueTrimmed,
+    debounceMs
+  );
 
   const trimAndUpdateValue = useCallback((text: string) => {
     setValueTrimmed(text.trim());
diff --git a/packages/jsapi-components/src/useDebouncedViewportSelectionFilter.ts b/packages/jsapi-components/src/useDebouncedViewportSelectionFilter.ts
index dd64864d8c..75ea6a89d7 100644
--- a/packages/jsapi-components/src/useDebouncedViewportSelectionFilter.ts
+++ b/packages/jsapi-components/src/useDebouncedViewportSelectionFilter.ts
@@ -44,7 +44,7 @@ export function useDebouncedViewportSelectionFilter<TItem, TValue>({
 
   // Debounce so user can rapidly select multiple items in a row without the
   // cost of updating the table on each change
-  const debouncedValuesSelection = useDebouncedValue(
+  const { value: debouncedValuesSelection } = useDebouncedValue(
     valuesSelection,
     DEBOUNCE_MS
   );
diff --git a/packages/react-hooks/src/useDebouncedValue.ts b/packages/react-hooks/src/useDebouncedValue.ts
index 93c9969ea8..f17ffca0a8 100644
--- a/packages/react-hooks/src/useDebouncedValue.ts
+++ b/packages/react-hooks/src/useDebouncedValue.ts
@@ -1,4 +1,4 @@
-import { useEffect, useMemo, useState } from 'react';
+import { useEffect, useState } from 'react';
 
 /**
  * Debounces a value.
@@ -13,18 +13,22 @@ export function useDebouncedValue<T>(
   debounceMs: number
 ): { isDebouncing: boolean; value: T } {
   const [isDebouncing, setIsDebouncing] = useState(true);
-  const [debouncedValue, setDebouncedValue] = useState<T>(value);
+  const [debouncedValue, setDebouncedValue] = useState(value);
 
-  // Set isDebouncing to true immediately whenever the value changes. Using
-  // `useMemo` instead of `useEffect` so that state is never out of sync whenever
-  // value and / or debounceMs have changed.
-  useMemo(() => {
+  // Keep `isDebouncing` in sync with `value` and `debounceMs` by setting state
+  // during render instead of in `useEffect`
+  // https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
+  const [previousValue, setPreviousValue] = useState(value);
+  const [previousDebounceMs, setPreviousDebounceMs] = useState(debounceMs);
+  if (value !== previousValue || debounceMs !== previousDebounceMs) {
     setIsDebouncing(true);
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [value, debounceMs]);
+    setPreviousValue(value);
+    setPreviousDebounceMs(debounceMs);
+  }
 
   useEffect(() => {
     let isCancelled = false;
+    setIsDebouncing(true);
 
     const timeoutId = setTimeout(() => {
       if (!isCancelled) {