software-mansion / react-native-reanimated

React Native's Animated library reimplemented
https://docs.swmansion.com/react-native-reanimated/
MIT License
9.04k stars 1.31k forks source link

[iOS] ScrollView event fatal crash #2285

Closed piaskowyk closed 1 year ago

piaskowyk commented 3 years ago

Description

REANodeManager try to handle events from ScrollView but it causes a fatal crash. Mouting block is expected to not be set

Actual behavior & steps to reproduce

Run code from PR: https://github.com/software-mansion/react-native-reanimated/pull/2122 try to move the content ScrollView.

https://user-images.githubusercontent.com/36106620/129409776-b51c2680-6cd7-4336-9423-8839de073bcd.mov

Package versions

Affected platforms

valentinoConti commented 3 years ago

Fixed!

I'm almost sure that at the time I created that PR that wasn't happening, the first thing I did was going to the code were that crash happens (on ios/REANodesManager.m) and commented it, and it started working fine without crashing:

- (BOOL)uiManager:(RCTUIManager *)manager performMountingWithBlock:(RCTUIManagerMountingBlock)block {
  RCTAssert(_mounting == nil, @"Mouting block is expected to not be set");
  _mounting = block;
  return YES;
}

But, because I know nothing about native code I did a research to see if I found something else to do, I found this open issue https://github.com/software-mansion/react-native-reanimated/issues/1947 with same problem and used the same fix that they say on the comments, wrap the interpolate on a withTiming with only 10 of duration, works perfectly fine now, but I don't really know if that is the way to go or if there is some issue on the native code used on the reanimated repo

Cheers!

bnovarini commented 1 year ago

Hey! I used the solution proposed by @valentinoConti as a quick fix to avoid crashing the app, however the scroll keeps flickering/bouncing with this approach. I've tried several durations and also tried both inside useAnimatedScrollHandler and wrapping the interpolate function. Both avoid crashing, but result in the flickering behavior.

Using v2.13.0


  const scrollHandler = useAnimatedScrollHandler((event) => {
    animatedOffsetY.value = withTiming(event.contentOffset.y, { duration: 10 })
  })

  const magicScroll = useAnimatedStyle(() => {
    const paddingTop = interpolate(animatedOffsetY.value, [0, 64], [100, 41], Extrapolate.CLAMP)
    return {
      paddingTop
    }
  })

Attached is a video showing the behavior

https://user-images.githubusercontent.com/49925472/204670604-130cdc08-4713-4482-aced-bc1e20c580ae.mp4

tcolinpa commented 1 year ago

@piaskowyk is there any development on this? there's all kind of issues/pr closed about this but no clear answer

https://github.com/software-mansion/react-native-reanimated/pull/2681 https://github.com/software-mansion/react-native-reanimated/issues/1947

thanks.

FutureOfRussia commented 1 year ago

After updating to version 3.0.0 found the same problem. Adding withTiming and/or interpolate doesn't solve the problem. In my case, the application crashes when the height of the component inside the Animated.ScrollView is animated to decrease.

ludwig-pro commented 1 year ago

We also have the same problem on our application but we need the "hard stress" the screen.

The workaround seems to work.

  const scrollHandler = useAnimatedScrollHandler((event) => {
    animatedOffsetY.value = withTiming(event.contentOffset.y, { duration: 10 })
  })
efstathiosntonas commented 1 year ago

Same here, tried to use withTiming but it still crashes. The list has only 1 item in it. If there are multiple items it doesn't crash, I'm using FlashList and reanimated 3.0.1

@bnovarini I've also encountered this filckering, 100% it's a math issue, try playing around with the [0, 64], [100, 41] values and the initial absolute position of the TabBar. This flickering was happening to me only on Android though, iOS was fine.

hadnet commented 1 year ago

Still happening on v2.14.4. Unfortunately, the withTiming workaround has some flickering.

yepMad commented 1 year ago

Still happening in v3.0.2. This is happening to me on a FlatList. It wasn't happening in version v2.14.4

lodev09 commented 1 year ago

After some few iterations, below is the solution I've come up with to fix the flicker/crash bug.

