From 5876052615f4858ed5fc32fa3da9b64695974238 Mon Sep 17 00:00:00 2001 From: Eli White Date: Wed, 25 Sep 2019 13:34:22 -0700 Subject: [PATCH] Make setting useNativeDriver required. Add runtime warning if not specified Summary: We found that many callsites existed that could be using the native driver, but weren't. In order to help people use it when appropriate and eventually switch the default, we are requiring that useNativeDriver is explicit, even when set to false. This change adds a runtime warning if useNativeDriver is not specified, hopefully giving some light feedback to remember to use the native driver when you can. Without it being explicit it is very easy to forget setting this. Reviewed By: JoshuaGross Differential Revision: D17575918 fbshipit-source-id: e54612d87177e1821692b7de20fe673df0e890d2 --- Libraries/Animated/src/AnimatedEvent.js | 10 ++++++++-- Libraries/Animated/src/AnimatedImplementation.js | 2 +- Libraries/Animated/src/AnimatedMock.js | 2 +- Libraries/Animated/src/NativeAnimatedHelper.js | 7 +++++++ .../Animated/AnimatedGratuitousApp/AnExApp.js | 11 +++++++---- .../Animated/AnimatedGratuitousApp/AnExBobble.js | 2 +- .../Animated/AnimatedGratuitousApp/AnExChained.js | 1 + .../Animated/AnimatedGratuitousApp/AnExScroll.js | 1 + .../Animated/AnimatedGratuitousApp/AnExSet.js | 1 + .../Animated/AnimatedGratuitousApp/AnExTilt.js | 1 + 10 files changed, 29 insertions(+), 9 deletions(-) diff --git a/Libraries/Animated/src/AnimatedEvent.js b/Libraries/Animated/src/AnimatedEvent.js index 40a3bd1ae282a9..094a20c1a1f4ef 100644 --- a/Libraries/Animated/src/AnimatedEvent.js +++ b/Libraries/Animated/src/AnimatedEvent.js @@ -20,7 +20,7 @@ const {shouldUseNativeDriver} = require('./NativeAnimatedHelper'); export type Mapping = {[key: string]: Mapping} | AnimatedValue; export type EventConfig = { listener?: ?Function, - useNativeDriver?: boolean, + useNativeDriver: boolean, }; function attachNativeEvent( @@ -87,8 +87,14 @@ class AnimatedEvent { }; __isNative: boolean; - constructor(argMapping: Array, config?: EventConfig = {}) { + constructor(argMapping: Array, config: EventConfig) { this._argMapping = argMapping; + + if (config == null) { + console.warn('Animated.event now requires a second argument for options'); + config = {}; + } + if (config.listener) { this.__addListener(config.listener); } diff --git a/Libraries/Animated/src/AnimatedImplementation.js b/Libraries/Animated/src/AnimatedImplementation.js index 5ab98a5ce89ad6..b31ddec0f170f7 100644 --- a/Libraries/Animated/src/AnimatedImplementation.js +++ b/Libraries/Animated/src/AnimatedImplementation.js @@ -508,7 +508,7 @@ function unforkEvent( } } -const event = function(argMapping: Array, config?: EventConfig): any { +const event = function(argMapping: Array, config: EventConfig): any { const animatedEvent = new AnimatedEvent(argMapping, config); if (animatedEvent.__isNative) { return animatedEvent; diff --git a/Libraries/Animated/src/AnimatedMock.js b/Libraries/Animated/src/AnimatedMock.js index aed90a5ad05c19..83b0e72335c67a 100644 --- a/Libraries/Animated/src/AnimatedMock.js +++ b/Libraries/Animated/src/AnimatedMock.js @@ -119,7 +119,7 @@ const loop = function( return emptyAnimation; }; -const event = function(argMapping: Array, config?: EventConfig): any { +const event = function(argMapping: Array, config: EventConfig): any { return null; }; diff --git a/Libraries/Animated/src/NativeAnimatedHelper.js b/Libraries/Animated/src/NativeAnimatedHelper.js index 238a8fd2ba9d71..f37829abd8e3a3 100644 --- a/Libraries/Animated/src/NativeAnimatedHelper.js +++ b/Libraries/Animated/src/NativeAnimatedHelper.js @@ -265,6 +265,13 @@ function assertNativeAnimatedModule(): void { let _warnedMissingNativeAnimated = false; function shouldUseNativeDriver(config: AnimationConfig | EventConfig): boolean { + if (config.useNativeDriver == null) { + console.warn( + 'Animated: `useNativeDriver` was not specified. This is a required ' + + 'option and must be explicitly set to `true` or `false`', + ); + } + if (config.useNativeDriver === true && !NativeAnimatedModule) { if (!_warnedMissingNativeAnimated) { console.warn( diff --git a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExApp.js b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExApp.js index 17155ec2b41cd4..993dc4129f1f05 100644 --- a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExApp.js +++ b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExApp.js @@ -65,10 +65,13 @@ class Circle extends React.Component { this.setState( { panResponder: PanResponder.create({ - onPanResponderMove: Animated.event([ - null, // native event - ignore (step1: uncomment) - {dx: this.state.pan.x, dy: this.state.pan.y}, // links pan to gestureState (step1: uncomment) - ]), + onPanResponderMove: Animated.event( + [ + null, // native event - ignore (step1: uncomment) + {dx: this.state.pan.x, dy: this.state.pan.y}, // links pan to gestureState (step1: uncomment) + ], + {useNativeDriver: false}, + ), onPanResponderRelease: (e, gestureState) => { LayoutAnimation.easeInEaseOut(); // @flowfixme animates layout update as one batch (step3: uncomment) Animated.spring(this.state.pop, { diff --git a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExBobble.js b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExBobble.js index a2c95aadd08e1c..f845a4739bcce5 100644 --- a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExBobble.js +++ b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExBobble.js @@ -86,7 +86,7 @@ class AnExBobble extends React.Component { }, onPanResponderMove: Animated.event( [null, {dx: this.state.bobbles[0].x, dy: this.state.bobbles[0].y}], - {listener: bobblePanListener}, // async state changes with arbitrary logic + {listener: bobblePanListener, useNativeDriver: false}, // async state changes with arbitrary logic ), onPanResponderRelease: releaseBobble, onPanResponderTerminate: releaseBobble, diff --git a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExChained.js b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExChained.js index fb720bfe539673..4175fdf3684384 100644 --- a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExChained.js +++ b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExChained.js @@ -63,6 +63,7 @@ class AnExChained extends React.Component { }, onPanResponderMove: Animated.event( [null, {dx: this.state.stickers[0].x, dy: this.state.stickers[0].y}], // map gesture to leader + {useNativeDriver: false}, ), onPanResponderRelease: releaseChain, onPanResponderTerminate: releaseChain, diff --git a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExScroll.js b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExScroll.js index 87afc77f24b2ad..35c98ee89b8b78 100644 --- a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExScroll.js +++ b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExScroll.js @@ -33,6 +33,7 @@ class AnExScroll extends React.Component<$FlowFixMeProps, any> { scrollEventThrottle={16 /* get all events */} onScroll={Animated.event( [{nativeEvent: {contentOffset: {x: this.state.scrollX}}}], // nested event mapping + {useNativeDriver: false}, )} contentContainerStyle={{flex: 1, padding: 10}} pagingEnabled={true} diff --git a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExSet.js b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExSet.js index 439383f560370e..796f2c11f5374b 100644 --- a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExSet.js +++ b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExSet.js @@ -88,6 +88,7 @@ class AnExSet extends React.Component { }, onPanResponderMove: Animated.event( [null, {dy: this.state.dismissY}], // track pan gesture + {useNativeDriver: false}, ), onPanResponderRelease: (e, gestureState) => { if (gestureState.dy > 100) { diff --git a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExTilt.js b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExTilt.js index 61ad3591883f9a..ba8ca33efb9a71 100644 --- a/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExTilt.js +++ b/RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExTilt.js @@ -38,6 +38,7 @@ class AnExTilt extends React.Component { }, onPanResponderMove: Animated.event( [null, {dx: this.state.panX}], // panX is linked to the gesture + {useNativeDriver: false}, ), onPanResponderRelease: (e, gestureState) => { let toValue = 0;