From 2efe8094c0d4251213b5305ddaba11aaf76bfd56 Mon Sep 17 00:00:00 2001 From: Martin Booth Date: Tue, 29 Apr 2025 17:03:44 -0700 Subject: [PATCH] Include offset as separate field in payloads (#50970) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/50970 __onAnimatedValueUpdateReceived is called from 2 places. It is called from the onAnimatedValueUpdate subscription where the source of the values comes from here: https://fburl.com/8v2cwd2x with getValue() returning the offset + value components combined. It's also called from in the animation end callback here: https://fburl.com/h36xy2nw where the source of this value comes from https://fburl.com/sud7m7st. In this case it's accessing `nodeValue` directly rather than calling getValue() and so it only includes the value component. In this diff we pass both the value and offset in both onAnimatedValueUpdate callbacks as well as endCallback. This allows us to separate the value from the offset on the JS side and ensures we have the latest offset value from native Changelog: [Android][Fixed] - Sync offset and value from native -> js in separate fields Reviewed By: zeyap Differential Revision: D73795619 fbshipit-source-id: e8ed234497e3fcaf9d2a137aa1e17ca8d0f76d97 --- .../Animated/__tests__/AnimatedValue-test.js | 39 ++++++++++++++++--- .../Animated/animations/Animation.js | 11 ++++-- .../Libraries/Animated/nodes/AnimatedNode.js | 4 +- .../Libraries/Animated/nodes/AnimatedValue.js | 7 +++- .../__snapshots__/public-api-test.js.snap | 7 +++- .../ReactAndroid/api/ReactAndroid.api | 2 +- .../animated/AnimatedNodeValueListener.kt | 2 +- .../react/animated/NativeAnimatedModule.kt | 6 ++- .../animated/NativeAnimatedNodesManager.kt | 6 +++ .../react/animated/ValueAnimatedNode.kt | 2 +- .../NativeAnimatedNodeTraversalTest.kt | 4 +- .../modules/NativeAnimatedModule.js | 2 +- .../modules/NativeAnimatedTurboModule.js | 2 +- 13 files changed, 72 insertions(+), 22 deletions(-) diff --git a/packages/react-native/Libraries/Animated/__tests__/AnimatedValue-test.js b/packages/react-native/Libraries/Animated/__tests__/AnimatedValue-test.js index 8e08a3ccd8b..866fb557f01 100644 --- a/packages/react-native/Libraries/Animated/__tests__/AnimatedValue-test.js +++ b/packages/react-native/Libraries/Animated/__tests__/AnimatedValue-test.js @@ -16,13 +16,18 @@ describe('AnimatedValue', () => { return new AnimatedValue(0, {useNativeDriver: true}); } - function emitMockUpdate(node: AnimatedValue, mockValue: number): void { + function emitMockUpdate( + node: AnimatedValue, + mockValue: number, + mockOffset: number, + ): void { const nativeTag = node.__nativeTag; expect(nativeTag).not.toBe(undefined); NativeAnimatedHelper.nativeEventEmitter.emit('onAnimatedValueUpdate', { tag: nativeTag, value: mockValue, + offset: mockOffset, }); } @@ -63,12 +68,12 @@ describe('AnimatedValue', () => { const nativeTag = node.__nativeTag; expect(nativeTag).not.toBe(undefined); - emitMockUpdate(node, 123); + emitMockUpdate(node, 123, 50); expect(callback).toBeCalledTimes(1); node.removeListener(id); - emitMockUpdate(node, 456); + emitMockUpdate(node, 456, 60); expect(callback).toBeCalledTimes(1); }); @@ -102,7 +107,7 @@ describe('AnimatedValue', () => { node.__attach(); node.addListener(callbackA); - emitMockUpdate(node, 123); + emitMockUpdate(node, 123, 50); expect(callbackA).toBeCalledTimes(1); node.__detach(); @@ -112,7 +117,7 @@ describe('AnimatedValue', () => { node.__attach(); node.addListener(callbackB); - emitMockUpdate(node, 456); + emitMockUpdate(node, 456, 60); expect(callbackA).toBeCalledTimes(1); expect(callbackB).toBeCalledTimes(1); }); @@ -199,4 +204,28 @@ describe('AnimatedValue', () => { ).toBeCalledTimes(0); }); }); + + describe('when receiving an update event', () => { + it('calls __onAnimatedValueUpdateReceived with value and offset', () => { + const callback = jest.fn(); + const node = createNativeAnimatedValue(); + node.__attach(); + node.addListener(callback); + + const nativeTag = node.__nativeTag; + expect(nativeTag).not.toBe(undefined); + + emitMockUpdate(node, 123, 50); + + const spy = jest.spyOn(node, '__onAnimatedValueUpdateReceived'); + + const mockValue = 100; + const mockOffset = 50; + + emitMockUpdate(node, mockValue, mockOffset); + + expect(spy).toHaveBeenCalledWith(mockValue, mockOffset); + spy.mockRestore(); + }); + }); }); diff --git a/packages/react-native/Libraries/Animated/animations/Animation.js b/packages/react-native/Libraries/Animated/animations/Animation.js index 7af8f303789..6587e9189fd 100644 --- a/packages/react-native/Libraries/Animated/animations/Animation.js +++ b/packages/react-native/Libraries/Animated/animations/Animation.js @@ -15,7 +15,12 @@ import type AnimatedValue from '../nodes/AnimatedValue'; import NativeAnimatedHelper from '../../../src/private/animated/NativeAnimatedHelper'; import AnimatedProps from '../nodes/AnimatedProps'; -export type EndResult = {finished: boolean, value?: number, ...}; +export type EndResult = { + finished: boolean, + value?: number, + offset?: number, + ... +}; export type EndCallback = (result: EndResult) => void; export type AnimationConfig = $ReadOnly<{ @@ -140,9 +145,9 @@ export default class Animation { // When using natively driven animations, once the animation completes, // we need to ensure that the JS side nodes are synced with the updated // values. - const {value} = result; + const {value, offset} = result; if (value != null) { - animatedValue.__onAnimatedValueUpdateReceived(value); + animatedValue.__onAnimatedValueUpdateReceived(value, offset); if (this.__isLooping === true) { return; diff --git a/packages/react-native/Libraries/Animated/nodes/AnimatedNode.js b/packages/react-native/Libraries/Animated/nodes/AnimatedNode.js index a851bf40eaf..8f0b1b9078f 100644 --- a/packages/react-native/Libraries/Animated/nodes/AnimatedNode.js +++ b/packages/react-native/Libraries/Animated/nodes/AnimatedNode.js @@ -112,8 +112,8 @@ export default class AnimatedNode { return this.#listeners.size > 0; } - __onAnimatedValueUpdateReceived(value: number): void { - this.__callListeners(value); + __onAnimatedValueUpdateReceived(value: number, offset: number): void { + this.__callListeners(value + offset); } __callListeners(value: number): void { diff --git a/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js b/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js index a115cb4c617..e8fb1cccbf0 100644 --- a/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js +++ b/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js @@ -168,7 +168,7 @@ export default class AnimatedValue extends AnimatedWithChildren { 'onAnimatedValueUpdate', data => { if (data.tag === nativeTag) { - this.__onAnimatedValueUpdateReceived(data.value); + this.__onAnimatedValueUpdateReceived(data.value, data.offset); } }, ); @@ -288,8 +288,11 @@ export default class AnimatedValue extends AnimatedWithChildren { } } - __onAnimatedValueUpdateReceived(value: number): void { + __onAnimatedValueUpdateReceived(value: number, offset?: number): void { this._updateValue(value, false /*flush*/); + if (offset != null) { + this._offset = offset; + } } /** diff --git a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap index bb987eef5e7..2b4978f9b47 100644 --- a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap +++ b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap @@ -417,7 +417,12 @@ declare export function fromBouncinessAndSpeed( `; exports[`public API should not change unintentionally Libraries/Animated/animations/Animation.js 1`] = ` -"export type EndResult = { finished: boolean, value?: number, ... }; +"export type EndResult = { + finished: boolean, + value?: number, + offset?: number, + ... +}; export type EndCallback = (result: EndResult) => void; export type AnimationConfig = $ReadOnly<{ isInteraction?: boolean, diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 6e57f9ae6c0..95887143f0d 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -444,7 +444,7 @@ public abstract class com/facebook/react/animated/AnimatedNode { } public abstract interface class com/facebook/react/animated/AnimatedNodeValueListener { - public abstract fun onValueUpdate (D)V + public abstract fun onValueUpdate (DD)V } public final class com/facebook/react/animated/NativeAnimatedModule : com/facebook/fbreact/specs/NativeAnimatedModuleSpec, com/facebook/react/bridge/LifecycleEventListener, com/facebook/react/bridge/UIManagerListener { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/AnimatedNodeValueListener.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/AnimatedNodeValueListener.kt index 234f1ce01a8..ace96289546 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/AnimatedNodeValueListener.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/AnimatedNodeValueListener.kt @@ -9,5 +9,5 @@ package com.facebook.react.animated /** Interface used to listen to [ValueAnimatedNode] updates. */ public fun interface AnimatedNodeValueListener { - public fun onValueUpdate(value: Double) + public fun onValueUpdate(value: Double, offset: Double) } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt index 5512e7db59b..c3e6d6d7775 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.kt @@ -542,10 +542,11 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext?) : FLog.d(NAME, "queue startListeningToAnimatedNodeValue: $tag") } - val listener = AnimatedNodeValueListener { value -> + val listener = AnimatedNodeValueListener { value, offset -> val onAnimatedValueData = Arguments.createMap() onAnimatedValueData.putInt("tag", tag) onAnimatedValueData.putDouble("value", value) + onAnimatedValueData.putDouble("offset", offset) val reactApplicationContext = reactApplicationContextIfActiveOrWarn reactApplicationContext?.emitDeviceEvent("onAnimatedValueUpdate", onAnimatedValueData) @@ -976,10 +977,11 @@ public class NativeAnimatedModule(reactContext: ReactApplicationContext?) : BatchExecutionOpCodes.OP_START_LISTENING_TO_ANIMATED_NODE_VALUE -> { val tag = opsAndArgs.getInt(i++) - val listener = AnimatedNodeValueListener { value -> + val listener = AnimatedNodeValueListener { value, offset -> val onAnimatedValueData = Arguments.createMap() onAnimatedValueData.putInt("tag", tag) onAnimatedValueData.putDouble("value", value) + onAnimatedValueData.putDouble("offset", offset) val reactApplicationContext = reactApplicationContextIfActiveOrWarn reactApplicationContext?.emitDeviceEvent( diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt index 24c9ccbe6be..155cd6292a4 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.kt @@ -272,6 +272,7 @@ public class NativeAnimatedNodesManager( val endCallbackResponse = Arguments.createMap() endCallbackResponse.putBoolean("finished", false) endCallbackResponse.putDouble("value", animatedValueNonnull.nodeValue) + endCallbackResponse.putDouble("offset", animatedValueNonnull.offset) animation.endCallback?.invoke(endCallbackResponse) } else if (reactApplicationContext != null) { // If no callback is passed in, this /may/ be an animation set up by the single-op @@ -281,6 +282,7 @@ public class NativeAnimatedNodesManager( params.putInt("animationId", animation.id) params.putBoolean("finished", false) params.putDouble("value", animatedValueNonnull.nodeValue) + params.putDouble("offset", animatedValueNonnull.offset) events = events ?: Arguments.createArray() events.pushMap(params) } @@ -309,6 +311,7 @@ public class NativeAnimatedNodesManager( val endCallbackResponse = Arguments.createMap() endCallbackResponse.putBoolean("finished", false) endCallbackResponse.putDouble("value", checkNotNull(animation.animatedValue).nodeValue) + endCallbackResponse.putDouble("offset", checkNotNull(animation.animatedValue).offset) checkNotNull(animation.endCallback).invoke(endCallbackResponse) } else if (reactApplicationContext != null) { // If no callback is passed in, this /may/ be an animation set up by the single-op @@ -318,6 +321,7 @@ public class NativeAnimatedNodesManager( params.putInt("animationId", animation.id) params.putBoolean("finished", false) params.putDouble("value", checkNotNull(animation.animatedValue).nodeValue) + params.putDouble("offset", checkNotNull(animation.animatedValue).offset) events = events ?: Arguments.createArray() events.pushMap(params) } @@ -578,6 +582,7 @@ public class NativeAnimatedNodesManager( val endCallbackResponse = Arguments.createMap() endCallbackResponse.putBoolean("finished", true) endCallbackResponse.putDouble("value", animatedValueNonnull.nodeValue) + endCallbackResponse.putDouble("offset", animatedValueNonnull.offset) animation.endCallback?.invoke(endCallbackResponse) } else if (reactApplicationContext != null) { // If no callback is passed in, this /may/ be an animation set up by the single-op @@ -587,6 +592,7 @@ public class NativeAnimatedNodesManager( params.putInt("animationId", animation.id) params.putBoolean("finished", true) params.putDouble("value", animatedValueNonnull.nodeValue) + params.putDouble("offset", animatedValueNonnull.offset) events = events ?: Arguments.createArray() events.pushMap(params) } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/ValueAnimatedNode.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/ValueAnimatedNode.kt index 635025ef621..c7f21a7a889 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/ValueAnimatedNode.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/ValueAnimatedNode.kt @@ -38,7 +38,7 @@ internal open class ValueAnimatedNode(config: ReadableMap? = null) : AnimatedNod } fun onValueUpdate(): Unit { - valueListener?.onValueUpdate(getValue()) + valueListener?.onValueUpdate(getValue() - offset, offset) } fun setValueListener(listener: AnimatedNodeValueListener?): Unit { diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.kt index 8aff7f40aaa..4f3a9e629cb 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.kt @@ -242,7 +242,7 @@ class NativeAnimatedNodeTraversalTest { animationCallback) nativeAnimatedNodesManager.runUpdates(nextFrameTime()) - verify(valueListener).onValueUpdate(eq(0.0)) + verify(valueListener).onValueUpdate(eq(0.0), eq(0.0)) nativeAnimatedNodesManager.stopListeningToAnimatedNodeValue(nodeId) @@ -271,7 +271,7 @@ class NativeAnimatedNodeTraversalTest { for (i in 0 until frames.size()) { reset(valueListener) nativeAnimatedNodesManager.runUpdates(nextFrameTime()) - verify(valueListener).onValueUpdate(eq(frames.getDouble(i))) + verify(valueListener).onValueUpdate(eq(frames.getDouble(i)), eq(0.0)) } reset(valueListener) diff --git a/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedModule.js b/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedModule.js index b7746551e61..91f64cdcdc3 100644 --- a/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedModule.js +++ b/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedModule.js @@ -13,7 +13,7 @@ import type {TurboModule} from '../../../../Libraries/TurboModule/RCTExport'; import shouldUseTurboAnimatedModule from '../../../../Libraries/Animated/shouldUseTurboAnimatedModule'; import * as TurboModuleRegistry from '../../../../Libraries/TurboModule/TurboModuleRegistry'; -type EndResult = {finished: boolean, value?: number, ...}; +type EndResult = {finished: boolean, value?: number, offset?: number, ...}; type EndCallback = (result: EndResult) => void; type SaveValueCallback = (value: number) => void; diff --git a/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedTurboModule.js b/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedTurboModule.js index 7f584685eb1..5a20ac1539d 100644 --- a/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedTurboModule.js +++ b/packages/react-native/src/private/specs_DEPRECATED/modules/NativeAnimatedTurboModule.js @@ -13,7 +13,7 @@ import type {TurboModule} from '../../../../Libraries/TurboModule/RCTExport'; import shouldUseTurboAnimatedModule from '../../../../Libraries/Animated/shouldUseTurboAnimatedModule'; import * as TurboModuleRegistry from '../../../../Libraries/TurboModule/TurboModuleRegistry'; -type EndResult = {finished: boolean, value?: number, ...}; +type EndResult = {finished: boolean, value?: number, offset?: number, ...}; type EndCallback = (result: EndResult) => void; type SaveValueCallback = (value: number) => void;