fix: dependency array; removes shared value access warning from react-native-reanimated

[nmassey]

Fixes #706

What: the bug

Where in our code

In ScrollViewGesture.tsx, we had included touching.value and scrollEndTranslation.value in the dependency arrays for a couple of React callbacks. Not only is this not necessary, but it also causes a warning message to appear (and introduces the potential performance implication mentioned below).

These dependency array .values were introduced in eb21293 (as a part of #116).

Warning message in react-native-reanimated

As per #706 (thank you, @joaofelippe911), a console warning appears when using react-native-reanimated version 3.16.0 or higher. This new warning was added via software-mansion/react-native-reanimated#6310

The warning looks like:

 WARN  [Reanimated] Reading from `value` during component render. Please ensure that you do not access the `value` property or use `get` method of a shared value while React is rendering a component.

If you don't want to see this message, you can disable the `strict` mode. Refer to:
https://docs.swmansion.com/react-native-reanimated/docs/debugging/logger-configuration for more details.

Why: potential performance implication

As per https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue/#remarks ,

When you read the sv.value on the JavaScript thread, the thread will get blocked until the value is fetched from the UI thread. In most cases it will be negligible, but if the UI thread is busy or you are reading a value multiple times, the wait time needed to synchronize both threads may significantly increase.

What: the fix

Don't depend on touching.value and scrollEndTranslation.value in our dependency arrays in useCallback().

These aren't needed: all accesses to touching.value and scrollEndTranslation.value are within worklets, and as per https://docs.swmansion.com/react-native-reanimated/docs/core/useSharedValue/#remarks :

When you change the sv.value the update will happen synchronously on the UI thread.

Verification: in local testing, the warning disappears!

In my local testing, this makes the warning disappear completely! And the carousel still behaves as normal.

Aside: where is this code used anyway?

In ScrollViewGesture, there is a useAnimatedReaction() which calls resetBoundary() for changes to translation.value whenever pagingEnabled is false.

resetBoundary() calls activeDecay() in some cases... which in turn calls onFinish() in some cases.

It is worth noting that onFinish, activeDecay, and resetBoundary are only ever used when pagingEnabled is false.

(After a few minutes of playing around with my code for my use case, I was not able to cause activeDecay() to ever run.)

[changeset-bot]

🦋 Changeset detected

Latest commit: 86b8308

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]

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	Nov 5, 2024 8:41pm

[MatkoMilic]

I still get these warnings on newest version?

[Faruktulumcu]

Are there any updates on this pull request? Do we know if it’s going to be merged?

[el-lsan]

Would be great if we can have this merged
Thanks

[BrodaNoel]

Thanks for this fix! It's killing the console on Expo SDK 52 beta

[bigriot]

Do we have an ETA on when this will be merged?

[angelo-hub]

bump on this too

[LukasMod]

For me this fix did not help with all warnings. I had to remove other .value from arrays. Here is my patch fix for 3.5.1

react-native-reanimated-carousel+3.5.1.patch

[nmassey]

For me this fix did not help with all warnings. I had to remove other .value from arrays. Here is my patch fix

react-native-reanimated-carousel+3.5.1.patch

hi @LukasMod - it looks like your patch is for an older version (3.5.1) than the current mainline branch. I think those other .values are no longer relevant to what's on main.

[BrodaNoel]

This is another reason why we need to release 4.0.0 ASAP.

[ansh]

Please release this ASAP! @dohooo