From 6c80f886ae0c7c1e81712e06f51c4f119f8a5a30 Mon Sep 17 00:00:00 2001 From: Krzysztof Magiera Date: Wed, 4 May 2016 02:47:47 -0700 Subject: [PATCH] Fix setValue to work properly on natively driven nodes Summary: This change fixes an issue with calling `setValue` for natively driven nodes. As of now an attempt to call this method from JS would trigger an error "Attempting to run JS driven animation on animated". That is because for natively animated nodes we don't allow for `setNativeProps` to be executed and method `_flush` is now responsible for triggering that call. To fix the issue we add extra flag to `_updateValue` method that indicates if we should be "flushing" updated values using `setNativeProps` and we pass an appropriate value depending on the status of the node (native/non-native). Note that in animation callback we always pass `true` - that is because natively driven animations will never call into that callback. **Test Plan** Run JS tests: `npm test Libraries/Animated/src/__tests__/AnimatedNative-test.js` Closes https://github.com/facebook/react-native/pull/7138 Differential Revision: D3257696 fb-gh-sync-id: 13ec26bc36b56b3208f4279a95532bbe60551087 fbshipit-source-id: 13ec26bc36b56b3208f4279a95532bbe60551087 --- .../Animated/src/AnimatedImplementation.js | 12 +++++++---- .../src/__tests__/AnimatedNative-test.js | 21 +++++++++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/Libraries/Animated/src/AnimatedImplementation.js b/Libraries/Animated/src/AnimatedImplementation.js index 91b7bb3b5..eb04cc8fb 100644 --- a/Libraries/Animated/src/AnimatedImplementation.js +++ b/Libraries/Animated/src/AnimatedImplementation.js @@ -652,7 +652,7 @@ class AnimatedValue extends AnimatedWithChildren { this._animation.stop(); this._animation = null; } - this._updateValue(value); + this._updateValue(value, !this.__isNative /* don't perform a flush for natively driven values */); if (this.__isNative) { NativeAnimatedAPI.setAnimatedNodeValue(this.__getNativeTag(), value); } @@ -730,7 +730,9 @@ class AnimatedValue extends AnimatedWithChildren { animation.start( this._value, (value) => { - this._updateValue(value); + // Natively driven animations will never call into that callback, therefore we can always pass `flush = true` + // to allow the updated value to propagate to native with `setNativeProps` + this._updateValue(value, true /* flush */); }, (result) => { this._animation = null; @@ -760,9 +762,11 @@ class AnimatedValue extends AnimatedWithChildren { this._tracking = tracking; } - _updateValue(value: number): void { + _updateValue(value: number, flush: bool): void { this._value = value; - _flush(this); + if (flush) { + _flush(this); + } for (var key in this._listeners) { this._listeners[key]({value: this.__getValue()}); } diff --git a/Libraries/Animated/src/__tests__/AnimatedNative-test.js b/Libraries/Animated/src/__tests__/AnimatedNative-test.js index 9fff20cb2..166d836d3 100644 --- a/Libraries/Animated/src/__tests__/AnimatedNative-test.js +++ b/Libraries/Animated/src/__tests__/AnimatedNative-test.js @@ -210,10 +210,27 @@ describe('Animated', () => { var anim = new Animated.Value(0); Animated.timing(anim, {toValue: 10, duration: 1000, useNativeDriver: true}).start(); - anim.setValue(5); + var c = new Animated.View(); + c.props = { + style: { + opacity: anim, + }, + }; + c.componentWillMount(); + + // We expect `setValue` not to propagate down to `setNativeProps`, otherwise it may try to access `setNativeProps` + // via component refs table that we override here. + c.refs = { + node: { + setNativeProps: jest.genMockFunction(), + }, + }; + + anim.setValue(0.5); var nativeAnimatedModule = require('NativeModules').NativeAnimatedModule; - expect(nativeAnimatedModule.setAnimatedNodeValue).toBeCalledWith(jasmine.any(Number), 5); + expect(nativeAnimatedModule.setAnimatedNodeValue).toBeCalledWith(jasmine.any(Number), 0.5); + expect(c.refs.node.setNativeProps.mock.calls.length).toBe(0); }); it('doesn\'t call into native API if useNativeDriver is set to false', () => {