Skip to content

Commit 2164c02

Browse files
committed
fix(#2043): handle unnecessary invocation of index side effect (#2073)(inspired by @IslamRustamov)
1 parent d41a374 commit 2164c02

File tree

2 files changed

+59
-66
lines changed

2 files changed

+59
-66
lines changed

src/components/bottomSheet/BottomSheet.tsx

+53-61
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
usePropsValidator,
4848
useReactiveSharedValue,
4949
useScrollable,
50+
useStableCallback,
5051
} from '../../hooks';
5152
import type { BottomSheetMethods } from '../../types';
5253
import {
@@ -1022,68 +1023,56 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
10221023
//#endregion
10231024

10241025
//#region public methods
1025-
// biome-ignore lint/correctness/useExhaustiveDependencies(BottomSheet.name): used for debug only
1026-
const handleSnapToIndex = useCallback(
1027-
function handleSnapToIndex(
1028-
index: number,
1029-
animationConfigs?: WithSpringConfig | WithTimingConfig
1030-
) {
1031-
const snapPoints = animatedSnapPoints.value;
1032-
invariant(
1033-
index >= -1 && index <= snapPoints.length - 1,
1034-
`'index' was provided but out of the provided snap points range! expected value to be between -1, ${
1035-
snapPoints.length - 1
1036-
}`
1037-
);
1038-
if (__DEV__) {
1039-
print({
1040-
component: BottomSheet.name,
1041-
method: handleSnapToIndex.name,
1042-
params: {
1043-
index,
1044-
},
1045-
});
1046-
}
1026+
const handleSnapToIndex = useStableCallback(function handleSnapToIndex(
1027+
index: number,
1028+
animationConfigs?: WithSpringConfig | WithTimingConfig
1029+
) {
1030+
const snapPoints = animatedSnapPoints.value;
1031+
invariant(
1032+
index >= -1 && index <= snapPoints.length - 1,
1033+
`'index' was provided but out of the provided snap points range! expected value to be between -1, ${
1034+
snapPoints.length - 1
1035+
}`
1036+
);
1037+
if (__DEV__) {
1038+
print({
1039+
component: BottomSheet.name,
1040+
method: handleSnapToIndex.name,
1041+
params: {
1042+
index,
1043+
},
1044+
});
1045+
}
10471046

1048-
const nextPosition = snapPoints[index];
1047+
const nextPosition = snapPoints[index];
10491048

1050-
/**
1051-
* exit method if :
1052-
* - layout is not calculated.
1053-
* - already animating to next position.
1054-
* - sheet is forced closing.
1055-
*/
1056-
if (
1057-
!isLayoutCalculated.value ||
1058-
index === animatedNextPositionIndex.value ||
1059-
nextPosition === animatedNextPosition.value ||
1060-
isForcedClosing.value
1061-
) {
1062-
return;
1063-
}
1049+
/**
1050+
* exit method if :
1051+
* - layout is not calculated.
1052+
* - already animating to next position.
1053+
* - sheet is forced closing.
1054+
*/
1055+
if (
1056+
!isLayoutCalculated.value ||
1057+
index === animatedNextPositionIndex.value ||
1058+
nextPosition === animatedNextPosition.value ||
1059+
isForcedClosing.value
1060+
) {
1061+
return;
1062+
}
10641063

1065-
/**
1066-
* reset temporary position boolean.
1067-
*/
1068-
isInTemporaryPosition.value = false;
1064+
/**
1065+
* reset temporary position boolean.
1066+
*/
1067+
isInTemporaryPosition.value = false;
10691068

1070-
runOnUI(animateToPosition)(
1071-
nextPosition,
1072-
ANIMATION_SOURCE.USER,
1073-
0,
1074-
animationConfigs
1075-
);
1076-
},
1077-
[
1078-
animateToPosition,
1079-
isLayoutCalculated,
1080-
isInTemporaryPosition,
1081-
isForcedClosing,
1082-
animatedSnapPoints,
1083-
animatedNextPosition,
1084-
animatedNextPositionIndex,
1085-
]
1086-
);
1069+
runOnUI(animateToPosition)(
1070+
nextPosition,
1071+
ANIMATION_SOURCE.USER,
1072+
0,
1073+
animationConfigs
1074+
);
1075+
});
10871076
const handleSnapToPosition = useWorkletCallback(
10881077
function handleSnapToPosition(
10891078
position: number | string,
@@ -1867,10 +1856,13 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
18671856
* @alias onIndexChange
18681857
*/
18691858
useEffect(() => {
1870-
if (isAnimatedOnMount.value) {
1871-
handleSnapToIndex(_providedIndex);
1859+
// early exit, if animate on mount is set and it did not animate yet.
1860+
if (animateOnMount && !isAnimatedOnMount.value) {
1861+
return;
18721862
}
1873-
}, [_providedIndex, isAnimatedOnMount, handleSnapToIndex]);
1863+
1864+
handleSnapToIndex(_providedIndex);
1865+
}, [animateOnMount, _providedIndex, isAnimatedOnMount, handleSnapToIndex]);
18741866
//#endregion
18751867

18761868
// render

src/hooks/useStableCallback.ts

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { useCallback, useEffect, useLayoutEffect, useRef } from 'react';
22

3-
// biome-ignore lint: to be addressed!
4-
type Callback<T> = (...args: T[]) => any;
3+
type Callback<T extends unknown[], R> = (...args: T) => R;
54

65
/**
76
* Provide a stable version of useCallback.
87
*/
9-
export function useStableCallback<T>(callback: Callback<T>) {
10-
const callbackRef = useRef<Callback<T>>();
8+
export function useStableCallback<T extends unknown[], R>(
9+
callback: Callback<T, R>
10+
) {
11+
const callbackRef = useRef<Callback<T, R>>();
1112

1213
useLayoutEffect(() => {
1314
callbackRef.current = callback;
@@ -19,7 +20,7 @@ export function useStableCallback<T>(callback: Callback<T>) {
1920
};
2021
}, []);
2122

22-
return useCallback<Callback<T>>((...args) => {
23+
return useCallback<Callback<T, R | undefined>>((...args) => {
2324
return callbackRef.current?.(...args);
2425
}, []);
2526
}

0 commit comments

Comments
 (0)