Fix validation of AnimatedEvent mapping

Summary: Noticed the _validateMapping call right now is a no-op, as the traverse method is never invoked. We can only really do validation once we've received a sample of an event, and can then verify the values being extracted indeed correspond with a valid key. Changelog: [General] [Fixed] - Fix validation of event mappings for AnimatedEvent Reviewed By: cpojer Differential Revision: D19498971 fbshipit-source-id: e978dda895498a7e567d5e18b3181b319d88d95c
facebook · Jan 23, 2020 · 19362f6 · 19362f6
1 parent 26d5faf
commit 19362f6
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 50 deletions.
diff --git a/Libraries/Animated/src/AnimatedEvent.js b/Libraries/Animated/src/AnimatedEvent.js
@@ -28,8 +28,8 @@ export type EventConfig = {
 function attachNativeEvent(
   viewRef: any,
   eventName: string,
-  argMapping: Array<?Mapping>,
-): {|detach: () => void|} {
+  argMapping: $ReadOnlyArray<?Mapping>,
+): {detach: () => void} {
   // Find animated values in `argMapping` and create an array representing their
   // key path inside the `nativeEvent` object. Ex.: ['contentOffset', 'x'].
   const eventMappings = [];
@@ -58,7 +58,6 @@ function attachNativeEvent(
   traverse(argMapping[0].nativeEvent, []);
 
   const viewTag = ReactNative.findNodeHandle(viewRef);
-
   if (viewTag != null) {
     eventMappings.forEach(mapping => {
       NativeAnimatedHelper.API.addAnimatedEventToView(
@@ -84,14 +83,59 @@ function attachNativeEvent(
   };
 }
 
+function validateMapping(argMapping, args) {
+  const validate = (recMapping, recEvt, key) => {
+    if (recMapping instanceof AnimatedValue) {
+      invariant(
+        typeof recEvt === 'number',
+        'Bad mapping of event key ' +
+          key +
+          ', should be number but got ' +
+          typeof recEvt,
+      );
+      return;
+    }
+    if (typeof recEvt === 'number') {
+      invariant(
+        recMapping instanceof AnimatedValue,
+        'Bad mapping of type ' +
+          typeof recMapping +
+          ' for key ' +
+          key +
+          ', event value must map to AnimatedValue',
+      );
+      return;
+    }
+    invariant(
+      typeof recMapping === 'object',
+      'Bad mapping of type ' + typeof recMapping + ' for key ' + key,
+    );
+    invariant(
+      typeof recEvt === 'object',
+      'Bad event of type ' + typeof recEvt + ' for key ' + key,
+    );
+    for (const mappingKey in recMapping) {
+      validate(recMapping[mappingKey], recEvt[mappingKey], mappingKey);
+    }
+  };
+
+  invariant(
+    args.length >= argMapping.length,
+    'Event has less arguments than mapping',
+  );
+  argMapping.forEach((mapping, idx) => {
+    validate(mapping, args[idx], 'arg' + idx);
+  });
+}
+
 class AnimatedEvent {
-  _argMapping: Array<?Mapping>;
+  _argMapping: $ReadOnlyArray<?Mapping>;
   _listeners: Array<Function> = [];
   _callListeners: Function;
   _attachedEvent: ?{detach: () => void, ...};
   __isNative: boolean;
 
-  constructor(argMapping: Array<?Mapping>, config: EventConfig) {
+  constructor(argMapping: $ReadOnlyArray<?Mapping>, config: EventConfig) {
     this._argMapping = argMapping;
 
     if (config == null) {
@@ -105,10 +149,6 @@ class AnimatedEvent {
     this._callListeners = this._callListeners.bind(this);
     this._attachedEvent = null;
     this.__isNative = shouldUseNativeDriver(config);
-
-    if (__DEV__) {
-      this._validateMapping();
-    }
   }
 
   __addListener(callback: Function): void {
@@ -143,62 +183,49 @@ class AnimatedEvent {
 
   __getHandler(): any | ((...args: any) => void) {
     if (this.__isNative) {
-      return this._callListeners;
+      if (__DEV__) {
+        let validatedMapping = false;
+        return (...args: any) => {
+          if (!validatedMapping) {
+            validateMapping(this._argMapping, args);
+            validatedMapping = true;
+          }
+          this._callListeners(...args);
+        };
+      } else {
+        return this._callListeners;
+      }
     }
 
+    let validatedMapping = false;
     return (...args: any) => {
+      if (__DEV__ && !validatedMapping) {
+        validateMapping(this._argMapping, args);
+        validatedMapping = true;
+      }
+
       const traverse = (recMapping, recEvt, key) => {
-        if (typeof recEvt === 'number' && recMapping instanceof AnimatedValue) {
-          recMapping.setValue(recEvt);
+        if (recMapping instanceof AnimatedValue) {
+          if (typeof recEvt === 'number') {
+            recMapping.setValue(recEvt);
+          }
         } else if (typeof recMapping === 'object') {
           for (const mappingKey in recMapping) {
-            /* $FlowFixMe(>=0.53.0 site=react_native_fb,react_native_oss) This
-             * comment suppresses an error when upgrading Flow's support for
-             * React. To see the error delete this comment and run Flow. */
             traverse(recMapping[mappingKey], recEvt[mappingKey], mappingKey);
           }
         }
       };
+      this._argMapping.forEach((mapping, idx) => {
+        traverse(mapping, args[idx], 'arg' + idx);
+      });
 
-      if (!this.__isNative) {
-        this._argMapping.forEach((mapping, idx) => {
-          traverse(mapping, args[idx], 'arg' + idx);
-        });
-      }
       this._callListeners(...args);
     };
   }
 
   _callListeners(...args: any) {
     this._listeners.forEach(listener => listener(...args));
   }
-
-  _validateMapping() {
-    const traverse = (recMapping, recEvt, key) => {
-      if (typeof recEvt === 'number') {
-        invariant(
-          recMapping instanceof AnimatedValue,
-          'Bad mapping of type ' +
-            typeof recMapping +
-            ' for key ' +
-            key +
-            ', event value must map to AnimatedValue',
-        );
-        return;
-      }
-      invariant(
-        typeof recMapping === 'object',
-        'Bad mapping of type ' + typeof recMapping + ' for key ' + key,
-      );
-      invariant(
-        typeof recEvt === 'object',
-        'Bad event of type ' + typeof recEvt + ' for key ' + key,
-      );
-      for (const mappingKey in recMapping) {
-        traverse(recMapping[mappingKey], recEvt[mappingKey], mappingKey);
-      }
-    };
-  }
 }
 
 module.exports = {AnimatedEvent, attachNativeEvent};
diff --git a/Libraries/Animated/src/AnimatedImplementation.js b/Libraries/Animated/src/AnimatedImplementation.js
@@ -522,7 +522,10 @@ function unforkEvent(
   }
 }
 
-const event = function(argMapping: Array<?Mapping>, config: EventConfig): any {
+const event = function(
+  argMapping: $ReadOnlyArray<?Mapping>,
+  config: EventConfig,
+): any {
   const animatedEvent = new AnimatedEvent(argMapping, config);
   if (animatedEvent.__isNative) {
     return animatedEvent;

diff --git a/Libraries/Animated/src/__tests__/AnimatedNative-test.js b/Libraries/Animated/src/__tests__/AnimatedNative-test.js
@@ -260,9 +260,9 @@ describe('Native Animated', () => {
         listener,
       });
       const handler = event.__getHandler();
-      handler({foo: 42});
+      handler({nativeEvent: {foo: 42}});
       expect(listener).toHaveBeenCalledTimes(1);
-      expect(listener).toBeCalledWith({foo: 42});
+      expect(listener).toBeCalledWith({nativeEvent: {foo: 42}});
     });
   });