-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
resets styles when detached on RN 0.70.0
#34665
Comments
To clarify is this an Android only issue? Ah it seems in software-mansion/react-native-gesture-handler#2208 it appears on both platforms |
Will take a look over 0.70 PRs that may have affected later today, or if anyone has leads, please share! |
Could this be due to d8c25ca? Could you try backing it out to validate? |
@lunaleaps / @javache any updates on this? |
Confirmed reverting relevant changes d8c25ca? fixes the issue and that it's only reproducible non-Fabric |
@lunaleaps is there already a "revert commit" in main? I'd rather cherry-pick that then do a revert locally in 0.70-branch |
I'm not sure we can easily revert, it fixes an issue on Fabric. I'm trying some superficial attempts at fixing forward as the author is not in office right now. @javache if you have more ideas? |
#34927 attempts to resolve this issue by enabling the new behaviour only on Fabric. I verified this resolves the repro given here, but would appreciate help testing other scenarios too (eg the drawerlayout issue) |
Summary: Pull Request resolved: #34927 The changes made in D36902220 (a041951) and D36958882 (d8c25ca) attempted to reduce flickering and consistency issues when using Animated. In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if `initialProps` are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly `restoreDefaultValues` on detaching the animated node, and instead rely on the latest state being correct(?). Changelog: [General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer Fixes #34665 Reviewed By: rshest Differential Revision: D40194072 fbshipit-source-id: 1b3fb1d1f4a39036a501a8a21e57002035dd5659 # Conflicts: # Libraries/Animated/createAnimatedComponent.js
@javache Thank you for fixing this. Your PR seems to be fixing the drawer layout issue. |
@javache @lunaleaps Sorry for the ping, but I've missed a case where an ongoing animation gets interrupted and a new one starts from the position where the old one got interrupted. Here's how it looks: device-2022-12-07-112210.mp4If I let the animation finish it behaves correctly, however, if I tap the button twice, the view snaps to the edge of the screen instead of staying where the animation finished. Here's the updated reproductionimport React, { useRef, useState } from 'react';
import { View, Animated, Button } from 'react-native';
export default function App() {
const value = useRef(new Animated.Value(0)).current;
const isMoved = useRef(false);
const [useAnimatedValue, setUseAnimatedValue] = useState(false);
function animate() {
setUseAnimatedValue(true);
isMoved.current = !isMoved.current;
Animated.spring(value, {
toValue: isMoved.current ? 1 : 0,
bounciness: 3000,
velocity: 0.5,
useNativeDriver: true,
}).start(({ finished }) => {
if (finished) {
setUseAnimatedValue(false);
}
});
}
const offset = useAnimatedValue
? value.interpolate({
inputRange: [0, 1],
outputRange: [0, 250],
extrapolate: 'clamp',
})
: isMoved.current
? 250
: 0;
return (
<View style={{ flex: 1 }}>
<Animated.View
style={{
width: 100,
height: 100,
backgroundColor: 'red',
transform: [{ translateX: offset }],
}}
/>
<Button
title="Animate"
onPress={() => {
animate();
}}
/>
</View>
);
} |
I'll open the issue again, @javache can you give this a look if you have a moment? |
Trying to clarify the urgency of this -- @j-piasecki, @kelset this is still breaking DrawerLayout for 0.70 re: software-mansion/react-native-gesture-handler#2208? cc @genkikondo |
@lunaleaps Yes, it's still breaking |
Does this apply to RN latest? We've landed 5cdf3cf last month which switches out the implementation of Animated to be concurrent-safe and may have different behaviour. |
If by latest you mean |
yeah I think @javache means 0.71 - good to hear that it works there, I wonder if we need to attempt to cherry-pick that commit on 0.70 |
I don't exactly remember now, but it seems like I've checked only the new architecture (where the behavior is correct), but on the old architecture it's still broken. I'm really sorry for the confusion. |
on which version of RN did you do that test? 0.70 or 0.71? can you try with 0.71.0? We released it yesterday |
I tried it on 0.71.0, and while it works as expected on the new architecture, on the old arch the layout jump is still visible. This applies to both, Android and iOS. |
Summary: Pull Request resolved: facebook#34927 The changes made in D36902220 (facebook@a041951) and D36958882 (facebook@d8c25ca) attempted to reduce flickering and consistency issues when using Animated. In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if `initialProps` are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly `restoreDefaultValues` on detaching the animated node, and instead rely on the latest state being correct(?). Changelog: [General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer Fixes facebook#34665 Reviewed By: rshest Differential Revision: D40194072 fbshipit-source-id: 1b3fb1d1f4a39036a501a8a21e57002035dd5659
This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
This issue was closed because it has been stalled for 7 days with no activity. |
It still exists on |
Description
When
Animated.Value
in styles gets replaced by a constant value, theAnimated.Node
gets correctly detached, however, the new value is not assigned - the prop gets restored to its default value.It only happens when using native drivers, when running animation with
useNativeDriver: false
it works correctly.Behavior on React Native 0.70.0:
rn70.webm
Correct behavior on React Native 0.69.5:
rn69.webm
Related issue: software-mansion/react-native-gesture-handler#2208
Version
0.70.0
Output of
npx react-native info
Steps to reproduce
Run the code below and press the
Animate
button. When the animation ends the view snaps to the edge of the screen instead of staying in place.Snack, code example, screenshot, or link to a repository
The text was updated successfully, but these errors were encountered: