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

Animated.event() with useNativeDriver returns an object, not a function #16250

Closed
benadamstyles opened this Issue Oct 8, 2017 · 7 comments

Comments

Projects
None yet
5 participants
@benadamstyles

benadamstyles commented Oct 8, 2017

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 7.8.0
Yarn: 1.0.1
npm: 4.2.0
Watchman: 4.7.0
Xcode: Xcode 9.0 Build version 9A235
Android Studio: Not Found

Packages: (wanted => installed)
react: 16.0.0-beta.5 => 16.0.0-beta.5
react-native: 0.49.1 => 0.49.1

Target Platform: iOS (11)

Steps to Reproduce

I just hit #13377 but with onScroll on a ScrollView. This line checks if I have used useNativeDriver, and if so, returns the AnimatedEvent object, not a function. Therefore, any prop expecting a function – onScroll, onPanResponderMove or any other – will throw an error.

Is this a bug or expected behaviour? If useNativeDriver cannot be used with any prop expecting a function, this should be documented.

@DragonRaider5

This comment has been minimized.

Show comment
Hide comment
@DragonRaider5

DragonRaider5 Oct 10, 2017

This is expected behaviour, as the PanResponder doesn't support native animations yet, because it's implemented in pure JS - hence it can't generate native events, as it has to pass the JS to native bridge to update the animations (because it handles touches in JS). Furthermore it even has to pass the bridge twice and performs worse than standard JS thread animations (which already can be pretty laggy), due to the fact, that the touch also has to be passed to the JS side, making it like this:

Native          |    JS
Touch Event ->  |    handle touch
                |    adjust animation
Animate         | <- new animated value

Problem: The communication over the JS - native bridge is pretty expensive. You could try using something like https://github.com/wix/react-native-interactable (it says experimental, but in my experience it's quiet stable - don't quote me on that tho), which has native implementations with animations, callbacks and configurable physics so you can do quiet a lot with it.

Also see #13377

DragonRaider5 commented Oct 10, 2017

This is expected behaviour, as the PanResponder doesn't support native animations yet, because it's implemented in pure JS - hence it can't generate native events, as it has to pass the JS to native bridge to update the animations (because it handles touches in JS). Furthermore it even has to pass the bridge twice and performs worse than standard JS thread animations (which already can be pretty laggy), due to the fact, that the touch also has to be passed to the JS side, making it like this:

Native          |    JS
Touch Event ->  |    handle touch
                |    adjust animation
Animate         | <- new animated value

Problem: The communication over the JS - native bridge is pretty expensive. You could try using something like https://github.com/wix/react-native-interactable (it says experimental, but in my experience it's quiet stable - don't quote me on that tho), which has native implementations with animations, callbacks and configurable physics so you can do quiet a lot with it.

Also see #13377

@benadamstyles

This comment has been minimized.

Show comment
Hide comment
@benadamstyles

benadamstyles Oct 10, 2017

@DragonRaider5 Thanks for your reply, but I think this is still unexpected behaviour. My use case doesn't involve PanResponder at all.

Animated.event() with useNativeDriver doesn't work with onScroll either, and the docs specifically say here that:

The native driver also works with Animated.event. This is specially useful for animations that follow the scroll position as without the native driver, the animation will always run a frame behind the gesture due to the async nature of React Native.

Just to be totally clear: the bug I am reporting is that useNativeDriver cannot currently be used with any prop expecting a function, including onScroll.

Also: thanks for the interactable recommendation 🙂 – unfortunately it currently has several bugs related to scrolling which as I say is specifically my use case.

benadamstyles commented Oct 10, 2017

@DragonRaider5 Thanks for your reply, but I think this is still unexpected behaviour. My use case doesn't involve PanResponder at all.

Animated.event() with useNativeDriver doesn't work with onScroll either, and the docs specifically say here that:

The native driver also works with Animated.event. This is specially useful for animations that follow the scroll position as without the native driver, the animation will always run a frame behind the gesture due to the async nature of React Native.

Just to be totally clear: the bug I am reporting is that useNativeDriver cannot currently be used with any prop expecting a function, including onScroll.

Also: thanks for the interactable recommendation 🙂 – unfortunately it currently has several bugs related to scrolling which as I say is specifically my use case.

@DragonRaider5

This comment has been minimized.

Show comment
Hide comment
@DragonRaider5

DragonRaider5 Oct 10, 2017

Hi,
are you using an Animated.ScrollView?

DragonRaider5 commented Oct 10, 2017

Hi,
are you using an Animated.ScrollView?

@benadamstyles

This comment has been minimized.

Show comment
Hide comment
@benadamstyles

benadamstyles Oct 10, 2017

Amazing, that fixed it! Thanks so much. I had no idea I had to use an Animated component on the source of the event as well as the target of the animation (I am not animating the ScrollView itself).

I think maybe this should be highlighted in the docs? The docs currently say "Only animatable components can be animated", they don't mention that the source of onScroll events should also be Animated.

EDIT: I've just found a mention of using Animated.ScrollView here. I'll close this, but I do think the requirement to use Animated.ScrollView when using onScroll with useNativeDriver could be made a lot clearer. Thanks for your help @DragonRaider5 !

benadamstyles commented Oct 10, 2017

Amazing, that fixed it! Thanks so much. I had no idea I had to use an Animated component on the source of the event as well as the target of the animation (I am not animating the ScrollView itself).

I think maybe this should be highlighted in the docs? The docs currently say "Only animatable components can be animated", they don't mention that the source of onScroll events should also be Animated.

EDIT: I've just found a mention of using Animated.ScrollView here. I'll close this, but I do think the requirement to use Animated.ScrollView when using onScroll with useNativeDriver could be made a lot clearer. Thanks for your help @DragonRaider5 !

@DragonRaider5

This comment has been minimized.

Show comment
Hide comment
@DragonRaider5

DragonRaider5 Oct 10, 2017

You're welcome 👍 it already says so in the docs, look closer at the given example - but you're right, it's a tiny detail easy to overlook. Highlighting might help there.
Happy to have helped 😃

DragonRaider5 commented Oct 10, 2017

You're welcome 👍 it already says so in the docs, look closer at the given example - but you're right, it's a tiny detail easy to overlook. Highlighting might help there.
Happy to have helped 😃

@ryanab

This comment has been minimized.

Show comment
Hide comment
@ryanab

ryanab Mar 26, 2018

Same issue same solution; thanks @DragonRaider5

ryanab commented Mar 26, 2018

Same issue same solution; thanks @DragonRaider5

@MSSPL-PiyaleeMaiti

This comment has been minimized.

Show comment
Hide comment
@MSSPL-PiyaleeMaiti

MSSPL-PiyaleeMaiti Aug 10, 2018

@DragonRaider5 If I use Flalist for animated then can I use Animated.FlatList ? Is this Animated.FlatList supported fully with others features of FlatList?

MSSPL-PiyaleeMaiti commented Aug 10, 2018

@DragonRaider5 If I use Flalist for animated then can I use Animated.FlatList ? Is this Animated.FlatList supported fully with others features of FlatList?

@facebook facebook locked as resolved and limited conversation to collaborators Oct 10, 2018

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