Fix Animated.event attach/detach on component re-render

Summary:
Need to make sure `detach` happens on the old `scrollableNode` before it's unmounted and that `attach` happens on the new `scrollableNode` after it's mounted. This should also be more performant because the detach step no longer requires iterating through all the props, most of which are not animated, and we filter out unneeded updates if props or ref haven't changed.

= Test Plan =

Hook up native onscroll events in `FlatListExample` and toggle "debug" - before, the events would stop working because they would try to attach to the old unmounted node, but with this diff it keeps working as expected.

Reviewed By: fkgozali

Differential Revision: D4687186

fbshipit-source-id: 313a7964d4614190308490a51fc4f56abb6690f8

Libraries/Animated/src/AnimatedImplementation.js

@@ -1510,9 +1510,9 @@ class AnimatedStyle extends AnimatedWithChildren {
 
   // Recursively get values for nested styles (like iOS's shadowOffset)
   __walkStyleAndGetValues(style) {
-    let updatedStyle = {};
-    for (let key in style) {
-      let value = style[key];
+    const updatedStyle = {};
+    for (const key in style) {
+      const value = style[key];
       if (value instanceof Animated) {
         if (!value.__isNative) {
           // We cannot use value of natively driven nodes this way as the value we have access from
@@ -1535,9 +1535,9 @@ class AnimatedStyle extends AnimatedWithChildren {
 
   // Recursively get animated values for nested styles (like iOS's shadowOffset)
   __walkStyleAndGetAnimatedValues(style) {
-    let updatedStyle = {};
-    for (let key in style) {
-      let value = style[key];
+    const updatedStyle = {};
+    for (const key in style) {
+      const value = style[key];
       if (value instanceof Animated) {
         updatedStyle[key] = value.__getAnimatedValue();
       } else if (value && !Array.isArray(value) && typeof value === 'object') {
@@ -1690,7 +1690,9 @@ class AnimatedProps extends Animated {
   }
 
   setNativeView(animatedView: any): void {
-    invariant(this._animatedView === undefined, 'Animated view already set.');
+    if (this._animatedView === animatedView) {
+      return;
+    }
     this._animatedView = animatedView;
     if (this.__isNative) {
       this.__connectAnimatedView();
@@ -1729,7 +1731,9 @@ class AnimatedProps extends Animated {
 function createAnimatedComponent(Component: any): any {
   class AnimatedComponent extends React.Component {
     _component: any;
+    _prevComponent: any;
     _propsAnimated: AnimatedProps;
+    _eventDetachers: Array<Function> = [];
     _setComponentRef: Function;
 
     constructor(props: Object) {
@@ -1739,7 +1743,7 @@ function createAnimatedComponent(Component: any): any {
 
     componentWillUnmount() {
       this._propsAnimated && this._propsAnimated.__detach();
-      this._detachNativeEvents(this.props);
+      this._detachNativeEvents();
     }
 
     setNativeProps(props) {
@@ -1752,42 +1756,28 @@ function createAnimatedComponent(Component: any): any {
 
     componentDidMount() {
       this._propsAnimated.setNativeView(this._component);
-
-      this._attachNativeEvents(this.props);
+      this._attachNativeEvents();
     }
 
-    _attachNativeEvents(newProps) {
-      if (newProps !== this.props) {
-        this._detachNativeEvents(this.props);
-      }
-
+    _attachNativeEvents() {
       // Make sure to get the scrollable node for components that implement
       // `ScrollResponder.Mixin`.
-      const ref = this._component.getScrollableNode ?
+      const scrollableNode = this._component.getScrollableNode ?
         this._component.getScrollableNode() :
         this._component;
 
-      for (const key in newProps) {
-        const prop = newProps[key];
+      for (const key in this.props) {
+        const prop = this.props[key];
         if (prop instanceof AnimatedEvent && prop.__isNative) {
-          prop.__attach(ref, key);
+           prop.__attach(scrollableNode, key);
+           this._eventDetachers.push(() => prop.__detach(scrollableNode, key));
         }
       }
     }
 
-    _detachNativeEvents(props) {
-      // Make sure to get the scrollable node for components that implement
-      // `ScrollResponder.Mixin`.
-      const ref = this._component.getScrollableNode ?
-        this._component.getScrollableNode() :
-        this._component;
-
-      for (const key in props) {
-        const prop = props[key];
-        if (prop instanceof AnimatedEvent && prop.__isNative) {
-          prop.__detach(ref, key);
-        }
-      }
+    _detachNativeEvents() {
+      this._eventDetachers.forEach(remove => remove());
+      this._eventDetachers = [];
     }
 
     _attachProps(nextProps) {
@@ -1820,10 +1810,6 @@ function createAnimatedComponent(Component: any): any {
         callback,
       );
 
-      if (this._component) {
-        this._propsAnimated.setNativeView(this._component);
-      }
-
       // When you call detach, it removes the element from the parent list
       // of children. If it goes to 0, then the parent also detaches itself
       // and so on.
@@ -1835,9 +1821,18 @@ function createAnimatedComponent(Component: any): any {
       oldPropsAnimated && oldPropsAnimated.__detach();
     }
 
-    componentWillReceiveProps(nextProps) {
-      this._attachProps(nextProps);
-      this._attachNativeEvents(nextProps);
+    componentWillReceiveProps(newProps) {
+      this._attachProps(newProps);
+    }
+
+    componentDidUpdate(prevProps) {
+      if (this._component !== this._prevComponent) {
+        this._propsAnimated.setNativeView(this._component);
+      }
+      if (this._component !== this._prevComponent || prevProps !== this.props) {
+        this._detachNativeEvents();
+        this._attachNativeEvents();
+      }
     }
 
     render() {
@@ -1850,6 +1845,7 @@ function createAnimatedComponent(Component: any): any {
     }
 
     _setComponentRef(c) {
+      this._prevComponent = this._component;
       this._component = c;
     }