From be82b145501bd1af48e44f068cc157c088711823 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Tue, 9 May 2023 09:20:01 -0500
Subject: [PATCH] fix: DH-14630: useDebouncedViewportSearch: memoization bug
 (#1273)

Supports DH-14630

fixes #1272
---
 .../src/useDebouncedViewportSearch.ts         | 19 ++++++++++++++-----
 .../src/useInitializeViewportData.ts          |  3 ++-
 .../jsapi-components/src/useViewportData.ts   |  5 ++++-
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/packages/jsapi-components/src/useDebouncedViewportSearch.ts b/packages/jsapi-components/src/useDebouncedViewportSearch.ts
index de0850ba00..91fa5cf86c 100644
--- a/packages/jsapi-components/src/useDebouncedViewportSearch.ts
+++ b/packages/jsapi-components/src/useDebouncedViewportSearch.ts
@@ -2,9 +2,12 @@ import debounce from 'lodash.debounce';
 import type { Table, TreeTable } from '@deephaven/jsapi-types';
 import { TableUtils } from '@deephaven/jsapi-utils';
 import { useApi } from '@deephaven/jsapi-bootstrap';
+import Log from '@deephaven/log';
 import { useEffect, useMemo } from 'react';
 import { UseViewportDataResult } from './useViewportData';
 
+const log = Log.module('useDebouncedViewportSearch');
+
 export const DEBOUNCE_VIEWPORT_SEARCH_MS = 200;
 
 /**
@@ -13,6 +16,7 @@ export const DEBOUNCE_VIEWPORT_SEARCH_MS = 200;
  * @param viewportData Table viewport to filter
  * @param columnName Column name to filter by
  * @param debounceMs Millisecond value to debounce
+ * @returns A debounced search function
  */
 export default function useDebouncedViewportSearch<
   I,
@@ -24,34 +28,39 @@ export default function useDebouncedViewportSearch<
 ): (searchText: string) => void {
   const dh = useApi();
   const tableUtils = useMemo(() => new TableUtils(dh), [dh]);
+  const { table, applyFiltersAndRefresh } = viewportData;
+
   const debouncedSearch = useMemo(
     () =>
       debounce((searchText: string) => {
-        if (viewportData.table == null) {
+        log.debug(`Applying debounced searchText '${searchText}'`);
+
+        if (table == null) {
           return;
         }
 
         const searchTextTrimmed = searchText.trim();
 
         if (searchTextTrimmed === '') {
-          viewportData.applyFiltersAndRefresh([]);
+          applyFiltersAndRefresh([]);
           return;
         }
 
-        const column = viewportData.table.findColumn(columnName);
+        const column = table.findColumn(columnName);
         const value = tableUtils.makeFilterValue(
           column.type,
           searchTextTrimmed
         );
         const filter = [column.filter().contains(value)];
 
-        viewportData.applyFiltersAndRefresh(filter);
+        applyFiltersAndRefresh(filter);
       }, debounceMs),
-    [columnName, debounceMs, tableUtils, viewportData]
+    [applyFiltersAndRefresh, columnName, debounceMs, table, tableUtils]
   );
 
   useEffect(
     () => () => {
+      log.debug('Cancelling debounced search function');
       debouncedSearch.cancel();
     },
     [debouncedSearch]
diff --git a/packages/jsapi-components/src/useInitializeViewportData.ts b/packages/jsapi-components/src/useInitializeViewportData.ts
index 4af1a433ad..bf6575da4f 100644
--- a/packages/jsapi-components/src/useInitializeViewportData.ts
+++ b/packages/jsapi-components/src/useInitializeViewportData.ts
@@ -15,7 +15,8 @@ import useTableSize from './useTableSize';
  * source table. This is intended for "human" sized tables such as those used in
  * admin panels. This is not suitable for "machine" scale with millions+ rows.
  * @param table The table that will be used to determine the list size.
- * @returns
+ * @returns a React Stately ListData object. Note that this object is recreated
+ * by React Stately on every render.
  */
 export default function useInitializeViewportData<T>(
   table: Table | TreeTable | null
diff --git a/packages/jsapi-components/src/useViewportData.ts b/packages/jsapi-components/src/useViewportData.ts
index cc2f83501a..3eeae4d235 100644
--- a/packages/jsapi-components/src/useViewportData.ts
+++ b/packages/jsapi-components/src/useViewportData.ts
@@ -47,7 +47,10 @@ export interface UseViewportDataResult<
  * @param table
  * @param viewportSize
  * @param viewportPadding
- * @returns An object for managing Table viewport state.
+ * @returns An object for managing Table viewport state. Note that the returned
+ * object changes on every render due to the `viewportData` not being memoized.
+ * This is due to the underlying React Stately `useListData` implementation that
+ * also changes its returned object on every render.
  */
 export default function useViewportData<
   TItem,