[bug] ["with fix"] onSnapToItem not properly called

[BrodaNoel]

Describe the bug
Ok... Here I could reproduce the error: https://youtu.be/KADZzaGaUJM

The sum-up of that video is this: If I move to the next photo REALLY FAST, the onSnapToItem function gets called with the previous index. I mean, if I move from photo index 0 to the next (photo index 1), onSnapToItem is called with index = 0.
But, if I move to the next photo SLOW, the onSnapToItem gets called properly.

PS.: I'm not 100% sure if this issue is because of this PR, but I have this PR patched in my local env right now. I may have to remove the patch and try without it, but I can't because I see other issues.

The error
After adding 3 million console.log in the RNRC code, I have finally found the problem.

This is what I get in the console:

 LOG  $ useCarouselController.tsx : 2nd parameter of `useAnimatedReaction` (the one who uses newSharedIndexValue) has ran New newSharedIndexValue value new index: 0
 LOG  $ useCarouselController.tsx : 2nd parameter of `useAnimatedReaction` (the one who uses newSharedIndexValue) has ran New newSharedIndexValue value new index: 0
 LOG  $ Carousel.tsx : _onScrollEnd has ran
 LOG  Changed 0
 LOG  $ useCarouselController.tsx : 2nd parameter of `useAnimatedReaction` (the one who uses newSharedIndexValue) has ran New newSharedIndexValue value new index: 1

(scroll to the right and see the new index, at the end of the console lines)

So... Seems like 2nd parameter of useAnimatedReaction (the one who uses newSharedIndexValue) is ran after _onScrollEnd

Now...
Please check this in the vide: When I release my finger from the phone, there is no animation to the next item. It's like... Since I upgraded to SDK 50, there is no animation anymore from the moment I release the finger (where the Carousel should finish to move to the next image with an animation)... This make me think that probably there are some broken thing in a higer lever, who produces this error.

I've seen that other devs, like @nmassey does not have this problem. He has SDK 50, and the animations still smooth...

Versions (please complete the following information):
"react": "18.2.0",
"react-native": "0.73.6",
"react-native-reanimated": "~3.6.2",
"react-native-reanimated-carousel": "4.0.0-alpha.12",
"react-native-gesture-handler": "~2.14.0",

Smartphone (please complete the following information):

• Device: iPhone 13 mini
• OS: 17.4.1

[dosubot]

The issue with onSnapToItem not being called correctly when swiping quickly seems to be related to the synchronization between animations and event handling in the react-native-reanimated-carousel. The problem might stem from the way animations are configured, how events are handled, or how the carousel's state is managed, especially in response to rapid swipes.

1. Animation Configuration: Ensure that animation settings are optimized for quick interactions. This includes reviewing the scrollAnimationDuration and withAnimation props to ensure they can handle rapid swipes without lag or missed event triggers.

2. Event Handling: The sequencing issue where the useAnimatedReaction is called after _onScrollEnd suggests that the event handling logic might not be adequately capturing the end of the swipe gesture, especially for fast swipes. Enhancing the logic to better detect and handle rapid changes could mitigate this issue.

3. State Management: The current index update mechanism needs to robustly reflect user interactions, even when they occur quickly. This might involve refining how the carousel's state is updated in response to swipe gestures to ensure accurate tracking of the current index.

Regarding the SDK upgrade and its potential impact, it's worth noting that recent changes and fixes in react-native-reanimated-carousel and react-native-reanimated could influence the carousel's behavior. Specifically, enhancements to animation control and fixes for issues related to onSnapToItem behavior in certain conditions might be relevant. Additionally, adjustments to gesture handler configurations, such as modifying activeOffsetX for the carousel's pan gesture, could improve responsiveness and accuracy in detecting swipe gestures.

To address the issue, consider reviewing and adjusting the carousel's animation configurations, enhancing event handling logic for rapid swipes, and ensuring robust state management for accurate index tracking. Also, explore recent changes in the SDK and component library that might affect animation and event handling behavior. If the problem persists, providing more details about your implementation could help in identifying specific solutions.

