From a2d72ee89cf3981e3daf4a67a6e20f41ca4c925b Mon Sep 17 00:00:00 2001
From: Charlie Croom <ccroom@twitter.com>
Date: Tue, 20 Oct 2020 11:53:45 -0400
Subject: [PATCH] [fix] Avoid usePlatformMethods excess ref creation

Close #1777
---
 .../src/exports/Text/__tests__/index-test.js  | 13 +++++
 .../src/exports/View/__tests__/index-test.js  |  6 +--
 .../src/modules/usePlatformMethods/index.js   | 51 ++++++++++++-------
 3 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/packages/react-native-web/src/exports/Text/__tests__/index-test.js b/packages/react-native-web/src/exports/Text/__tests__/index-test.js
index 78c32960..9b7caeb7 100644
--- a/packages/react-native-web/src/exports/Text/__tests__/index-test.js
+++ b/packages/react-native-web/src/exports/Text/__tests__/index-test.js
@@ -123,6 +123,19 @@ describe('components/Text', () => {
       expect(ref).toBeCalled();
     });
 
+    test('is not called for prop changes', () => {
+      const ref = jest.fn();
+      let rerender;
+      act(() => {
+        ({ rerender } = render(<Text nativeID="123" ref={ref} style={{ borderWidth: 5 }} />));
+      });
+      expect(ref).toHaveBeenCalledTimes(1);
+      act(() => {
+        rerender(<Text nativeID="1234" ref={ref} style={{ borderWidth: 6 }} />);
+      });
+      expect(ref).toHaveBeenCalledTimes(1);
+    });
+
     test('node has imperative methods', () => {
       const ref = React.createRef();
       act(() => {
diff --git a/packages/react-native-web/src/exports/View/__tests__/index-test.js b/packages/react-native-web/src/exports/View/__tests__/index-test.js
index f5d51446..8aeba2d9 100644
--- a/packages/react-native-web/src/exports/View/__tests__/index-test.js
+++ b/packages/react-native-web/src/exports/View/__tests__/index-test.js
@@ -127,15 +127,15 @@ describe('components/View', () => {
       expect(ref).toBeCalled();
     });
 
-    test('is not called for props changes', () => {
+    test('is not called for prop changes', () => {
       const ref = jest.fn();
       let rerender;
       act(() => {
-        ({ rerender } = render(<View ref={ref} testID="123" />));
+        ({ rerender } = render(<View nativeID="123" ref={ref} style={{ borderWidth: 5 }} />));
       });
       expect(ref).toHaveBeenCalledTimes(1);
       act(() => {
-        rerender(<View ref={ref} testID="1234" />);
+        rerender(<View nativeID="1234" ref={ref} style={{ borderWidth: 6 }} />);
       });
       expect(ref).toHaveBeenCalledTimes(1);
     });
diff --git a/packages/react-native-web/src/modules/usePlatformMethods/index.js b/packages/react-native-web/src/modules/usePlatformMethods/index.js
index 15b50a7d..48ce8b8b 100644
--- a/packages/react-native-web/src/modules/usePlatformMethods/index.js
+++ b/packages/react-native-web/src/modules/usePlatformMethods/index.js
@@ -7,9 +7,15 @@
  * @flow
  */
 
+import type { GenericStyleProp } from '../../types';
+import type { ViewProps } from '../../Exports/View';
+
 import UIManager from '../../exports/UIManager';
 import createDOMProps from '../createDOMProps';
-import { useMemo, useRef } from 'react';
+import useStable from '../useStable';
+import { useRef } from 'react';
+
+const emptyObject = {};
 
 function setNativeProps(node, nativeProps, classList, pointerEvents, style, previousStyleRef) {
   if (node != null && nativeProps) {
@@ -43,22 +49,33 @@ function setNativeProps(node, nativeProps, classList, pointerEvents, style, prev
  * Adds non-standard methods to the hode element. This is temporarily until an
  * API like `ReactNative.measure(hostRef, callback)` is added to React Native.
  */
-export default function usePlatformMethods(props: Object) {
+export default function usePlatformMethods({
+  classList,
+  pointerEvents,
+  style
+}: {
+  classList?: Array<string | boolean>,
+  style?: GenericStyleProp<*>,
+  pointerEvents?: $PropertyType<ViewProps, 'pointerEvents'>
+}) {
   const previousStyleRef = useRef(null);
-  const { classList, style, pointerEvents } = props;
+  const setNativePropsArgsRef = useRef(null);
+  setNativePropsArgsRef.current = { classList, pointerEvents, style };
 
-  return useMemo(
-    () => (hostNode: any) => {
-      if (hostNode != null) {
-        hostNode.measure = callback => UIManager.measure(hostNode, callback);
-        hostNode.measureLayout = (relativeToNode, success, failure) =>
-          UIManager.measureLayout(hostNode, relativeToNode, failure, success);
-        hostNode.measureInWindow = callback => UIManager.measureInWindow(hostNode, callback);
-        hostNode.setNativeProps = nativeProps =>
-          setNativeProps(hostNode, nativeProps, classList, pointerEvents, style, previousStyleRef);
-      }
-      return hostNode;
-    },
-    [classList, pointerEvents, style]
-  );
+  // Avoid creating a new ref on every render. The props only need to be
+  // available to 'setNativeProps' when it is called.
+  const ref = useStable(() => (hostNode: any) => {
+    if (hostNode != null) {
+      hostNode.measure = callback => UIManager.measure(hostNode, callback);
+      hostNode.measureLayout = (relativeToNode, success, failure) =>
+        UIManager.measureLayout(hostNode, relativeToNode, failure, success);
+      hostNode.measureInWindow = callback => UIManager.measureInWindow(hostNode, callback);
+      hostNode.setNativeProps = nativeProps => {
+        const { classList, style, pointerEvents } = setNativePropsArgsRef.current || emptyObject;
+        setNativeProps(hostNode, nativeProps, classList, pointerEvents, style, previousStyleRef);
+      };
+    }
+  });
+
+  return ref;
 }