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

Animated.Value does not have correct value after animation if component was re-mounted #23712

Closed
karhatsu opened this issue Mar 1, 2019 · 10 comments
Labels

Comments

@karhatsu
Copy link

karhatsu commented Mar 1, 2019

🐛 Bug Report

Animated.Value's internal value stops updating during animations (e.g. Animated.timing) if the component using it in styles have been unmounted.

To Reproduce

  1. Create Animated.Value av and add a listener for it.
  2. Use av in styles for component C (e.g. translateX).
  3. Run an animation (useNativeDriver: true) with av and see with the help of the listener how the value was changed correctly.
  4. Unmount C (and all other components that utilize av in their styles).
  5. Mount C
  6. Run the same animation again and see how the value does not change (UI changes correctly though).
  7. Unmount and mount C again.
  8. C won't be positioned correctly since av's internal value reflects the state after the phase 3 instead of the phase 6.

Expected Behavior

The Animated.Value's internal value should change always to the value defined in the animation.

Code Example

https://github.com/karhatsu/react-native-animated-value-bug

Environment

React Native Environment Info:
System:
OS: macOS High Sierra 10.13.6
CPU: (4) x64 Intel(R) Core(TM) i5-4258U CPU @ 2.40GHz
Memory: 48.68 MB / 8.00 GB
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 8.9.4 - ~/.nvm/versions/node/v8.9.4/bin/node
Yarn: 1.13.0 - ~/.yarn/bin/yarn
npm: 6.7.0 - ~/.nvm/versions/node/v8.9.4/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 12.1, macOS 10.14, tvOS 12.1, watchOS 5.1
Android SDK:
API Levels: 23, 24, 25, 26, 27, 28
Build Tools: 23.0.1, 25.0.1, 25.0.2, 26.0.1, 26.0.2, 27.0.2, 27.0.3, 28.0.0, 28.0.3
System Images: android-27 | Android TV Intel x86 Atom, android-27 | Google APIs Intel x86 Atom, android-27 | Google Play Intel x86 Atom
IDEs:
Android Studio: 3.3 AI-182.5107.16.33.5264788
Xcode: 10.1/10B61 - /usr/bin/xcodebuild
npmPackages:
react: 16.5.0 => 16.5.0
react-native: https://github.com/expo/react-native/archive/sdk-32.0.0.tar.gz => 0.57.1

@karhatsu
Copy link
Author

karhatsu commented Mar 1, 2019

The workaround for this is to keep the animation styles mounted in some component all the time, as in https://github.com/karhatsu/react-native-animated-value-bug/blob/master/App.js#L36. However, it's really difficult to identify the issue so fixing the real issue would be great.

@CatapultJesse
Copy link

Created snack reproducing: https://snack.expo.io/@jkcooper/rn-issue-#23712---animated-value-remounting
Seems adding a new listener also does nothing, the whole object needs to be recreated to fix it

@karhatsu
Copy link
Author

karhatsu commented Mar 2, 2019

Created snack reproducing: https://snack.expo.io/@jkcooper/rn-issue-#23712---animated-value-remounting

The link does not seem to work.

@CatapultJesse
Copy link

Created snack reproducing: https://snack.expo.io/@jkcooper/rn-issue-#23712---animated-value-remounting

The list does not seem to work.

Yeah it seems snack has an issue with special characters in titles/urls and i can only change it if i can open it. I've sent them a message to check it out so hopefully they'll fix it and it'll be available shortly

@dulmandakh
Copy link
Contributor

@karhatsu what is the platform?

@karhatsu
Copy link
Author

@karhatsu what is the platform?

I've tested only with Android. But if this is the same as #23621, then it's also iOS.

@kelset
Copy link
Contributor

kelset commented Mar 19, 2019

I've tested your repro on both platforms in a fresh 0.59 project and it still behaves the way you described.

I don't have enough knowledge about the internals of how Animation works to be precisely secure that this is a bug and not ""the way"" it's supposed to work, so I'll add a label that will mean that we'll look more into it in the future.

@Elijen
Copy link

Elijen commented Mar 19, 2019

@kelset I'd suggest bumping the priority as the bug might cause crashes in some cases: #23620

The original issue that was closed as duplicate has a reproduction sample on Snack: #23621

@cpojer cpojer removed the Follow Up label Mar 26, 2019
@cpojer
Copy link
Contributor

cpojer commented Mar 26, 2019

Is there anybody who'd like to try sending a PR to fix this? We should probably stop the animation when something is unmounted.

@djonin
Copy link

djonin commented Feb 10, 2020

We've noticed a similar issue. Whenever the component is re-mounted, the animated value resets. It seems this is due to the fact the native node which has the up to date value is destroyed after a component is dismounted. The JS animated value node doesn't get the native value updates unless there is a listener attached, so the next time the component is mounted it uses the value as it was before the native animation. This does not happen in the react-native-animated-value-bug example, as there is a listener attached to x, if you remove it the square will reset back to its original position on step 5.

Currently the only way to preserve the value is to attach a listener, meaning the value updates will be sent over the bridge often during animations. In addition, it's not obvious to users that attaching a listener is required for correct re-mounting behavior.

One possible fix would be to make a new event to call back to JS with the value just once when the node is being detached, so that we can preserve whatever the final native value was in our JS object, and for the JS animated value node to always listen to that new event, even with no listeners attached explicitly.

The above fix #24571 only fixes the part with the listener not correctly re-binding to new value, the issue mentioned in #23621 is still there.

@facebook facebook locked as resolved and limited conversation to collaborators May 7, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label May 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants