Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commitf47efc6

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 theversion 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 iftimeout value is `0`- Updated tests to reflect changes- Cleaned up some comments and parameter names to make things more clear
1 parent6746e16 commitf47efc6

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-
functionrenderDebouncedValue<T=unknown>(value:T,time:number){
14+
describe(useDebouncedValue.name,()=>{
15+
functionrenderDebouncedValue<T>(value:T,time:number){
1616
returnrenderHook(
1717
({ value, time}:{value:T;time:number})=>{
1818
returnuseDebouncedValue(value,time);
@@ -23,6 +23,25 @@ describe(`${useDebouncedValue.name}`, () => {
2323
);
2424
}
2525

26+
it("Should throw for non-nonnegative integer timeouts",()=>{
27+
constinvalidInputs:readonlynumber[]=[
28+
Number.NaN,
29+
Number.NEGATIVE_INFINITY,
30+
Number.POSITIVE_INFINITY,
31+
Math.PI,
32+
-42,
33+
];
34+
35+
constdummyValue=false;
36+
for(constinputofinvalidInputs){
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
constvalue={};
2847
const{ result}=renderDebouncedValue(value,2000);
@@ -58,6 +77,24 @@ describe(`${useDebouncedValue.name}`, () => {
5877
awaitjest.runAllTimersAsync();
5978
awaitwaitFor(()=>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+
constinitialValue=false;
84+
consttime=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+
constinvalidInputs:readonlynumber[]=[
118+
Number.NaN,
119+
Number.NEGATIVE_INFINITY,
120+
Number.POSITIVE_INFINITY,
121+
Math.PI,
122+
-42,
123+
];
124+
125+
constdummyFunction=jest.fn();
126+
for(constinputofinvalidInputs){
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
consttime=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-
typeuseDebouncedFunctionReturn<Argsextendsunknown[]>=Readonly<{
13+
typeUseDebouncedFunctionReturn<Argsextendsunknown[]>=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
exportfunctionuseDebouncedFunction<
3633
// Parameterizing on the args instead of the whole callback function type to
37-
// avoid typecontra-variance issues
34+
// avoid typecontravariance issues
3835
Argsextendsunknown[]=unknown[],
3936
>(
4037
callback:(...args:Args)=>void|Promise<void>,
41-
debounceTimeMs:number,
42-
):useDebouncedFunctionReturn<Args>{
43-
consttimeoutIdRef=useRef<number|null>(null);
38+
debounceTimeoutMs:number,
39+
):UseDebouncedFunctionReturn<Args>{
40+
if(!Number.isInteger(debounceTimeoutMs)||debounceTimeoutMs<0){
41+
thrownewError(
42+
`Invalid value${debounceTimeoutMs} for debounceTimeoutMs. Value must be an integer greater than or equal to zero.`,
43+
);
44+
}
45+
46+
consttimeoutIdRef=useRef<number|undefined>(undefined);
4447
constcancelDebounce=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-
constdebounceTimeRef=useRef(debounceTimeMs);
55+
constdebounceTimeRef=useRef(debounceTimeoutMs);
5356
useEffect(()=>{
5457
cancelDebounce();
55-
debounceTimeRef.current=debounceTimeMs;
56-
},[cancelDebounce,debounceTimeMs]);
58+
debounceTimeRef.current=debounceTimeoutMs;
59+
},[cancelDebounce,debounceTimeoutMs]);
5760

5861
constcallbackRef=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-
exportfunctionuseDebouncedValue<T=unknown>(
85-
value:T,
86-
debounceTimeMs:number,
87-
):T{
87+
exportfunctionuseDebouncedValue<T>(value:T,debounceTimeoutMs:number):T{
88+
if(!Number.isInteger(debounceTimeoutMs)||debounceTimeoutMs<0){
89+
thrownewError(
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
consttimeoutId=window.setTimeout(()=>{
92109
setDebouncedValue(value);
93-
},debounceTimeMs);
94-
110+
},debounceTimeoutMs);
95111
return()=>window.clearTimeout(timeoutId);
96-
},[value,debounceTimeMs]);
112+
},[value,debounceTimeoutMs]);
97113

98114
returndebouncedValue;
99115
}

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp