Skip to content

Commit f47efc6

Browse files
authored
fix(site): speed up state syncs and validate input for debounce hook logic (#18877)
No issue to link – I'm basically pushing some updates upstream from the version of the hook I copied over for the Registry website. ## Changes made - Updated debounce functions to have input validation for timeouts - Updated `useDebouncedValue` to flush state syncs immediately if timeout value is `0` - Updated tests to reflect changes - Cleaned up some comments and parameter names to make things more clear
1 parent 6746e16 commit f47efc6

File tree

2 files changed

+101
-27
lines changed

2 files changed

+101
-27
lines changed

site/src/hooks/debounce.test.ts

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ afterAll(() => {
1111
jest.clearAllMocks();
1212
});
1313

14-
describe(`${useDebouncedValue.name}`, () => {
15-
function renderDebouncedValue<T = unknown>(value: T, time: number) {
14+
describe(useDebouncedValue.name, () => {
15+
function renderDebouncedValue<T>(value: T, time: number) {
1616
return renderHook(
1717
({ value, time }: { value: T; time: number }) => {
1818
return useDebouncedValue(value, time);
@@ -23,6 +23,25 @@ describe(`${useDebouncedValue.name}`, () => {
2323
);
2424
}
2525

26+
it("Should throw for non-nonnegative integer timeouts", () => {
27+
const invalidInputs: readonly number[] = [
28+
Number.NaN,
29+
Number.NEGATIVE_INFINITY,
30+
Number.POSITIVE_INFINITY,
31+
Math.PI,
32+
-42,
33+
];
34+
35+
const dummyValue = false;
36+
for (const input of invalidInputs) {
37+
expect(() => {
38+
renderDebouncedValue(dummyValue, input);
39+
}).toThrow(
40+
`Invalid value ${input} for debounceTimeoutMs. Value must be an integer greater than or equal to zero.`,
41+
);
42+
}
43+
});
44+
2645
it("Should immediately return out the exact same value (by reference) on mount", () => {
2746
const value = {};
2847
const { result } = renderDebouncedValue(value, 2000);
@@ -58,6 +77,24 @@ describe(`${useDebouncedValue.name}`, () => {
5877
await jest.runAllTimersAsync();
5978
await waitFor(() => expect(result.current).toEqual(true));
6079
});
80+
81+
// Very important that we not do any async logic for this test
82+
it("Should immediately resync without any render/event loop delays if timeout is zero", () => {
83+
const initialValue = false;
84+
const time = 5000;
85+
86+
const { result, rerender } = renderDebouncedValue(initialValue, time);
87+
expect(result.current).toEqual(false);
88+
89+
// Just to be on the safe side, re-render once with the old timeout to
90+
// verify that nothing has been flushed yet
91+
rerender({ value: !initialValue, time });
92+
expect(result.current).toEqual(false);
93+
94+
// Then do the real re-render once we know the coast is clear
95+
rerender({ value: !initialValue, time: 0 });
96+
expect(result.current).toBe(true);
97+
});
6198
});
6299

63100
describe(`${useDebouncedFunction.name}`, () => {
@@ -75,6 +112,27 @@ describe(`${useDebouncedFunction.name}`, () => {
75112
);
76113
}
77114

115+
describe("input validation", () => {
116+
it("Should throw for non-nonnegative integer timeouts", () => {
117+
const invalidInputs: readonly number[] = [
118+
Number.NaN,
119+
Number.NEGATIVE_INFINITY,
120+
Number.POSITIVE_INFINITY,
121+
Math.PI,
122+
-42,
123+
];
124+
125+
const dummyFunction = jest.fn();
126+
for (const input of invalidInputs) {
127+
expect(() => {
128+
renderDebouncedFunction(dummyFunction, input);
129+
}).toThrow(
130+
`Invalid value ${input} for debounceTimeoutMs. Value must be an integer greater than or equal to zero.`,
131+
);
132+
}
133+
});
134+
});
135+
78136
describe("hook", () => {
79137
it("Should provide stable function references across re-renders", () => {
80138
const time = 5000;

site/src/hooks/debounce.ts

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,15 @@
22
* @file Defines hooks for created debounced versions of functions and arbitrary
33
* values.
44
*
5-
* It is not safe to call a general-purpose debounce utility inside a React
6-
* render. It will work on the initial render, but the memory reference for the
7-
* value will change on re-renders. Most debounce functions create a "stateful"
8-
* version of a function by leveraging closure; but by calling it repeatedly,
9-
* you create multiple "pockets" of state, rather than a centralized one.
10-
*
11-
* Debounce utilities can make sense if they can be called directly outside the
12-
* component or in a useEffect call, though.
5+
* It is not safe to call most general-purpose debounce utility functions inside
6+
* a React render. This is because the state for handling the debounce logic
7+
* lives in the utility instead of React. If you call a general-purpose debounce
8+
* function inline, that will create a new stateful function on every render,
9+
* which has a lot of risks around conflicting/contradictory state.
1310
*/
1411
import { useCallback, useEffect, useRef, useState } from "react";
1512

16-
type useDebouncedFunctionReturn<Args extends unknown[]> = Readonly<{
13+
type UseDebouncedFunctionReturn<Args extends unknown[]> = Readonly<{
1714
debounced: (...args: Args) => void;
1815

1916
// Mainly here to make interfacing with useEffect cleanup functions easier
@@ -34,26 +31,32 @@ type useDebouncedFunctionReturn<Args extends unknown[]> = Readonly<{
3431
*/
3532
export function useDebouncedFunction<
3633
// Parameterizing on the args instead of the whole callback function type to
37-
// avoid type contra-variance issues
34+
// avoid type contravariance issues
3835
Args extends unknown[] = unknown[],
3936
>(
4037
callback: (...args: Args) => void | Promise<void>,
41-
debounceTimeMs: number,
42-
): useDebouncedFunctionReturn<Args> {
43-
const timeoutIdRef = useRef<number | null>(null);
38+
debounceTimeoutMs: number,
39+
): UseDebouncedFunctionReturn<Args> {
40+
if (!Number.isInteger(debounceTimeoutMs) || debounceTimeoutMs < 0) {
41+
throw new Error(
42+
`Invalid value ${debounceTimeoutMs} for debounceTimeoutMs. Value must be an integer greater than or equal to zero.`,
43+
);
44+
}
45+
46+
const timeoutIdRef = useRef<number | undefined>(undefined);
4447
const cancelDebounce = useCallback(() => {
45-
if (timeoutIdRef.current !== null) {
48+
if (timeoutIdRef.current !== undefined) {
4649
window.clearTimeout(timeoutIdRef.current);
4750
}
4851

49-
timeoutIdRef.current = null;
52+
timeoutIdRef.current = undefined;
5053
}, []);
5154

52-
const debounceTimeRef = useRef(debounceTimeMs);
55+
const debounceTimeRef = useRef(debounceTimeoutMs);
5356
useEffect(() => {
5457
cancelDebounce();
55-
debounceTimeRef.current = debounceTimeMs;
56-
}, [cancelDebounce, debounceTimeMs]);
58+
debounceTimeRef.current = debounceTimeoutMs;
59+
}, [cancelDebounce, debounceTimeoutMs]);
5760

5861
const callbackRef = useRef(callback);
5962
useEffect(() => {
@@ -81,19 +84,32 @@ export function useDebouncedFunction<
8184
/**
8285
* Takes any value, and returns out a debounced version of it.
8386
*/
84-
export function useDebouncedValue<T = unknown>(
85-
value: T,
86-
debounceTimeMs: number,
87-
): T {
87+
export function useDebouncedValue<T>(value: T, debounceTimeoutMs: number): T {
88+
if (!Number.isInteger(debounceTimeoutMs) || debounceTimeoutMs < 0) {
89+
throw new Error(
90+
`Invalid value ${debounceTimeoutMs} for debounceTimeoutMs. Value must be an integer greater than or equal to zero.`,
91+
);
92+
}
93+
8894
const [debouncedValue, setDebouncedValue] = useState(value);
8995

96+
// If the debounce timeout is ever zero, synchronously flush any state syncs.
97+
// Doing this mid-render instead of in useEffect means that we drastically cut
98+
// down on needless re-renders, and we also avoid going through the event loop
99+
// to do a state sync that is *intended* to happen immediately
100+
if (value !== debouncedValue && debounceTimeoutMs === 0) {
101+
setDebouncedValue(value);
102+
}
90103
useEffect(() => {
104+
if (debounceTimeoutMs === 0) {
105+
return;
106+
}
107+
91108
const timeoutId = window.setTimeout(() => {
92109
setDebouncedValue(value);
93-
}, debounceTimeMs);
94-
110+
}, debounceTimeoutMs);
95111
return () => window.clearTimeout(timeoutId);
96-
}, [value, debounceTimeMs]);
112+
}, [value, debounceTimeoutMs]);
97113

98114
return debouncedValue;
99115
}

0 commit comments

Comments
 (0)