-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
AnimatedImplementation: Make sure we don't attempt to start an animation with index above array-length. #17847
Conversation
…ion with index above array-length. fixes facebook#17699
@facebook-github-bot label Android Generated by 🚫 dangerJS |
@runar-rkmedia Do you know why the index is out of range (maybe a repro example)? Would love to understand the cause of the bug before merging this fix. |
@janicduplessis First of all, thank you for looking into this. I'm quite sorry, but I'm not quite sure what is causing this. In the linked issue #17699 there is a reproducible minimal example, but I fail to find the exact reason for this happening. |
This comment has been minimized.
This comment has been minimized.
It seems like this is an issue that only the original author @runar-rkmedia ran into and all of us, including the author of the PR, are unsure about what the issue is and how it happens. I'm not excited about merging a change to React Native where we have such little confidence so I'm gonna close this PR. I'm happy to reopen and merge this with proper explanations and verification, though. |
I was running this issue as well. I think the problem occurs when starting an animation sequence without resetting the animation. Here's the implementation of
The variable |
Sometimes, the index for the animations-array will be outside the range of the array, so we need a nullcheck before attempting to do
animations[current].start(onComplete);
fixes #17699
Motivation
Fix the bug.
Test Plan
Tested it in application, simple fix so should not have any side-effects.
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)
Release Notes
[Android] [Fixes] - AnimatedImplementation : Make sure we don't attempt to start an animation with index above array-length.