Merge pull request #139 from callstack/fix-invalid-handles

Fix Invalid View Registration Handles
callstack · Oct 24, 2018 · 563472d · 563472d
2 parents a2230d7 + 4e56654
commit 563472d
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 27 deletions.
diff --git a/src/native-ads/withNativeAd.tsx b/src/native-ads/withNativeAd.tsx
@@ -14,6 +14,7 @@ import {
 } from './contexts';
 import { HasNativeAd, NativeAd } from './nativeAd';
 import AdsManager from './NativeAdsManager';
+import { areSetsEqual } from '../util/areSetsEqual';
 
 interface NativeAdViewProps {
   adsManager: string;
@@ -89,35 +90,36 @@ export default <T extends HasNativeAd>(
       );
     }
 
-    public componentDidUpdate(
-      prevProps: AdWrapperProps,
-      prevState: AdWrapperState,
-    ) {
+    public componentDidUpdate(_: AdWrapperProps, prevState: AdWrapperState) {
       if (
-        this.state.mediaViewNodeHandle !== -1 ||
-        this.state.adIconViewNodeHandle !== -1 ||
-        this.state.clickableChildren.size > 0
+        this.state.mediaViewNodeHandle === -1 ||
+        this.state.adIconViewNodeHandle === -1
       ) {
-        const mediaViewNodeHandleChanged =
-          this.state.mediaViewNodeHandle !== prevState.mediaViewNodeHandle;
-        const adIconViewNodeHandleChanged =
-          this.state.adIconViewNodeHandle !== prevState.adIconViewNodeHandle;
-        const clickableChildrenChanged = [
-          ...prevState.clickableChildren,
-        ].filter(child => !this.state.clickableChildren.has(child));
-
-        if (
-          mediaViewNodeHandleChanged ||
-          adIconViewNodeHandleChanged ||
-          clickableChildrenChanged
-        ) {
-          AdsManager.registerViewsForInteractionAsync(
-            findNodeHandle(this.nativeAdViewRef!)!,
-            this.state.mediaViewNodeHandle,
-            this.state.adIconViewNodeHandle,
-            [...this.state.clickableChildren],
-          );
-        }
+        // Facebook's SDK requires both MediaView and AdIconView references in order to register
+        // interactable views. If one of them is missing, we can't proceed with the registration.
+        return;
+      }
+
+      const mediaViewNodeHandleChanged =
+        this.state.mediaViewNodeHandle !== prevState.mediaViewNodeHandle;
+      const adIconViewNodeHandleChanged =
+        this.state.adIconViewNodeHandle !== prevState.adIconViewNodeHandle;
+      const clickableChildrenChanged = areSetsEqual(
+        prevState.clickableChildren,
+        this.state.clickableChildren,
+      );
+
+      if (
+        mediaViewNodeHandleChanged ||
+        adIconViewNodeHandleChanged ||
+        clickableChildrenChanged
+      ) {
+        AdsManager.registerViewsForInteractionAsync(
+          findNodeHandle(this.nativeAdViewRef!)!,
+          this.state.mediaViewNodeHandle,
+          this.state.adIconViewNodeHandle,
+          [...this.state.clickableChildren],
+        );
       }
     }
 

diff --git a/src/util/areSetsEqual.ts b/src/util/areSetsEqual.ts
@@ -0,0 +1,7 @@
+export const areSetsEqual = <T>(a: Set<T>, b: Set<T>) => {
+  if (a.size !== b.size) return false;
+  for (const aItem of a) {
+    if (!b.has(aItem)) return false;
+  }
+  return true;
+};