-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
Make animatedStyle keep reference through renders #5333
Conversation
dc8bf2c
to
cb46d6a
Compare
I changed how we compare animated styles in animated components (since with current implementation I believe it was faulty, but that's another topic). I also created a test-suite for this change and everything passes. Cases that I test:
In various configurations. Test suite in diff format for `git apply`diff --git a/app/src/examples/EmptyExample.tsx b/app/src/examples/EmptyExample.tsx
index 19a20bdce..bbc803f8d 100644
--- a/app/src/examples/EmptyExample.tsx
+++ b/app/src/examples/EmptyExample.tsx
@@ -1,19 +1,45 @@
-import { Text, StyleSheet, View } from 'react-native';
-
-import React from 'react';
-
+// 1. Retains same reference on re-render without register.
+import ReferenceRetainNoRegister from './Testing/ReferenceRetainNoRegister';
export default function EmptyExample() {
- return (
- <View style={styles.container}>
- <Text>Hello world!</Text>
- </View>
- );
+ return ReferenceRetainNoRegister();
}
-const styles = StyleSheet.create({
- container: {
- flex: 1,
- alignItems: 'center',
- justifyContent: 'center',
- },
-});
+// 2. Retains same reference on re-render with register.
+// import ReferenceRetainRegister from './Testing/ReferenceRetainRegister';
+// export default function EmptyExample() {
+// return ReferenceRetainRegister();
+// }
+
+// 3. Retains same reference with parent and child on:
+// 3.1 re-render,
+// 3.2 hot-reloading parent,
+// 3.3 hot-reloading child.
+// import ParentReferenceRetain from './Testing/ParentReferenceRetain';
+// export default function EmptyExample() {
+// return ParentReferenceRetain();
+// }
+
+// 4. Keeps animating with single component on:
+// 4.1 re-render,
+// 4.2 hot-reloading.
+// import AnimationRetainSingleComponent from './Testing/AnimationRetainSingleComponent';
+// export default function EmptyExample() {
+// return AnimationRetainSingleComponent();
+// }
+
+// 5. Keeps animating with multiple components on:
+// 5.1 re-render,
+// 5.2 hot-reloading.
+// import AnimationRetainMultipleComponents from './Testing/AnimationRetainMultipleComponents';
+// export default function EmptyExample() {
+// return AnimationRetainMultipleComponents();
+// }
+
+// 6. Keeps animating with parent and child on:
+// 6.1 re-render,
+// 6.2 hot-reloading parent,
+// 6.3 hot-reloading child.
+// import ParentAnimationRetain from './Testing/ParentAnimationRetain';
+// export default function EmptyExample() {
+// return ParentAnimationRetain();
+// }
diff --git a/app/src/examples/Testing/AnimationRetainMultipleComponents.tsx b/app/src/examples/Testing/AnimationRetainMultipleComponents.tsx
new file mode 100644
index 000000000..63e9e9c30
--- /dev/null
+++ b/app/src/examples/Testing/AnimationRetainMultipleComponents.tsx
@@ -0,0 +1,45 @@
+import { StyleSheet, View, Button } from 'react-native';
+
+import Animated, {
+ useAnimatedStyle,
+ useSharedValue,
+ withTiming,
+} from 'react-native-reanimated';
+
+import React from 'react';
+
+export default function Component() {
+ const [state, setState] = React.useState(0);
+ const width = useSharedValue(100);
+ const animatedStyle = useAnimatedStyle(() => ({
+ height: 100,
+ width: width.value,
+ }));
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <View style={styles.container}>
+ <Animated.View style={[animatedStyle, { backgroundColor: 'blue' }]} />
+ <Animated.View
+ style={[animatedStyle, { backgroundColor: 'aquamarine' }]}
+ />
+ <Button
+ title="Animate"
+ onPress={() => {
+ width.value = withTiming(width.value + 100, { duration: 8000 });
+ }}
+ />
+ <Button title="Re-render" onPress={() => setState(state + 1)} />
+ </View>
+ );
+}
+
+const styles = StyleSheet.create({
+ container: {
+ flex: 1,
+ alignItems: 'center',
+ justifyContent: 'center',
+ },
+});
diff --git a/app/src/examples/Testing/AnimationRetainSingleComponent.tsx b/app/src/examples/Testing/AnimationRetainSingleComponent.tsx
new file mode 100644
index 000000000..6eca52493
--- /dev/null
+++ b/app/src/examples/Testing/AnimationRetainSingleComponent.tsx
@@ -0,0 +1,43 @@
+import { StyleSheet, View, Button } from 'react-native';
+
+import Animated, {
+ useAnimatedStyle,
+ useSharedValue,
+ withTiming,
+} from 'react-native-reanimated';
+
+import React from 'react';
+
+export default function Component() {
+ const [state, setState] = React.useState(0);
+ const width = useSharedValue(100);
+ const animatedStyle = useAnimatedStyle(() => ({
+ height: 100,
+ backgroundColor: 'blue',
+ width: width.value,
+ }));
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <View style={styles.container}>
+ <Animated.View style={animatedStyle} />
+ <Button
+ title="Animate"
+ onPress={() => {
+ width.value = withTiming(width.value + 100, { duration: 8000 });
+ }}
+ />
+ <Button title="Re-render" onPress={() => setState(state + 1)} />
+ </View>
+ );
+}
+
+const styles = StyleSheet.create({
+ container: {
+ flex: 1,
+ alignItems: 'center',
+ justifyContent: 'center',
+ },
+});
diff --git a/app/src/examples/Testing/ChildAnimationRetain.tsx b/app/src/examples/Testing/ChildAnimationRetain.tsx
new file mode 100644
index 000000000..9c2041444
--- /dev/null
+++ b/app/src/examples/Testing/ChildAnimationRetain.tsx
@@ -0,0 +1,22 @@
+import { Button } from 'react-native';
+
+import Animated from 'react-native-reanimated';
+
+import React from 'react';
+
+export default function Component({ animatedStyle }) {
+ const [state, setState] = React.useState(0);
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Child pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <>
+ <Animated.View style={[animatedStyle, { backgroundColor: 'blue' }]} />
+ <Animated.View
+ style={[animatedStyle, { backgroundColor: 'aquamarine' }]}
+ />
+ <Button title="Re-render child" onPress={() => setState(state + 1)} />
+ </>
+ );
+}
diff --git a/app/src/examples/Testing/ChildReferenceRetain.tsx b/app/src/examples/Testing/ChildReferenceRetain.tsx
new file mode 100644
index 000000000..9c2041444
--- /dev/null
+++ b/app/src/examples/Testing/ChildReferenceRetain.tsx
@@ -0,0 +1,22 @@
+import { Button } from 'react-native';
+
+import Animated from 'react-native-reanimated';
+
+import React from 'react';
+
+export default function Component({ animatedStyle }) {
+ const [state, setState] = React.useState(0);
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Child pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <>
+ <Animated.View style={[animatedStyle, { backgroundColor: 'blue' }]} />
+ <Animated.View
+ style={[animatedStyle, { backgroundColor: 'aquamarine' }]}
+ />
+ <Button title="Re-render child" onPress={() => setState(state + 1)} />
+ </>
+ );
+}
diff --git a/app/src/examples/Testing/ParentAnimationRetain.tsx b/app/src/examples/Testing/ParentAnimationRetain.tsx
new file mode 100644
index 000000000..504242c49
--- /dev/null
+++ b/app/src/examples/Testing/ParentAnimationRetain.tsx
@@ -0,0 +1,41 @@
+import { StyleSheet, View, Button } from 'react-native';
+import {
+ useAnimatedStyle,
+ useSharedValue,
+ withTiming,
+} from 'react-native-reanimated';
+import React from 'react';
+import ChildAnimationRetain from './ChildAnimationRetain';
+
+export default function Component() {
+ const [state, setState] = React.useState(0);
+ const width = useSharedValue(100);
+ const animatedStyle = useAnimatedStyle(() => ({
+ height: 100,
+ width: width.value,
+ }));
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Parent pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <View style={styles.container}>
+ <ChildAnimationRetain animatedStyle={animatedStyle} />
+ <Button
+ title="Animate"
+ onPress={() => {
+ width.value = withTiming(width.value + 100, { duration: 8000 });
+ }}
+ />
+ <Button title="Re-render parent" onPress={() => setState(state + 1)} />
+ </View>
+ );
+}
+
+const styles = StyleSheet.create({
+ container: {
+ flex: 1,
+ alignItems: 'center',
+ justifyContent: 'center',
+ },
+});
diff --git a/app/src/examples/Testing/ParentReferenceRetain.tsx b/app/src/examples/Testing/ParentReferenceRetain.tsx
new file mode 100644
index 000000000..a6e3e58b9
--- /dev/null
+++ b/app/src/examples/Testing/ParentReferenceRetain.tsx
@@ -0,0 +1,33 @@
+import { StyleSheet, View, Button } from 'react-native';
+import { useSharedValue, useAnimatedStyle } from 'react-native-reanimated';
+import ChildReferenceRetain from './ChildReferenceRetain';
+
+import React from 'react';
+
+export default function Parent() {
+ const [state, setState] = React.useState(0);
+ const width = useSharedValue(100);
+ const animatedStyle = useAnimatedStyle(() => ({
+ height: 100,
+ backgroundColor: 'blue',
+ width: width.value,
+ }));
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Parent pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <View style={styles.container}>
+ <ChildReferenceRetain animatedStyle={animatedStyle} />
+ <Button title="Re-render parent" onPress={() => setState(state + 1)} />
+ </View>
+ );
+}
+
+const styles = StyleSheet.create({
+ container: {
+ flex: 1,
+ alignItems: 'center',
+ justifyContent: 'center',
+ },
+});
diff --git a/app/src/examples/Testing/ReferenceRetainNoRegister.tsx b/app/src/examples/Testing/ReferenceRetainNoRegister.tsx
new file mode 100644
index 000000000..36b4fbe82
--- /dev/null
+++ b/app/src/examples/Testing/ReferenceRetainNoRegister.tsx
@@ -0,0 +1,27 @@
+import { StyleSheet, View, Button } from 'react-native';
+
+import { useAnimatedStyle } from 'react-native-reanimated';
+
+import React from 'react';
+
+export default function Component() {
+ const [state, setState] = React.useState(0);
+ const animatedStyle = useAnimatedStyle(() => ({}));
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <View style={styles.container}>
+ <Button title="Re-render" onPress={() => setState(state + 1)} />
+ </View>
+ );
+}
+
+const styles = StyleSheet.create({
+ container: {
+ flex: 1,
+ alignItems: 'center',
+ justifyContent: 'center',
+ },
+});
diff --git a/app/src/examples/Testing/ReferenceRetainRegister.tsx b/app/src/examples/Testing/ReferenceRetainRegister.tsx
new file mode 100644
index 000000000..7bbcacf16
--- /dev/null
+++ b/app/src/examples/Testing/ReferenceRetainRegister.tsx
@@ -0,0 +1,28 @@
+import { StyleSheet, View, Button } from 'react-native';
+
+import Animated, { useAnimatedStyle } from 'react-native-reanimated';
+
+import React from 'react';
+
+export default function Component() {
+ const [state, setState] = React.useState(0);
+ const animatedStyle = useAnimatedStyle(() => ({}));
+ const animatedStyleRef = React.useRef(animatedStyle);
+
+ console.log(`Pass: ${animatedStyle === animatedStyleRef.current}`);
+
+ return (
+ <View style={styles.container}>
+ <Animated.View style={animatedStyle} />
+ <Button title="Re-render" onPress={() => setState(state + 1)} />
+ </View>
+ );
+}
+
+const styles = StyleSheet.create({
+ container: {
+ flex: 1,
+ alignItems: 'center',
+ justifyContent: 'center',
+ },
+});
diff --git a/src/createAnimatedComponent/createAnimatedComponent.tsx b/src/createAnimatedComponent/createAnimatedComponent.tsx
index 0afc17547..be4cc0c37 100644
--- a/src/createAnimatedComponent/createAnimatedComponent.tsx
+++ b/src/createAnimatedComponent/createAnimatedComponent.tsx
@@ -67,17 +67,6 @@ function onlyAnimatedStyles(styles: StyleProps[]): StyleProps[] {
return styles.filter((style) => style?.viewDescriptors);
}
-function isSameAnimatedStyle(
- style1?: StyleProps,
- style2?: StyleProps
-): boolean {
- // We cannot use equality check to compare useAnimatedStyle outputs directly.
- // Instead, we can compare its viewsRefs.
- return style1?.viewsRef === style2?.viewsRef;
-}
-
-const isSameAnimatedProps = isSameAnimatedStyle;
-
type Options<P> = {
setNativeProps: (ref: AnimatedComponentRef, props: P) => void;
};
@@ -401,14 +390,12 @@ export function createAnimatedComponent(
const hasOneSameStyle =
styles.length === 1 &&
prevStyles.length === 1 &&
- isSameAnimatedStyle(styles[0], prevStyles[0]);
+ styles[0] === prevStyles[0];
if (!hasOneSameStyle) {
// otherwise, remove each style that is not present in new styles
for (const prevStyle of prevStyles) {
- const isPresent = styles.some((style) =>
- isSameAnimatedStyle(style, prevStyle)
- );
+ const isPresent = styles.some((style) => style === prevStyle);
if (!isPresent) {
prevStyle.viewDescriptors.remove(viewTag);
}
@@ -438,10 +425,7 @@ export function createAnimatedComponent(
});
// detach old animatedProps
- if (
- prevAnimatedProps &&
- !isSameAnimatedProps(prevAnimatedProps, this.props.animatedProps)
- ) {
+ if (prevAnimatedProps && prevAnimatedProps !== this.props.animatedProps) {
prevAnimatedProps.viewDescriptors!.remove(viewTag as number);
}
diff --git a/src/reanimated2/hook/useAnimatedStyle.ts b/src/reanimated2/hook/useAnimatedStyle.ts
index d2a27fe7c..c6c5ee925 100644
--- a/src/reanimated2/hook/useAnimatedStyle.ts
+++ b/src/reanimated2/hook/useAnimatedStyle.ts
@@ -532,8 +532,15 @@ For more, see the docs: \`https://docs.swmansion.com/react-native-reanimated/doc
checkSharedValueUsage(initial.value);
- const style = isJest()
- ? { viewDescriptors, initial, viewsRef, jestAnimatedStyle }
- : { initial, viewsRef, viewDescriptors };
- return useRef(style).current;
+ const animatedStyleHandle = useRef<
+ AnimatedStyleHandle<Style> | JestAnimatedStyleHandle<Style> | null
+ >(null);
+
+ if (!animatedStyleHandle.current) {
+ animatedStyleHandle.current = isJest()
+ ? { viewDescriptors, initial, viewsRef, jestAnimatedStyle }
+ : { initial, viewsRef, viewDescriptors };
+ }
+
+ return animatedStyleHandle.current;
}
|
@ranaavneet I haven't been commenting since I think it's better to just do it one day instead of assuring that "it will be done sometime in the future". Anticipating your question, I don't see any reason not to include it in release of 3.7.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice anticipation! 😜 |
Hey @tjzel! Quick questions.
|
@ranaavneet Not sure what's your setup but just wanted to remind you that you can also install Reanimated from a specific commit or use a nightly release from npm. |
Hey @tomekzaw, yes, I am aware of the nightlies. I am actually still using 3.6.1, because I wasn't sure whether passing Is it possible if your team has benchmarked both these different configurations and have any findings? @tjzel mentioned that #4300 performed better than |
@ranaavneet I see now, thanks. Nope, we don't have any performance comparisions between |
|
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please follow the template so that the reviewers can easily understand what the code changes affect. --> ## Summary Closes #1767 Some of our hooks (`useSharedValue`, `useDerivedValue`) return immutable refs. ```javascript export function useSharedValue<Value>( init: Value, oneWayReadsOnly = false ): SharedValue<Value> { const ref = useRef<SharedValue<Value>>(makeMutable(init, oneWayReadsOnly)); // code return ref.current; } ``` However `useAnimatedStyle` return an object. Therefore animated style is getting a new reference each render. This issue was noticed here: #1767 ## Test plan <details><summary>code</summary> ```javascript /* eslint-disable no-inline-styles/no-inline-styles */ import Animated, { useSharedValue, useAnimatedStyle, useDerivedValue, withSpring, } from 'react-native-reanimated'; import { Button, View } from 'react-native'; import React, { useEffect, useState } from 'react'; export default function AnimatedStyleUpdateExample() { const [, setCounter] = useState(0); const width = useSharedValue(10); const derivedValue = useDerivedValue(() => { return width.value + 10; }); const style = useAnimatedStyle(() => { return { width: width.value, }; }); useEffect(() => { setInterval(() => { setCounter((counterVal) => counterVal + 1); }, 2000); }, []); console.log('component re-render'); useEffect(() => { console.log('width changed identity'); }, [width]); useEffect(() => { console.log('derivedValue changed identity'); }, [derivedValue]); useEffect(() => { console.log('style changed identity'); }, [style]); return ( <View style={{ flex: 1, flexDirection: 'column', }}> <Animated.View style={[style, { height: 80, backgroundColor: 'black', margin: 30 }]} /> <Button title="TOGGLE" onPress={() => { width.value = withSpring(Math.random() * 100); }} /> </View> ); } ``` <details> I've tested that animations still work as desired: <summary><details>Recording</details> https://github.com/software-mansion/react-native-reanimated/assets/56199675/d2180c2d-b5a6-4736-ad94-1d245b3a5407 </summary> <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. --> --------- Co-authored-by: Aleksandra Cynk <[email protected]> Co-authored-by: Aleksandra Cynk <[email protected]> Co-authored-by: Tomasz Żelawski <[email protected]>
Summary
Closes #1767
Some of our hooks (
useSharedValue
,useDerivedValue
) return immutable refs.However
useAnimatedStyle
return an object. Therefore animated style is getting a new reference each render. This issue was noticed here: #1767Test plan
code
I've tested that animations still work as desired:
Screen.Recording.2023-10-31.at.12.02.26.mov