fix: DH-14630: useDebouncedViewportSearch: memoization bug (#1273)

Supports DH-14630 fixes #1272
deephaven · May 9, 2023 · be82b14 · be82b14
1 parent 588cb8f
commit be82b14
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 7 deletions.
diff --git 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
@@ -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
@@ -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,