Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.0.0-alpha.12 crashes carousel on android and ios #607

Open
ckanissatran opened this issue May 10, 2024 · 3 comments
Open

4.0.0-alpha.12 crashes carousel on android and ios #607

ckanissatran opened this issue May 10, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@ckanissatran
Copy link

Describe the bug
Updating from 4.0.0-alpha.10 to 12 crashes a carousel that has gifs in it. (memory issue?)
This also strangely only happens in production, not in an emulator.

Using the patch someone else posted here in the past with 4.0.0-alpha.10 works fine though
Any ideas?

diff --git a/node_modules/react-native-reanimated-carousel/src/components/ScrollViewGesture.tsx b/node_modules/react-native-reanimated-carousel/src/components/ScrollViewGesture.tsx
index 41e39b4..0bac93c 100644
--- a/node_modules/react-native-reanimated-carousel/src/components/ScrollViewGesture.tsx
+++ b/node_modules/react-native-reanimated-carousel/src/components/ScrollViewGesture.tsx
@@ -116,15 +116,18 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
   );
 
   const endWithSpring = React.useCallback(
-    (onFinished?: () => void) => {
+    (scrollEndTranslationValue: number,
+      scrollEndVelocityValue: number,
+      onFinished?: () => void,
+    ) => {
       "worklet";
       const origin = translation.value;
-      const velocity = scrollEndVelocity.value;
+      const velocity = scrollEndVelocityValue;
       // Default to scroll in the direction of the slide (with deceleration)
       let finalTranslation: number = withDecay({ velocity, deceleration: 0.999 });
 
       // If the distance of the swipe exceeds the max scroll distance, keep the view at the current position
-      if (maxScrollDistancePerSwipeIsSet && Math.abs(scrollEndTranslation.value) > maxScrollDistancePerSwipe) {
+      if (maxScrollDistancePerSwipeIsSet && Math.abs(scrollEndTranslationValue) > maxScrollDistancePerSwipe) {
         finalTranslation = origin;
       }
       else {
@@ -137,9 +140,9 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
         * */
         if (pagingEnabled) {
           // distance with direction
-          const offset = -(scrollEndTranslation.value >= 0 ? 1 : -1); // 1 or -1
+          const offset = -(scrollEndTranslationValue >= 0 ? 1 : -1); // 1 or -1
           const computed = offset < 0 ? Math.ceil : Math.floor;
-          const page = computed(-translation.value / size);
+          const page = computed(-origin / size);
 
           if (loop) {
             const finalPage = page + offset;
@@ -178,9 +181,9 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
       snapEnabled,
       translation,
       pagingEnabled,
-      scrollEndVelocity.value,
+      // scrollEndVelocity.value,
       maxScrollDistancePerSwipe,
-      scrollEndTranslation.value,
+      // scrollEndTranslation.value,
       maxScrollDistancePerSwipeIsSet,
     ],
   );
@@ -335,10 +338,12 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
     "worklet";
 
     const { velocityX, velocityY, translationX, translationY } = e;
-    scrollEndVelocity.value = isHorizontal.value
+    const scrollEndVelocityValue = isHorizontal.value
       ? velocityX
       : velocityY;
     
+    scrollEndVelocity.value = scrollEndVelocityValue; // may update async: see https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue#remarks
+
     let panTranslation = isHorizontal.value
       ? translationX
       : translationY;
@@ -349,9 +354,9 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
     else if (fixedDirection === "positive")
       panTranslation = +Math.abs(panTranslation);
 
-    scrollEndTranslation.value = panTranslation;
+      scrollEndTranslation.value = panTranslation; // may update async: see https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue#remarks
 
-    const totalTranslation = scrollEndVelocity.value + scrollEndTranslation.value;
+      const totalTranslation = scrollEndVelocityValue + panTranslation;
 
     /**
      * If the maximum scroll distance is set and the translation `exceeds the maximum scroll distance`,
@@ -374,7 +379,7 @@ const IScrollViewGesture: React.FC<PropsWithChildren<Props>> = (props) => {
       translation.value = withSpring(withProcessTranslation(nextPage), onScrollEnd);
     }
     else {
-      endWithSpring(onScrollEnd);
+      endWithSpring(panTranslation, scrollEndVelocityValue, onScrollEnd);
     }
 
     if (!loop)


To Reproduce
Steps to reproduce the behavior:

  1. Create a carousel with gifs in them
  2. Try to view the carousel
  3. See that it crashes in production build

Expected behavior
No crash

Versions (please complete the following information):

  • react: v18.2.0
  • react-native: v0.73.6
  • react-native-reanimated: v3.6.2
  • react-native-reanimated-carousel: v4.0.0-alpha.12
  • react-native-gesture-handler: v2.14.0

Smartphone (please complete the following information):

  • Device: [e.g. iPhone12]
  • OS: [e.g. iOS17.4.1]
@ckanissatran ckanissatran added the bug Something isn't working label May 10, 2024
Copy link

dosubot bot commented May 10, 2024

To address the crash issue with react-native-reanimated-carousel version 4.0.0-alpha.12 when using GIFs in a carousel, especially since it only occurs in production and not in an emulator, you've correctly identified that applying a specific patch to ScrollViewGesture.tsx resolves the issue. This suggests the problem might be related to asynchronous updates of shared values used in gesture handling.

Here's a concise plan to implement a fix:

  1. Apply the Patch: Directly apply the changes from version 4.0.0-alpha.10 to ScrollViewGesture.tsx in version 4.0.0-alpha.12. This is a temporary workaround and involves modifying library files directly, which is generally not recommended.

  2. Use Patch-Package: To ensure these changes persist across installs, utilize patch-package. This tool lets you maintain modifications to your node_modules directory. After making the necessary changes, run npx patch-package react-native-reanimated-carousel to create a patch file. Then, add "postinstall": "patch-package" to your package.json scripts to apply the patch automatically after each install.

  3. Test Thoroughly: Ensure to test the patched version on both Android and iOS devices to confirm the crashes are resolved without introducing new issues.

  4. Stay Updated: Keep an eye on the official repository for an official fix. Once available, you can remove the patch and update to the fixed version.

This approach provides a temporary solution while waiting for an official update. Additionally, there's an open issue Not working with reanimated v3.5.1 that might offer further insights or a more permanent solution to this problem.

Details

Sources


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.

@ThoaiBuiMCV
Copy link

me too

@adamhari
Copy link
Contributor

It's happening for me too in production builds and I am not using GIFs in the carousel. It doesn't crash in development builds. Reverting to alpha 9 fixes it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants