From d042c3cb83d7c648b3a2256bb2f9abbe38308a00 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Wed, 1 May 2024 10:54:27 -0400 Subject: [PATCH 1/8] fix: Typing in notebooks is laggy - 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 --- packages/dashboard/src/DashboardLayout.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/dashboard/src/DashboardLayout.tsx b/packages/dashboard/src/DashboardLayout.tsx index 2073800889..77489cd0f4 100644 --- a/packages/dashboard/src/DashboardLayout.tsx +++ b/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 { useDebouncedCallback, usePrevious } 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_DEBOUNCE_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; @@ -228,6 +230,11 @@ export function DashboardLayout({ } }, [dehydrateComponent, isItemDragging, lastConfig, layout, onLayoutChange]); + const debouncedHandleLayoutStateChanged = useDebouncedCallback( + handleLayoutStateChanged, + STATE_CHANGE_DEBOUNCE_MS + ); + const handleLayoutItemPickedUp = useCallback( (component: Container) => { const componentId = LayoutUtils.getIdFromContainer(component); @@ -269,7 +276,7 @@ export function DashboardLayout({ setLayoutChildren(layout.getReactChildren()); }, [layout]); - useListener(layout, 'stateChanged', handleLayoutStateChanged); + useListener(layout, 'stateChanged', debouncedHandleLayoutStateChanged); useListener(layout, 'itemPickedUp', handleLayoutItemPickedUp); useListener(layout, 'itemDropped', handleLayoutItemDropped); useListener(layout, 'componentCreated', handleComponentCreated); From 2c572239a289523bfeda9d94f66ddf2646ed6db4 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Wed, 1 May 2024 11:26:22 -0400 Subject: [PATCH 2/8] Change to a throttled instead of debounced - Otherwise it's possible for a dashboard to never save if it is constantly updating --- packages/dashboard/src/DashboardLayout.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/dashboard/src/DashboardLayout.tsx b/packages/dashboard/src/DashboardLayout.tsx index 77489cd0f4..fe4f7b2824 100644 --- a/packages/dashboard/src/DashboardLayout.tsx +++ b/packages/dashboard/src/DashboardLayout.tsx @@ -14,8 +14,9 @@ import type { ReactComponentConfig, } from '@deephaven/golden-layout'; import Log from '@deephaven/log'; -import { useDebouncedCallback, usePrevious } from '@deephaven/react-hooks'; +import { usePrevious } from '@deephaven/react-hooks'; import { RootState } from '@deephaven/redux'; +import throttle from 'lodash.throttle'; import { useDispatch, useSelector } from 'react-redux'; import PanelManager, { ClosedPanels } from './PanelManager'; import PanelErrorBoundary from './PanelErrorBoundary'; @@ -230,9 +231,9 @@ export function DashboardLayout({ } }, [dehydrateComponent, isItemDragging, lastConfig, layout, onLayoutChange]); - const debouncedHandleLayoutStateChanged = useDebouncedCallback( - handleLayoutStateChanged, - STATE_CHANGE_DEBOUNCE_MS + const throttledHandleLayoutStateChanged = useMemo( + () => throttle(handleLayoutStateChanged, STATE_CHANGE_DEBOUNCE_MS), + [handleLayoutStateChanged] ); const handleLayoutItemPickedUp = useCallback( @@ -276,7 +277,7 @@ export function DashboardLayout({ setLayoutChildren(layout.getReactChildren()); }, [layout]); - useListener(layout, 'stateChanged', debouncedHandleLayoutStateChanged); + useListener(layout, 'stateChanged', throttledHandleLayoutStateChanged); useListener(layout, 'itemPickedUp', handleLayoutItemPickedUp); useListener(layout, 'itemDropped', handleLayoutItemDropped); useListener(layout, 'componentCreated', handleComponentCreated); From c318d922738eb993c318a5cae893a32021ba46ef Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Wed, 1 May 2024 21:28:05 -0400 Subject: [PATCH 3/8] Add useThrottledCallback hook --- packages/react-hooks/package.json | 1 + .../src/useThrottledCallback.test.ts | 86 +++++++++++++++++++ .../react-hooks/src/useThrottledCallback.ts | 27 ++++++ 3 files changed, 114 insertions(+) create mode 100644 packages/react-hooks/src/useThrottledCallback.test.ts create mode 100644 packages/react-hooks/src/useThrottledCallback.ts diff --git a/packages/react-hooks/package.json b/packages/react-hooks/package.json index 614def1b31..8565384009 100644 --- a/packages/react-hooks/package.json +++ b/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": { diff --git a/packages/react-hooks/src/useThrottledCallback.test.ts b/packages/react-hooks/src/useThrottledCallback.test.ts new file mode 100644 index 0000000000..5995a462c2 --- /dev/null +++ b/packages/react-hooks/src/useThrottledCallback.test.ts @@ -0,0 +1,86 @@ +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(); +}); + +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 cancel throttle if callback reference 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); + + rerender(jest.fn()); + + jest.advanceTimersByTime(5); + + expect(callback).not.toHaveBeenCalled(); +}); diff --git a/packages/react-hooks/src/useThrottledCallback.ts b/packages/react-hooks/src/useThrottledCallback.ts new file mode 100644 index 0000000000..1e797eb086 --- /dev/null +++ b/packages/react-hooks/src/useThrottledCallback.ts @@ -0,0 +1,27 @@ +import { useEffect, useMemo } from 'react'; +import type { DebouncedFunc } from 'lodash'; +import throttle from 'lodash.throttle'; + +/** + * Wraps a given callback in a cancelable, throttled function. The throttled + * callback is automatically cancelled if the callback reference changes or the + * component unmounts. + * @param callback callback function to throttle + * @param throttleMs throttle milliseconds + * @returns a cancelable, throttled function + */ +export function useThrottledCallback( + callback: (...args: TArgs) => TResult, + throttleMs: number +): DebouncedFunc<(...args: TArgs) => TResult> { + const throttledCallback = useMemo( + () => throttle(callback, throttleMs), + [callback, throttleMs] + ); + + useEffect(() => () => throttledCallback.cancel(), [throttledCallback]); + + return throttledCallback; +} + +export default useThrottledCallback; From af191031059d490ea96d5dbafcea404102b7063e Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Wed, 1 May 2024 22:05:22 -0400 Subject: [PATCH 4/8] Clean up useThrottledCallback --- packages/dashboard/src/DashboardLayout.tsx | 66 ++++++++++++---------- packages/react-hooks/src/index.ts | 1 + 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/packages/dashboard/src/DashboardLayout.tsx b/packages/dashboard/src/DashboardLayout.tsx index fe4f7b2824..00b92be81f 100644 --- a/packages/dashboard/src/DashboardLayout.tsx +++ b/packages/dashboard/src/DashboardLayout.tsx @@ -14,9 +14,8 @@ 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 throttle from 'lodash.throttle'; import { useDispatch, useSelector } from 'react-redux'; import PanelManager, { ClosedPanels } from './PanelManager'; import PanelErrorBoundary from './PanelErrorBoundary'; @@ -198,6 +197,34 @@ export function DashboardLayout({ ] ); + // eslint-disable-next-line react-hooks/exhaustive-deps + const processDehydratedLayoutConfig = useCallback( + dehydratedLayoutConfig => { + 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()); + } + }, + [lastConfig, layout, onLayoutChange] + ); + + // Throttle the calls so that we don't flood comparing these layouts + const throttledProcessDehydratedLayoutConfig = useThrottledCallback( + processDehydratedLayoutConfig, + STATE_CHANGE_DEBOUNCE_MS + ); + 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 @@ -209,32 +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]); - - const throttledHandleLayoutStateChanged = useMemo( - () => throttle(handleLayoutStateChanged, STATE_CHANGE_DEBOUNCE_MS), - [handleLayoutStateChanged] - ); + throttledProcessDehydratedLayoutConfig(dehydratedLayoutConfig); + }, [ + dehydrateComponent, + isItemDragging, + layout, + throttledProcessDehydratedLayoutConfig, + ]); const handleLayoutItemPickedUp = useCallback( (component: Container) => { @@ -277,7 +285,7 @@ export function DashboardLayout({ setLayoutChildren(layout.getReactChildren()); }, [layout]); - useListener(layout, 'stateChanged', throttledHandleLayoutStateChanged); + useListener(layout, 'stateChanged', handleLayoutStateChanged); useListener(layout, 'itemPickedUp', handleLayoutItemPickedUp); useListener(layout, 'itemDropped', handleLayoutItemDropped); useListener(layout, 'componentCreated', handleComponentCreated); diff --git a/packages/react-hooks/src/index.ts b/packages/react-hooks/src/index.ts index a3d98b2498..83145b70db 100644 --- a/packages/react-hooks/src/index.ts +++ b/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'; From 6e303eba3f2c99dc34b5d73976958da141644878 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Wed, 1 May 2024 22:11:37 -0400 Subject: [PATCH 5/8] Flush the throttled callback --- packages/dashboard/src/DashboardLayout.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/dashboard/src/DashboardLayout.tsx b/packages/dashboard/src/DashboardLayout.tsx index 00b92be81f..68b46601d7 100644 --- a/packages/dashboard/src/DashboardLayout.tsx +++ b/packages/dashboard/src/DashboardLayout.tsx @@ -14,9 +14,10 @@ import type { ReactComponentConfig, } from '@deephaven/golden-layout'; import Log from '@deephaven/log'; -import { usePrevious, useThrottledCallback } from '@deephaven/react-hooks'; +import { usePrevious } from '@deephaven/react-hooks'; import { RootState } from '@deephaven/redux'; import { useDispatch, useSelector } from 'react-redux'; +import throttle from 'lodash.throttle'; import PanelManager, { ClosedPanels } from './PanelManager'; import PanelErrorBoundary from './PanelErrorBoundary'; import LayoutUtils from './layout/LayoutUtils'; @@ -220,9 +221,14 @@ export function DashboardLayout({ ); // Throttle the calls so that we don't flood comparing these layouts - const throttledProcessDehydratedLayoutConfig = useThrottledCallback( - processDehydratedLayoutConfig, - STATE_CHANGE_DEBOUNCE_MS + const throttledProcessDehydratedLayoutConfig = useMemo( + () => throttle(processDehydratedLayoutConfig, STATE_CHANGE_DEBOUNCE_MS), + [processDehydratedLayoutConfig] + ); + + useEffect( + () => () => throttledProcessDehydratedLayoutConfig.flush(), + [throttledProcessDehydratedLayoutConfig] ); const handleLayoutStateChanged = useCallback(() => { From f7e61d671fff92410d220b387359d4e93e946bd6 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Wed, 1 May 2024 22:22:49 -0400 Subject: [PATCH 6/8] Clean up useThrottledCallback and tests - Need to use refs so that the callbacks don't change each time, and the flush/cancel are stable - Fun! --- packages/dashboard/src/DashboardLayout.tsx | 10 +++--- .../src/useThrottledCallback.test.ts | 35 ++++++++++++++++-- .../react-hooks/src/useThrottledCallback.ts | 36 +++++++++++++++---- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/packages/dashboard/src/DashboardLayout.tsx b/packages/dashboard/src/DashboardLayout.tsx index 68b46601d7..b6fb773eab 100644 --- a/packages/dashboard/src/DashboardLayout.tsx +++ b/packages/dashboard/src/DashboardLayout.tsx @@ -14,10 +14,9 @@ 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 throttle from 'lodash.throttle'; import PanelManager, { ClosedPanels } from './PanelManager'; import PanelErrorBoundary from './PanelErrorBoundary'; import LayoutUtils from './layout/LayoutUtils'; @@ -221,9 +220,10 @@ export function DashboardLayout({ ); // Throttle the calls so that we don't flood comparing these layouts - const throttledProcessDehydratedLayoutConfig = useMemo( - () => throttle(processDehydratedLayoutConfig, STATE_CHANGE_DEBOUNCE_MS), - [processDehydratedLayoutConfig] + const throttledProcessDehydratedLayoutConfig = useThrottledCallback( + processDehydratedLayoutConfig, + STATE_CHANGE_DEBOUNCE_MS, + { flushOnUnmount: true } ); useEffect( diff --git a/packages/react-hooks/src/useThrottledCallback.test.ts b/packages/react-hooks/src/useThrottledCallback.test.ts index 5995a462c2..d0ef983c8c 100644 --- a/packages/react-hooks/src/useThrottledCallback.test.ts +++ b/packages/react-hooks/src/useThrottledCallback.test.ts @@ -60,7 +60,7 @@ it('should cancel throttle if component unmounts', () => { expect(callback).not.toHaveBeenCalled(); }); -it('should cancel throttle if callback reference changes', () => { +it('should call the updated callback if the ref changes', () => { const { rerender, result } = renderHook( fn => useThrottledCallback(fn, throttleMs), { @@ -78,9 +78,38 @@ it('should cancel throttle if callback reference changes', () => { jest.advanceTimersByTime(throttleMs - 1); - rerender(jest.fn()); + const newCallback = jest.fn(); + rerender(newCallback); - jest.advanceTimersByTime(5); + 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); }); diff --git a/packages/react-hooks/src/useThrottledCallback.ts b/packages/react-hooks/src/useThrottledCallback.ts index 1e797eb086..3a714f759c 100644 --- a/packages/react-hooks/src/useThrottledCallback.ts +++ b/packages/react-hooks/src/useThrottledCallback.ts @@ -1,5 +1,5 @@ -import { useEffect, useMemo } from 'react'; -import type { DebouncedFunc } from 'lodash'; +import { useEffect, useMemo, useRef } from 'react'; +import type { DebouncedFunc, ThrottleSettings } from 'lodash'; import throttle from 'lodash.throttle'; /** @@ -8,18 +8,42 @@ import throttle from 'lodash.throttle'; * component unmounts. * @param callback callback function to throttle * @param throttleMs throttle milliseconds + * @param options lodash throttle options. Will not react to changes to this param * @returns a cancelable, throttled function */ export function useThrottledCallback( callback: (...args: TArgs) => TResult, - throttleMs: number + 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(callback, throttleMs), - [callback, throttleMs] + () => + throttle( + (...args: TArgs) => callbackRef.current(...args), + throttleMs, + options.current + ), + [throttleMs] ); - useEffect(() => () => throttledCallback.cancel(), [throttledCallback]); + useEffect( + () => () => { + if (options.current?.flushOnUnmount ?? false) { + throttledCallback.flush(); + } else { + throttledCallback.cancel(); + } + }, + [throttledCallback] + ); return throttledCallback; } From dfd35cac9a45e511570723f7090d91976c0a8bd2 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 2 May 2024 08:12:47 -0400 Subject: [PATCH 7/8] Clean up from review, update comments --- packages/dashboard/src/DashboardLayout.tsx | 12 +++--------- packages/react-hooks/src/useThrottledCallback.ts | 9 +++++---- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/dashboard/src/DashboardLayout.tsx b/packages/dashboard/src/DashboardLayout.tsx index b6fb773eab..ef0c5b6c53 100644 --- a/packages/dashboard/src/DashboardLayout.tsx +++ b/packages/dashboard/src/DashboardLayout.tsx @@ -197,9 +197,9 @@ export function DashboardLayout({ ] ); - // eslint-disable-next-line react-hooks/exhaustive-deps - const processDehydratedLayoutConfig = useCallback( - dehydratedLayoutConfig => { + // 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); @@ -216,12 +216,6 @@ export function DashboardLayout({ setLayoutChildren(layout.getReactChildren()); } }, - [lastConfig, layout, onLayoutChange] - ); - - // Throttle the calls so that we don't flood comparing these layouts - const throttledProcessDehydratedLayoutConfig = useThrottledCallback( - processDehydratedLayoutConfig, STATE_CHANGE_DEBOUNCE_MS, { flushOnUnmount: true } ); diff --git a/packages/react-hooks/src/useThrottledCallback.ts b/packages/react-hooks/src/useThrottledCallback.ts index 3a714f759c..3c4e571e6e 100644 --- a/packages/react-hooks/src/useThrottledCallback.ts +++ b/packages/react-hooks/src/useThrottledCallback.ts @@ -4,11 +4,12 @@ import throttle from 'lodash.throttle'; /** * Wraps a given callback in a cancelable, throttled function. The throttled - * callback is automatically cancelled if the callback reference changes or the - * component unmounts. - * @param callback callback function to throttle + * 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 options lodash throttle options. Will not react to changes to this param + * @param initialOptions lodash throttle options. Will not react to changes to this param * @returns a cancelable, throttled function */ export function useThrottledCallback( From 1739edb5affd9c6a5c57afabcabf3c52fea7a01f Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Thu, 2 May 2024 08:16:14 -0400 Subject: [PATCH 8/8] Address review comments --- packages/dashboard/src/DashboardLayout.tsx | 4 ++-- packages/react-hooks/src/useDebouncedCallback.test.ts | 4 ++++ packages/react-hooks/src/useThrottledCallback.test.ts | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/dashboard/src/DashboardLayout.tsx b/packages/dashboard/src/DashboardLayout.tsx index ef0c5b6c53..4694e39742 100644 --- a/packages/dashboard/src/DashboardLayout.tsx +++ b/packages/dashboard/src/DashboardLayout.tsx @@ -46,7 +46,7 @@ const DEFAULT_LAYOUT_CONFIG: DashboardLayoutConfig = []; const DEFAULT_CALLBACK = (): void => undefined; -const STATE_CHANGE_DEBOUNCE_MS = 1000; +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; @@ -216,7 +216,7 @@ export function DashboardLayout({ setLayoutChildren(layout.getReactChildren()); } }, - STATE_CHANGE_DEBOUNCE_MS, + STATE_CHANGE_THROTTLE_MS, { flushOnUnmount: true } ); diff --git a/packages/react-hooks/src/useDebouncedCallback.test.ts b/packages/react-hooks/src/useDebouncedCallback.test.ts index 9618f13164..d55e849592 100644 --- a/packages/react-hooks/src/useDebouncedCallback.test.ts +++ b/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) diff --git a/packages/react-hooks/src/useThrottledCallback.test.ts b/packages/react-hooks/src/useThrottledCallback.test.ts index d0ef983c8c..1feac36f02 100644 --- a/packages/react-hooks/src/useThrottledCallback.test.ts +++ b/packages/react-hooks/src/useThrottledCallback.test.ts @@ -11,6 +11,10 @@ beforeEach(() => { jest.useFakeTimers(); }); +afterEach(() => { + jest.useRealTimers(); +}); + it('should throttle a given callback', () => { const { result } = renderHook(() => useThrottledCallback(callback, throttleMs)