react-native-reanimated-carousel icon indicating copy to clipboard operation
react-native-reanimated-carousel copied to clipboard

fix: typeof "function" onProgressChange - fixes app crash

Open nmassey opened this issue 1 year ago • 7 comments

What: the bug

On iOS, when running in "Release" configuration (i.e. not "development" configuration), if onProgressChange is a function, the app crashes. 😖

On Android, similar crash. Thanks to @yannick-softwerft for providing a traceback in a comment below.

This bug was accidentally introduced in 0d2b930f394f65fd70a03593ea8c7b16fb552e62 (in code here)

Why

Since the callback functions in useAnimatedReaction are automatically workletized, when running in Xcode "Release" configuration (i.e. not "development" configuration), if the variable onProgressChange is set to a function (from within the JS thread), then typeof onProgressChange will be equal to "object" from within the workletized function (in the UI thread).

So, the code assumes that the value is a SharedValue and attempts to set onProgressChange.value = absoluteProgress. However, this seems to immediately cause the app to crash.

My configuration

  1. expo: 50.0.19
  2. react-native-reanimated: 3.6.2
  3. react-native-reanimated-carousel: 4.0.0-alpha.12

What: the fix

Remember the value for typeof onProgressChange in the JS thread (instead trying to check it from within the UI thread).

App no longer crashes! And onProgressChange as function works correctly again!! 🥳🙌

nmassey avatar Jul 22 '24 02:07 nmassey

🦋 Changeset detected

Latest commit: 0abdb2d51fba1d309a065ed614713d792b6e70a5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-native-reanimated-carousel Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Jul 22 '24 02:07 changeset-bot[bot]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-reanimated-carousel ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 22, 2024 2:10am

vercel[bot] avatar Jul 22 '24 02:07 vercel[bot]

The error Message I got caused by this issue:

07-29 17:12:47.816  8490  8490 E AndroidRuntime: com.facebook.jni.CppException: TypeError: Cannot assign to property 'value' on HostObject with default setter
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 
07-29 17:12:47.816  8490  8490 E AndroidRuntime: Error: TypeError: Cannot assign to property 'value' on HostObject with default setter
07-29 17:12:47.816  8490  8490 E AndroidRuntime:     at anonymous (JavaScript:1:597)
07-29 17:12:47.816  8490  8490 E AndroidRuntime:     at anonymous (JavaScript:1:95)
07-29 17:12:47.816  8490  8490 E AndroidRuntime:     at mapperRun (JavaScript:1:1044)
07-29 17:12:47.816  8490  8490 E AndroidRuntime:     at anonymous (JavaScript:1:350)
07-29 17:12:47.816  8490  8490 E AndroidRuntime:     at callMicrotasksOnUIThread (JavaScript:1:61)
07-29 17:12:47.816  8490  8490 E AndroidRuntime:     at anonymous (JavaScript:1:139)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at com.swmansion.reanimated.AndroidUIScheduler.triggerUI(Native Method)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at com.swmansion.reanimated.AndroidUIScheduler$1.run(AndroidUIScheduler.java:24)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at com.swmansion.reanimated.AndroidUIScheduler$2.runGuarded(AndroidUIScheduler.java:43)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at com.facebook.react.bridge.GuardedRunnable.run(GuardedRunnable.java:29)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:958)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:99)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at android.os.Looper.loopOnce(Looper.java:205)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:294)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at android.app.ActivityThread.main(ActivityThread.java:8177)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
07-29 17:12:47.816  8490  8490 E AndroidRuntime: 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971)

Based on this it took me some time to find this PR. Thx so much for providing a patch.

yannick-softwerft avatar Jul 29 '24 17:07 yannick-softwerft

would love to see this merged soon

quememo avatar Aug 02 '24 13:08 quememo

Thanks for fixing, hope this is merged soon!

JanDoeTian avatar Aug 08 '24 18:08 JanDoeTian

@dohooo / @oliverloops Can you merge this in please?

jvgeee avatar Aug 09 '24 00:08 jvgeee

@dohooo Is there anything holding up this PR from being merged?

qwertychouskie avatar Aug 14 '24 20:08 qwertychouskie

We need this one 🙏 Thanks!

sebak94 avatar Sep 03 '24 12:09 sebak94

@dohooo Thanks for merging! Is there any approximate timeline on merging the other bugfix PRs that are waiting and publishing a new release?

qwertychouskie avatar Sep 09 '24 08:09 qwertychouskie