With animationType='spring', calling 2 times snapTo with same index will block subsequent calls to snapTo

[alexstrat]

Current Behavior

With this modification of the "bank" example:

diff --git a/example/screens/SectionListExample.tsx b/example/screens/SectionListExample.tsx
index 192ff26..dd1acdd 100644
--- a/example/screens/SectionListExample.tsx
+++ b/example/screens/SectionListExample.tsx
@@ -9,7 +9,7 @@
 import React from 'react';
 import Constants from 'expo-constants';
 import { FontAwesome5, Ionicons } from '@expo/vector-icons';
-import { Dimensions, Platform, StyleSheet, Text, View } from 'react-native';
+import { Dimensions, Platform, StyleSheet, Text, View, TouchableHighlight } from 'react-native';
 import { Colors, ProgressBar } from 'react-native-paper';
 import ScrollBottomSheet from 'react-native-scroll-bottom-sheet';
 import { StackNavigationProp } from '@react-navigation/stack';
@@ -36,7 +36,8 @@ const navBarHeight = 56;
 const sections = createMockData();
 
 const SectionListExample: React.FC<Props> = () => {
-  const snapPointsFromTop = [96, '45%', windowHeight - 264];
+  const snapPointsFromTop = [96, '45%', windowHeight];
+  const sheetRef = React.useRef<ScrollBottomSheet<ListItemData>>(null);
   const animatedPosition = React.useRef(new Value(0.5));
   const handleLeftRotate = concat(
     interpolate(animatedPosition.current, {
@@ -76,6 +77,11 @@ const SectionListExample: React.FC<Props> = () => {
 
   return (
     <View style={styles.container}>
+      <View style={{ flexDirection: 'row', justifyContent: 'space-around', width: '100%'}}>
+        <TouchableHighlight onPress={() => sheetRef.current?.snapTo(0)}><Text>Snap to 0</Text></TouchableHighlight>
+        <TouchableHighlight onPress={() => sheetRef.current?.snapTo(1)}><Text>Snap to 1</Text></TouchableHighlight>
+        <TouchableHighlight onPress={() => sheetRef.current?.snapTo(2)}><Text>Snap to 2</Text></TouchableHighlight>
+      </View>
       <View style={styles.balanceContainer}>
         <Text style={styles.poundSign}>£</Text>
         <Text style={styles.balance}>4,345</Text>
@@ -116,9 +122,11 @@ const SectionListExample: React.FC<Props> = () => {
         </View>
       </View>
       <ScrollBottomSheet<ListItemData>
+        ref={sheetRef}
         enableOverScroll
         removeClippedSubviews={Platform.OS === 'android' && sections.length > 0}
         componentType="SectionList"
+        animationType="spring"
         topInset={statusBarHeight + navBarHeight}
         animatedPosition={animatedPosition.current}
         snapPoints={snapPointsFromTop}

To reproduce:

• tap on "Snap to 1"
• => it snaps to 1 ✅
• tap a 2nd time on "Snap to 1"
• tap on "Snap to 0"
• => nothing happens ❌

Expected Behavior

Expected:

• tap on "Snap to 1"
• => it snaps to 1 ✅
• tap a 2nd time on "Snap to 1"
• tap on "Snap to 0"
• => it snaps to 0

You can verify the expected behavior if you remove the prop animationType="spring"

Your Environment

	version
Platform (Android, iOS or both)	iOS 14.0
react-native-scroll-bottom-sheet	master@4267d879b88d686b3c28a649dbaa3ec3203108dd
react-native	https://github.com/expo/react-native/archive/sdk-38.0.2.tar.gz
react-native-gesture-handler	1.6.1
react-native-reanimated	1.9.0

[alexstrat]

After some time of investigation, I realized that, when the animation is no-op (ie position is already at toValue), there is a race condition between resetting isManuallySetValue to 0 when the animation finished and reanimated being able to catch the change of isManuallySetValue at onChange(isManuallySetValue):

With animation=timing:

# tap "Snap to 1"
set isManuallySetValue to 1 in #snapTo
onChange(isManuallySetValue) actions are run
animation is run
[...a moment]
set isManuallySetValue to 0 in finished state of the animation
onChange(isManuallySetValue) actions are run

# tap "Snap to 1"
set isManuallySetValue to 1 in #snapTo
onChange(isManuallySetValue) actions are run
animation is run <- no op, instantaneous 
set isManuallySetValue to 0 in finished state of the animation
onChange(isManuallySetValue) actions are run

# tap "Snap to 2"
set isManuallySetValue to 1 in #snapTo
onChange(isManuallySetValue) actions are run
animation is run
[...a moment]
set isManuallySetValue to 0 in finished state of the animation
onChange(isManuallySetValue) actions are run

With animation=spring:

# tap "Snap to 1"
set isManuallySetValue to 1 in #snapTo
onChange(isManuallySetValue) actions are run
animation is run
[...a moment]
set isManuallySetValue to 0 in finished state of the animation
onChange(isManuallySetValue) actions are run

# tap "Snap to 1"
set isManuallySetValue to 1 in #snapTo
animation is run <- no op, instantaneous 
set isManuallySetValue to 0 in finished state of the animation

# tap "Snap to 2"
set isManuallySetValue to 1 in #snapTo
animation is run <- no op, instantaneous because `this.destSnapPoint` was not properly updated because onChange(isManuallySetValue) actions are never run
set isManuallySetValue to 0 in finished state of the animation

I was able to debunk the race condition with this but it's not clean:

diff --git a/src/index.tsx b/src/index.tsx
@@ -557,7 +558,7 @@ export class ScrollBottomSheet<T extends any> extends Component<Props<T>> {
             cond(eq(this.destSnapPoint, snapPoints[0]), [
               set(this.dragWithHandle, 0),
             ]),
-            set(this.isManuallySetValue, 0),
+            call([], () => this.isManuallySetValue.setValue(0)),
             set(this.manualYOffset, 0),
             stopClock(clock),
             this.prevTranslateYOffset,

[alexstrat]

An other solution would be to use only imperative methods rather than relying on onChange

see #54