First, you need to determine the static height of the scrollview minus the height of the header, insets, tab bar, etc.

const insets = useSafeAreaInsets()
const dimentions = useWindowDimensions()

// The static height of the content within the screen
const contentHeight = useMemo(
  () =>
    dimentions.height -
    (insets.top + insets.bottom + HEADER_HEIGHT + SEARCH_BAR_HEIGHT + TAB_BAR_HEIGHT /* etc */),
  [insets.top, insets.bottom, dimentions.height],
)

Second, on the useAnimatedScrollHandler, you only want to update the scrollY.value if the scrolled height is smaller than the "actual" scrollview height. e.g.

const SCROLL_DRAG_AREA = 200
const scrollY = useSharedValue(0)
const scrollHandler = useAnimatedScrollHandler(
  {
    onScroll: (e) => {
      // If the "static" height is greater than the actual height
      // DO NOT update scroll value
      if (e.contentOffset.y + contentHeight + SCROLL_DRAG_AREA > e.contentSize.height) {
        return
      }

      // Safe
      scrollY.value = e.contentOffset.y
    },
  },
  [contentHeight],
)

Now you can interpolate the scroll value e.g.

const $animatedHeader = useAnimatedStyle(
  () => ({
    height: interpolate(
      scrollY.value,
      [0, SCROLL_DRAG_AREA],
      [HEADER_HEIGHT, 0],
      Extrapolation.CLAMP,
    ),
    opacity: interpolate(
      scrollY.value,
      [0, SCROLL_DRAG_AREA],
      [1, 0],
      Extrapolation.CLAMP,
    ),
  }),
)

The reason this works (at least on my case) is that the reanimated bug happens only if the actual content height of the scrollview is smaller than the static height when overScrollMode is enabled (default). The only drawback on this is that the header animated height is ignored during over scroll if there's less content 🤷‍♂️

I hope this helps on someway while debugging the root cause, but for now this works as expected without the flicker/crash

sweatherall commented 1 year ago

Still happening in v3.1.0. In addition, the withTiming + duration fix no longer seems to work (whereas it did work on v2.14.4).

LeviWilliams commented 1 year ago

Has anybody been able to get around this in v3? Unfortunately is very easy for me to reproduce (https://github.com/software-mansion/react-native-reanimated/issues/4394) and I have not been able to find a fix.

terrysahaidak commented 1 year ago

In our case, changing zIndex during the animation caused the crash on 3.1.0:

https://github.com/Expensify/react-navigation/blob/main/packages/drawer/src/views/modern/Overlay.tsx#L24

sunnylqm commented 1 year ago

Is this pr meant to fix this? https://github.com/software-mansion/react-native-reanimated/pull/4403

terrysahaidak commented 1 year ago

Can confirm https://github.com/software-mansion/react-native-reanimated/pull/4403 fixes our crash.

raphaelinmanage commented 1 year ago

heyy, i still have this issue on 2.15 and 2.17 on 2.14.4 works fine can anyone help me?

cepem commented 1 year ago

This is still happening on 2.x for me. Upgrading to 3.x doesn't seem to be an option as that causes even more crashes. I just migrated everything out of rage to Animated from RN and probably won't ever use Reanimated again.

batuhansahan commented 1 year ago

This is randomly happening on IOS could not found the source of issue and neither reproduced it.

yepMad commented 1 year ago

This is randomly happening on IOS could not found the source of issue and neither reproduced it.

Hey. Can you try it? https://github.com/software-mansion/react-native-reanimated/issues/4221

piaskowyk commented 1 year ago

Fixed by: https://github.com/software-mansion/react-native-reanimated/pull/4403 The fix will be available since 3.2.0

sunnylqm commented 1 year ago

@piaskowyk I think the fix is only landed to main branch but not released to any version yet.

piaskowyk commented 1 year ago

@sunnylqm you're right, me bad 🙏 that's mean I need to make a release as soon as possible

koplyarov commented 1 year ago

Hey @piaskowyk, can we please also backport this to 2.x, since the diff that causes this issue is in that branch as well? 🙂