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

Passing the same useAnimatedScrollHandler to different scroll views breaks

Open gaearon opened this issue 1 year ago • 14 comments

Description

We're seeing an issue where passing the same useAnimatedScrollHandler({ onScroll }) to multiple flat list nested inside a pager fails to register scroll events for the last scroll view.

Replacing it with a vanilla onScroll handler fixes it which makes me think it's a problem with Reanimated.

Moving the useAnimatedScrollHandler Hook into the leaf components (so creating a handler per component instead of passing down the same handler to many components) also works around the issue.

Steps to reproduce

I'm sorry, but I don't have time to extract an isolated repro right now.

I was thinking to file the issue anyway because maybe you can guess the source of the problem.

If you do want to try my non-isolated repro, you'd need a Bluesky account — happy to send an invite (pls DM me on twitter or email me). The repro step there is to open the profile tab, click on "Posts & Replies" and observe the onScroll handler from <PagerWithHeader> only firing for the first <Feed> to which it's passed to from screens/Profile.tsx.

If you want to try to recreate the issue from scratch, maybe you can try:

  1. Declaring one scroll handler
  2. Passing it to multiple flat lilsts
  3. If that's not enough, maybe wrapping those flat lists into an RN pager

Snack or a link to a repository

https://github.com/bluesky-social/social-app/commits/profile-tab-reanimated-bug-repro

Reanimated version

3.4.2

React Native version

0.72.5

Platforms

Android, iOS

JavaScript runtime

Hermes

Workflow

None

Architecture

Paper (Old Architecture)

Build type

None

Device

None

Device model

No response

Acknowledgements

Yes

gaearon avatar Nov 07 '23 02:11 gaearon

Hey! 👋

The issue doesn't seem to contain a minimal reproduction.

Could you provide a snack or a link to a GitHub repository under your username that reproduces the problem?

github-actions[bot] avatar Nov 07 '23 02:11 github-actions[bot]

There is a repro link, but it is not under my username. That said, it is indeed not a minimal repro.

gaearon avatar Nov 07 '23 02:11 gaearon

Maybe related, has a repro: https://github.com/software-mansion/react-native-reanimated/issues/5360

gaearon avatar Nov 10 '23 13:11 gaearon

Yea I think https://github.com/software-mansion/react-native-reanimated/issues/5360 is a separate issue but it also contains a reduced repro for this one.

gaearon avatar Nov 10 '23 13:11 gaearon

Here's a reduced repro for this specific issue.

import * as React from 'react'
import {View, Button} from 'react-native'
import Animated, {useAnimatedScrollHandler} from 'react-native-reanimated'

export default function App() {
  const [attachBoth, setAttachBoth] = React.useState(false)

  const [x, setX] = React.useState(0)
  React.useEffect(() => {
    let id = setInterval(() => setX(x => x + 1), 1000)
    return () => clearInterval(id)
  }, [])

  // props to pass into children render functions
  const onScroll = useAnimatedScrollHandler({
    onScroll(e) {
      console.log(x)
    },
  })

  return (
    <View style={{flex: 1, flexDirection: 'row'}}>
      <Button
        title="hi"
        onPress={e => {
          setAttachBoth(!attachBoth)
        }}>
        yo
      </Button>
      <Animated.FlatList
        onScroll={onScroll}
        style={{flex: 1, backgroundColor: 'red'}}
        contentContainerStyle={{
          paddingTop: 1000,
        }}
      />
      <Animated.FlatList
        onScroll={attachBoth ? onScroll : null}
        style={{flex: 1, backgroundColor: 'blue'}}
        contentContainerStyle={{
          paddingTop: 1000,
        }}
      />
    </View>
  )
}
  1. Scroll the first view
  2. Press the button
  3. Try scrolling both views

You'll notice the second view doesn't have the handler attached even though we passed it.

gaearon avatar Nov 10 '23 14:11 gaearon

Confirmed the issue is specifically with the Hook call: passing the same worklet function to two Hooks is fine.

This works:

import * as React from 'react'
import {View} from 'react-native'
import Animated, {useAnimatedScrollHandler} from 'react-native-reanimated'

export default function App() {
  const [x, setX] = React.useState(0)
  React.useEffect(() => {
    let id = setInterval(() => setX(x => x + 1), 1000)
    return () => clearInterval(id)
  }, [])

  // props to pass into children render functions
  const onScroll = e => {
    'worklet'
    console.log(x)
  }

  return (
    <View style={{flex: 1, flexDirection: 'row'}}>
      <Item
        onScroll={onScroll}
        style={{flex: 1, backgroundColor: 'red'}}
        contentContainerStyle={{
          paddingTop: 1000,
        }}
      />
      <Item
        onScroll={onScroll}
        style={{flex: 1, backgroundColor: 'blue'}}
        contentContainerStyle={{
          paddingTop: 1000,
        }}
      />
    </View>
  )
}

