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

Native Animated - Override __makeNative in AnimatedInterpolation #15077

Closed
wants to merge 1 commit into from

Conversation

syaau
Copy link
Contributor

@syaau syaau commented Jul 18, 2017

Summary:
Fixes the error Trying to update interpolation node that has not been attached to the parent in android which occurs when using multiple
Animated.Values along with interpolation and an animation is run before
another one that uses interpolation. On ios, no error is thrown in such
case but the animation also doesn't work as expected.

You can check the snack code here which works properly without
useNativeDriver: true. But fails on android and skips the first stage
of animation on ios.
https://snack.expo.io/HyD3zdjSZ

Test Plan
The animations worked properly after the __makeNative override made
the parent node native as well.

Summary:
Fixes the error `Trying to update interpolation node that has not been
attached to the parent` in android which occurs when using multiple
Animated.Values along with interpolation and an animation is run before
another one that uses interpolation. On ios, no error is thrown in such
case but the animation also doesn't work as expected.

You can check the snack code here which works properly without
useNativeDriver: true. But fails on android and skips the first stage
of animation on ios.
  https://snack.expo.io/HyD3zdjSZ

**Test Plan**
The animations worked properly after the __makeNative override made
the parent node native as well.
@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 18, 2017
@janicduplessis
Copy link
Contributor

Thanks for the fix! Looks good.

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Jul 19, 2017
@facebook-github-bot
Copy link
Contributor

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@Noitidart
Copy link

Wow so timely, I just ran into this bug today. What version does this land in?

@syaau
Copy link
Contributor Author

syaau commented Aug 10, 2017

Most likely 0.48. A quick hack into the node_modules/react-native/Libraries/Animated/src/AnimatedImplementation.js file can fix this issue if you are in a hurry.

@Noitidart
Copy link

@syaau thanks! Yep that's how I'm doing it. Very cool thanks!

@Noitidart
Copy link

Hey all, I was wondering if I'm right here, this PR landed in v0.49.1? Is this correct?

@janicduplessis
Copy link
Contributor

janicduplessis commented Oct 7, 2017

@Noitidart Yes, you can check the tags on the commit to know in which release it is in.

image

@Noitidart
Copy link

Awesome thank you sir!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants