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

[iOS] ScrollView event fatal crash #2285

Closed
1 of 3 tasks
piaskowyk opened this issue Aug 13, 2021 · 22 comments
Closed
1 of 3 tasks

[iOS] ScrollView event fatal crash #2285

piaskowyk opened this issue Aug 13, 2021 · 22 comments
Assignees
Labels
Bug bug-bash-jan22 Issues visited during Bug Bash Jan 2022 Crash 🏠 Reanimated 2

Comments

@piaskowyk
Copy link
Member

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: #2122 try to move the content ScrollView.

Screen.Recording.2021-08-13.at.21.33.32.mov

Package versions

  • React Native Reanimated: 2.3.0.alpha.2

Affected platforms

  • Android
  • iOS
  • Web
@valentinoConti
Copy link

valentinoConti commented Aug 13, 2021

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 #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
Copy link

bnovarini commented Nov 29, 2022

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

Simulator.Screen.Recording.-.iPhone.14.-.2022-11-29.at.18.24.53.mp4

@tcolinpa
Copy link

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

#2681
#1947

thanks.

@FutureOfRussia
Copy link

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
Copy link

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
Copy link
Contributor

efstathiosntonas commented Mar 5, 2023

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
Copy link

hadnet commented Mar 7, 2023

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

@yepMad
Copy link

yepMad commented Mar 10, 2023

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

@lodev09
Copy link

lodev09 commented Apr 2, 2023

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
Copy link

sweatherall commented Apr 21, 2023

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
Copy link

Has anybody been able to get around this in v3? Unfortunately is very easy for me to reproduce (#4394) and I have not been able to find a fix.

@terrysahaidak
Copy link
Contributor

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
Copy link
Contributor

sunnylqm commented May 2, 2023

Is this pr meant to fix this? #4403

@terrysahaidak
Copy link
Contributor

Can confirm #4403 fixes our crash.

@raphaelinmanage
Copy link

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

@cepem
Copy link

cepem commented May 12, 2023

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
Copy link

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

@yepMad
Copy link

yepMad commented May 19, 2023

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

Hey. Can you try it? #4221

@piaskowyk
Copy link
Member Author

piaskowyk commented May 23, 2023

Fixed by: #4403 The fix will be available since 3.2.0

@sunnylqm
Copy link
Contributor

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

@piaskowyk
Copy link
Member Author

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

@koplyarov
Copy link

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

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