Details

Sources

• onProgressChanged and onSnapToItem takes too long to give back an index
• [Expo SDK 50] [alpha.x] Weird animation issue
• onSnapToItem inconsistent behaviour
• src/hooks/useCarouselController.tsx
• src/components/Carousel.tsx
• example/app/src/pages/quick-swipe/index.tsx
• ARCHIVED.CHANGELOG.md
• src/hooks/useOnProgressChange.ts
• example/website/pages/faq.mdx
• CHANGELOG.md

About Dosu
This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.

[BrodaNoel]

Btw, this line is driving me crazy: const isPositive = handlerOffsetValue <= 0;

[BrodaNoel]

I have more data.

Seems like, while swiping fast, in this code:

  useAnimatedReaction(
    () => {
      const handlerOffsetValue = handlerOffset.value;
      console.log('$ useAnimatedReaction', handlerOffsetValue);
      const toInt = round(handlerOffsetValue / size) % dataInfo.length;
      const isPositive = handlerOffsetValue <= 0;
      const i = isPositive
        ? Math.abs(toInt)
        : Math.abs(toInt > 0 ? dataInfo.length - toInt : 0);

      const newSharedIndexValue = convertToSharedIndex({
        loop,
        rawDataLength: dataInfo.originalLength,
        autoFillData: autoFillData!,
        index: i,
      });

      console.log('$ lista la data', toInt, i, newSharedIndexValue);

      return {
        i,
        newSharedIndexValue,
      };
    },

What happens is that handlerOffsetValue is near -10, and this whole code is called only 1 time, so, const toInt = round(handlerOffsetValue / size) % dataInfo.length; is 0.

But... Milliseconds after, when the Carousel finished to show the next image (WITHOUT ANIMATION!), the value is the proper, but it's too late

[BrodaNoel]

As you may imagine, if I add this:

  function setSharedIndex(newSharedIndex: number) {
    sharedIndex.current = newSharedIndex;
    onScrollEnd(); // <<< THIS!
  }

It fixes the issue... But the onSnapToItem gets called a lot of times, instead of only 1

[BrodaNoel]

I just fixed it doing this:

    const _onScrollEnd = React.useCallback(() => {
      setTimeout(() => { // <<< PLEASE DON'T KILL ME!!! DON'T TELL MY MOM!!!
        const _sharedIndex = Math.round(getSharedIndex());
  
        const realIndex = computedRealIndexWithAutoFillData({
          index: _sharedIndex,
          dataLength: rawDataLength,
          loop,
          autoFillData,
        });
  
        if (onSnapToItem) onSnapToItem(realIndex);
  
        if (onScrollEnd) onScrollEnd(realIndex);
      }, 1)
    }, [
      loop,
      autoFillData,
      rawDataLength,
      getSharedIndex,
      onSnapToItem,
      onScrollEnd,
    ]);

[nmassey]

Ugh. I'm pretty sure you're right about this being a race condition due to how code is being called (really, scheduled) via runOnJS and useAnimatedReaction. 😭

@BrodaNoel - How are you "moving to the next photo REALLY FAST"? Are you just panning extremely quickly to the next slide?

Although I agree with you about wanting to get the value for this callback fixed properly, could using the onProgressChange prop be a viable alternative for your purposes?
(Note that in addition to setting onProgressChange to a callback function, the property can now be a shared value instead of a function (as of 4.0.0-alpha.11 - see here: code change and documentation change).

(And, yes, as you mention in the initial comment, my animations are good 👍, and I am using Expo SDK 50. But I do have a number of patches for RNRC installed, including #574, #576, #577, and #596.)

[BrodaNoel]

Yes, in order to swipe fast, I just do it super quickly with my finger.

The point is that you don't REALLY have to do it super fast. But you can reproduce it easier in that way.