-
Notifications
You must be signed in to change notification settings - Fork 31
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
6 changed files
with
215 additions
and
22 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; |