function Item({onScroll, ...rest}) {
  const handler = useAnimatedScrollHandler({
    onScroll,
  })
  return <Animated.FlatList {...rest} onScroll={handler} />
}

gaearon avatar Nov 10 '23 15:11 gaearon

facing the same issue here, having a nested scrollview in a pager view, only the first ScrollView works

I have a component that is a Pager View which can have multiple children, the scroll event handler for onScroll only work for the first page, but the second children never register

<PagerView>
{React.Children.map(
      children,
      (child: React.ReactElement<any>, index) => {
        if (React.isValidElement(child)) {
          return (
            <Animated.ScrollView
              key={index}
              scrollEventThrottle={16}
              onScroll={scrollViewHandler}
              contentContainerStyle={{
                flexGrow: 1,
              }}>
             // screens component here
            </Animated.ScrollView>
          );
        }

        return null;
      },
)}
</PagerView>

Page 1: ScrollView - Scroll handler works Page 2: ScrolView - Scroll handler doesnt work

jyan212 avatar Nov 15 '23 07:11 jyan212

Hey, found out the issue we both missed out, we are missing the dependencies parameter for useAnimatedScrollHandler.

const scrollViewHandler = useAnimatedScrollHandler({
 onScroll: handleScroll
}, []) // try adding a dependency here, so the function wont re-render every time

jyan212 avatar Nov 16 '23 01:11 jyan212

Sorry for the late answer, I finally got the time to get to it.

Unfortunately, with current implementation handlers provided by useAnimatedScrollHandler or any other handlers that are an instance of WorkletEventHandler, that being: useScrollViewOffset, useEvent and useAnimatedGestureHandler (last one deprecated) are supposed to be passed to only one component. This is because their class instances keep the data of the latest component only. Passing them to multiple components leads to undefined behavior.

I will consult with the team if we should change their implementation so they could be reusable, but this definitely wouldn't be shipped soon.

I'm also aware that this information isn't available in our documentation or in recently added TSDoc. We'll make amendments there.

tjzel avatar Dec 17 '23 15:12 tjzel

Hey, found out the issue we both missed out, we are missing the dependencies parameter for useAnimatedScrollHandler.

const scrollViewHandler = useAnimatedScrollHandler({
 onScroll: handleScroll
}, []) // try adding a dependency here, so the function wont re-render every time

Thank you for this. Was running into this exact issue and this solved the problem

NicholasBoccuzzi avatar Jan 24 '24 20:01 NicholasBoccuzzi

It seems like it would be worth keeping this issue open as a warning for other people? It violates basic principles of React and is unsolved — so even if it takes a lot of work to fix and isn’t planned in short term, this feels like sweeping it under the rug.

gaearon avatar Feb 23 '24 21:02 gaearon

You are right that this issue shouldn't be closed like that. It wasn't well communicated in @tjzel comment, but the reason for closing was that we weren't planning on supporting this particular usecase where scroll handler is used with multiple components. Despite that I believe the issue shouldn't be closed until at least we add this information to the documentation and add a good error message when someone attempts to do this.

I'll reopen this issue now and we will revisit it with the team soon to see how big of an effort it'd be to properly support this use case or will add a proper error and document this limitation.

kmagiera avatar Feb 26 '24 15:02 kmagiera

If it’s not supported, it seems like a Hook is a wrong API for it. Since any handler can be passed down. A valid API could be a built-in event handler on the wrapper component (since that wouldn’t let anyone pass it down to more handlers). Although that wouldn’t work either if you let people create animated components around their own code.

gaearon avatar Feb 26 '24 15:02 gaearon

I'd actually say it is supposed to work this way and we should allow for all event handlers to work as any handler and in particular to be used with many components. So semantically I believe this is the right API. The reason it doesn't work correctly is due to internal implementation limitations and legacy reasons. That being said, it is, for the vast majority of use cases, used directly next to the component where the handler is attached (that is in its parent component) and hence this issue along with #5364 remained undetected for so long.

We will try to revisit this issue soon to see if we can make it work the proper way w/o adding too much complexity.

kmagiera avatar Feb 26 '24 17:02 kmagiera