Skip to content

Conversation

@nmassey
Copy link
Contributor

@nmassey nmassey commented Jul 22, 2024

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 0d2b930 (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!! 🥳🙌

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 22, 2024
@changeset-bot
Copy link

changeset-bot bot commented Jul 22, 2024

🦋 Changeset detected

Latest commit: 0abdb2d

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

@vercel
Copy link

vercel bot commented Jul 22, 2024

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

@yannick-softwerft
Copy link

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.

@quememo
Copy link

quememo commented Aug 2, 2024

would love to see this merged soon

@JanDoeTian
Copy link

Thanks for fixing, hope this is merged soon!

@jvgeee
Copy link

jvgeee commented Aug 9, 2024

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

@qwertychouskie
Copy link

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

@sebak94
Copy link

sebak94 commented Sep 3, 2024

We need this one 🙏 Thanks!

@dohooo dohooo changed the base branch from main to dev September 8, 2024 15:15
@dohooo dohooo merged commit 7b025bd into dohooo:dev Sep 8, 2024
@qwertychouskie
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants