Skip to content

Commit

Permalink
Make setting useNativeDriver required. Add runtime warning if not spe…
Browse files Browse the repository at this point in the history
…cified

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
  • Loading branch information
elicwhite authored and facebook-github-bot committed Sep 25, 2019
1 parent 62acf6e commit 5876052
Show file tree
Hide file tree
Showing 10 changed files with 29 additions and 9 deletions.
10 changes: 8 additions & 2 deletions Libraries/Animated/src/AnimatedEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -87,8 +87,14 @@ class AnimatedEvent {
};
__isNative: boolean;

constructor(argMapping: Array<?Mapping>, config?: EventConfig = {}) {
constructor(argMapping: Array<?Mapping>, 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);
}
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Animated/src/AnimatedImplementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ function unforkEvent(
}
}

const event = function(argMapping: Array<?Mapping>, config?: EventConfig): any {
const event = function(argMapping: Array<?Mapping>, config: EventConfig): any {
const animatedEvent = new AnimatedEvent(argMapping, config);
if (animatedEvent.__isNative) {
return animatedEvent;
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Animated/src/AnimatedMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ const loop = function(
return emptyAnimation;
};

const event = function(argMapping: Array<?Mapping>, config?: EventConfig): any {
const event = function(argMapping: Array<?Mapping>, config: EventConfig): any {
return null;
};

Expand Down
7 changes: 7 additions & 0 deletions Libraries/Animated/src/NativeAnimatedHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
11 changes: 7 additions & 4 deletions RNTester/js/examples/Animated/AnimatedGratuitousApp/AnExApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,13 @@ class Circle extends React.Component<any, any> {
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, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class AnExBobble extends React.Component<Object, any> {
},
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class AnExChained extends React.Component<Object, any> {
},
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class AnExSet extends React.Component<Object, any> {
},
onPanResponderMove: Animated.event(
[null, {dy: this.state.dismissY}], // track pan gesture
{useNativeDriver: false},
),
onPanResponderRelease: (e, gestureState) => {
if (gestureState.dy > 100) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class AnExTilt extends React.Component<Object, any> {
},
onPanResponderMove: Animated.event(
[null, {dx: this.state.panX}], // panX is linked to the gesture
{useNativeDriver: false},
),
onPanResponderRelease: (e, gestureState) => {
let toValue = 0;
Expand Down

0 comments on commit 5876052

Please sign in to comment.