fix: Typing in notebooks is laggy (#1977)

- We were checking the diff of the layouts on each keystroke
- DashboardLayout should debounce checking that diff
- Tested by displaying FPS counter, having a Code Studio with a bunch of
tables in it, adding a notebook, and then typing in it. FPS is much
higher after the change

Author: mofojed

packages/dashboard/src/DashboardLayout.tsx

@@ -14,7 +14,7 @@ import type {
   ReactComponentConfig,
 } from '@deephaven/golden-layout';
 import Log from '@deephaven/log';
-import { usePrevious } from '@deephaven/react-hooks';
+import { usePrevious, useThrottledCallback } from '@deephaven/react-hooks';
 import { RootState } from '@deephaven/redux';
 import { useDispatch, useSelector } from 'react-redux';
 import PanelManager, { ClosedPanels } from './PanelManager';
@@ -46,6 +46,8 @@ const DEFAULT_LAYOUT_CONFIG: DashboardLayoutConfig = [];
 
 const DEFAULT_CALLBACK = (): void => undefined;
 
+const STATE_CHANGE_THROTTLE_MS = 1000;
+
 // If a component isn't registered, just pass through the props so they are saved if a plugin is loaded later
 const FALLBACK_CALLBACK = (props: unknown): unknown => props;
 
@@ -195,6 +197,34 @@ export function DashboardLayout({
     ]
   );
 
+  // Throttle the calls so that we don't flood comparing these layouts
+  const throttledProcessDehydratedLayoutConfig = useThrottledCallback(
+    (dehydratedLayoutConfig: DashboardLayoutConfig) => {
+      const hasChanged =
+        lastConfig == null ||
+        !LayoutUtils.isEqual(lastConfig, dehydratedLayoutConfig);
+
+      log.debug('handleLayoutStateChanged', hasChanged, dehydratedLayoutConfig);
+
+      if (hasChanged) {
+        setIsDashboardEmpty(layout.root.contentItems.length === 0);
+
+        setLastConfig(dehydratedLayoutConfig);
+
+        onLayoutChange(dehydratedLayoutConfig);
+
+        setLayoutChildren(layout.getReactChildren());
+      }
+    },
+    STATE_CHANGE_THROTTLE_MS,
+    { flushOnUnmount: true }
+  );
+
+  useEffect(
+    () => () => throttledProcessDehydratedLayoutConfig.flush(),
+    [throttledProcessDehydratedLayoutConfig]
+  );
+
   const handleLayoutStateChanged = useCallback(() => {
     // we don't want to emit stateChanges that happen during item drags or else
     // we risk the last saved state being one without that panel in the layout entirely
@@ -206,27 +236,13 @@ export function DashboardLayout({
       contentConfig,
       dehydrateComponent
     );
-    const hasChanged =
-      lastConfig == null ||
-      !LayoutUtils.isEqual(lastConfig, dehydratedLayoutConfig);
-
-    log.debug(
-      'handleLayoutStateChanged',
-      hasChanged,
-      contentConfig,
-      dehydratedLayoutConfig
-    );
-
-    if (hasChanged) {
-      setIsDashboardEmpty(layout.root.contentItems.length === 0);
-
-      setLastConfig(dehydratedLayoutConfig);
-
-      onLayoutChange(dehydratedLayoutConfig);
-
-      setLayoutChildren(layout.getReactChildren());
-    }
-  }, [dehydrateComponent, isItemDragging, lastConfig, layout, onLayoutChange]);
+    throttledProcessDehydratedLayoutConfig(dehydratedLayoutConfig);
+  }, [
+    dehydrateComponent,
+    isItemDragging,
+    layout,
+    throttledProcessDehydratedLayoutConfig,
+  ]);
 
   const handleLayoutItemPickedUp = useCallback(
     (component: Container) => {

packages/react-hooks/package.json

@@ -25,6 +25,7 @@
     "@deephaven/log": "file:../log",
     "@deephaven/utils": "file:../utils",
     "lodash.debounce": "^4.0.8",
+    "lodash.throttle": "^4.1.1",
     "shortid": "^2.2.16"
   },
   "peerDependencies": {

packages/react-hooks/src/index.ts

@@ -7,6 +7,7 @@ export * from './useCheckOverflow';
 export * from './useContentRect';
 export { default as useContextOrThrow } from './useContextOrThrow';
 export * from './useDebouncedCallback';
+export * from './useThrottledCallback';
 export * from './useDelay';
 export * from './useDependentState';
 export * from './useEffectNTimesWhen';

packages/react-hooks/src/useDebouncedCallback.test.ts

@@ -10,6 +10,10 @@ beforeEach(() => {
   jest.useFakeTimers();
 });
 
+afterEach(() => {
+  jest.useRealTimers();
+});
+
 it('should debounce a given callback', () => {
   const { result } = renderHook(() =>
     useDebouncedCallback(callback, debounceMs)

packages/react-hooks/src/useThrottledCallback.test.ts

@@ -0,0 +1,119 @@
+import { renderHook } from '@testing-library/react-hooks';
+import useThrottledCallback from './useThrottledCallback';
+
+const callback = jest.fn((text: string) => undefined);
+const arg = 'mock.arg';
+const arg2 = 'mock.arg2';
+const throttleMs = 400;
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  jest.useFakeTimers();
+});
+
+afterEach(() => {
+  jest.useRealTimers();
+});
+
+it('should throttle a given callback', () => {
+  const { result } = renderHook(() =>
+    useThrottledCallback(callback, throttleMs)
+  );
+
+  result.current(arg);
+  result.current(arg);
+
+  jest.advanceTimersByTime(5);
+
+  result.current(arg);
+
+  result.current(arg2);
+
+  expect(callback).toHaveBeenCalledTimes(1);
+  expect(callback).toHaveBeenCalledWith(arg);
+
+  jest.clearAllMocks();
+
+  jest.advanceTimersByTime(throttleMs);
+  expect(callback).toHaveBeenCalledTimes(1);
+  expect(callback).toHaveBeenCalledWith(arg2);
+});
+
+it('should cancel throttle if component unmounts', () => {
+  const { result, unmount } = renderHook(() =>
+    useThrottledCallback(callback, throttleMs)
+  );
+
+  result.current(arg);
+  result.current(arg2);
+
+  jest.advanceTimersByTime(throttleMs - 1);
+
+  expect(callback).toHaveBeenCalledTimes(1);
+  expect(callback).toHaveBeenCalledWith(arg);
+  callback.mockClear();
+
+  jest.spyOn(result.current, 'cancel');
+
+  unmount();
+
+  expect(result.current.cancel).toHaveBeenCalled();
+
+  jest.advanceTimersByTime(5);
+
+  expect(callback).not.toHaveBeenCalled();
+});
+
+it('should call the updated callback if the ref changes', () => {
+  const { rerender, result } = renderHook(
+    fn => useThrottledCallback(fn, throttleMs),
+    {
+      initialProps: callback,
+    }
+  );
+
+  result.current(arg);
+  result.current(arg2);
+
+  // Leading is always called
+  expect(callback).toHaveBeenCalledTimes(1);
+  expect(callback).toHaveBeenCalledWith(arg);
+  callback.mockClear();
+
+  jest.advanceTimersByTime(throttleMs - 1);
+
+  const newCallback = jest.fn();
+  rerender(newCallback);
+
+  jest.advanceTimersByTime(1);
+
+  expect(callback).not.toHaveBeenCalled();
+  expect(newCallback).toHaveBeenCalledTimes(1);
+  expect(newCallback).toHaveBeenCalledWith(arg2);
+});
+
+it('should flush on unmount if that option is set', () => {
+  const { result, unmount } = renderHook(() =>
+    useThrottledCallback(callback, throttleMs, { flushOnUnmount: true })
+  );
+
+  result.current(arg);
+  result.current(arg2);
+
+  jest.advanceTimersByTime(throttleMs - 1);
+
+  expect(callback).toHaveBeenCalledTimes(1);
+  expect(callback).toHaveBeenCalledWith(arg);
+  callback.mockClear();
+
+  jest.spyOn(result.current, 'flush');
+
+  unmount();
+
+  expect(result.current.flush).toHaveBeenCalled();
+
+  jest.advanceTimersByTime(1);
+
+  expect(callback).toHaveBeenCalledTimes(1);
+  expect(callback).toHaveBeenCalledWith(arg2);
+});

packages/react-hooks/src/useThrottledCallback.ts

@@ -0,0 +1,52 @@
+import { useEffect, useMemo, useRef } from 'react';
+import type { DebouncedFunc, ThrottleSettings } from 'lodash';
+import throttle from 'lodash.throttle';
+
+/**
+ * Wraps a given callback in a cancelable, throttled function. The throttled
+ * callback is stable and will never change. It will be automatically cancelled
+ * on unmount, unless the `flushOnUnmount` option is passed in, then it will be flushed on unmount.
+ * At the time the throttled function is called, it will call the latest callback that has been passed in.
+ * @param callback callback function to call with throttling
+ * @param throttleMs throttle milliseconds
+ * @param initialOptions lodash throttle options. Will not react to changes to this param
+ * @returns a cancelable, throttled function
+ */
+export function useThrottledCallback<TArgs extends unknown[], TResult>(
+  callback: (...args: TArgs) => TResult,
+  throttleMs: number,
+  initialOptions?: ThrottleSettings & { flushOnUnmount?: boolean }
+): DebouncedFunc<(...args: TArgs) => TResult> {
+  const options = useRef(initialOptions);
+
+  // Use a ref for the callback
+  // We want to keep a stable callback so the flush/cancel works as expected
+  // So we keep a ref to the current callback, then we have a throttled callback that will just call this
+  const callbackRef = useRef(callback);
+  callbackRef.current = callback;
+
+  const throttledCallback = useMemo(
+    () =>
+      throttle(
+        (...args: TArgs) => callbackRef.current(...args),
+        throttleMs,
+        options.current
+      ),
+    [throttleMs]
+  );
+
+  useEffect(
+    () => () => {
+      if (options.current?.flushOnUnmount ?? false) {
+        throttledCallback.flush();
+      } else {
+        throttledCallback.cancel();
+      }
+    },
+    [throttledCallback]
+  );
+
+  return throttledCallback;
+}
+
+export default useThrottledCallback;