Skip to content

Commit 2600c96

Browse files
authored
Fix buttongroup infinite update loop (#5010)
* Fix buttongroup infinite update loop
1 parent 3b92155 commit 2600c96

File tree

1 file changed

+20
-27
lines changed

1 file changed

+20
-27
lines changed

packages/@react-aria/utils/src/useValueEffect.ts

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {Dispatch, useCallback, useRef, useState} from 'react';
14-
import {useLayoutEffect} from './';
13+
import {Dispatch, useRef, useState} from 'react';
14+
import {useEffectEvent, useLayoutEffect} from './';
1515

1616
type SetValueAction<S> = (prev: S) => Generator<any, void, unknown>;
1717

@@ -21,48 +21,41 @@ type SetValueAction<S> = (prev: S) => Generator<any, void, unknown>;
2121
// written linearly.
2222
export function useValueEffect<S>(defaultValue: S | (() => S)): [S, Dispatch<SetValueAction<S>>] {
2323
let [value, setValue] = useState(defaultValue);
24-
let valueRef = useRef(value);
2524
let effect = useRef(null);
2625

27-
// Must be stable so that `queue` is stable.
28-
let nextIter = useCallback(() => {
26+
// Store the function in a ref so we can always access the current version
27+
// which has the proper `value` in scope.
28+
let nextRef = useEffectEvent(() => {
2929
// Run the generator to the next yield.
3030
let newValue = effect.current.next();
31-
while (!newValue.done && valueRef.current === newValue.value) {
32-
// If the value is the same as the current value,
33-
// then continue to the next yield. Otherwise,
34-
// set the value in state and wait for the next layout effect.
35-
newValue = effect.current.next();
36-
}
31+
3732
// If the generator is done, reset the effect.
3833
if (newValue.done) {
3934
effect.current = null;
4035
return;
4136
}
4237

43-
// Always update valueRef when setting the state.
44-
// This is needed because the function is not regenerated with the new state value since
45-
// they must be stable across renders. Instead, it gets carried in the ref, but the setState
46-
// is also needed in order to cause a rerender.
47-
setValue(newValue.value);
48-
valueRef.current = newValue.value;
49-
// this list of dependencies is stable, setState and refs never change after first render.
50-
}, [setValue, valueRef, effect]);
38+
// If the value is the same as the current value,
39+
// then continue to the next yield. Otherwise,
40+
// set the value in state and wait for the next layout effect.
41+
if (value === newValue.value) {
42+
nextRef();
43+
} else {
44+
setValue(newValue.value);
45+
}
46+
});
5147

5248
useLayoutEffect(() => {
5349
// If there is an effect currently running, continue to the next yield.
5450
if (effect.current) {
55-
nextIter();
51+
nextRef();
5652
}
5753
});
5854

59-
// queue must be a stable function, much like setState.
60-
let queue = useCallback(fn => {
61-
effect.current = fn(valueRef.current);
62-
nextIter();
63-
// this list of dependencies is stable, setState and refs never change after first render.
64-
// in addition, nextIter is stable as outlined above
65-
}, [nextIter, effect, valueRef]);
55+
let queue = useEffectEvent(fn => {
56+
effect.current = fn(value);
57+
nextRef();
58+
});
6659

6760
return [value, queue];
6861
}

0 commit comments

Comments
 (0)