[fix] Optimize ref merging

Close #1746
Fix #1665

.eslintrc

@@ -30,7 +30,8 @@
   "env": {
     "browser": true,
     "es6": true,
-    "node": true
+    "node": true,
+    "jest": true
   },
   "globals": {
 

packages/react-native-web/src/exports/Picker/index.js

@@ -11,7 +11,7 @@
 import type { ViewProps } from '../View';
 
 import createElement from '../createElement';
-import setAndForwardRef from '../../modules/setAndForwardRef';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePlatformMethods from '../../hooks/usePlatformMethods';
 import PickerItem from './PickerItem';
 import StyleSheet, { type StyleObj } from '../StyleSheet';
@@ -47,12 +47,7 @@ const Picker = forwardRef<PickerProps, *>((props, forwardedRef) => {
   } = props;
 
   const hostRef = useRef(null);
-  const setRef = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      hostRef.current = hostNode;
-    }
-  });
+  const setRef = useMergeRefs(forwardedRef, hostRef);
 
   function handleChange(e: Object) {
     const { selectedIndex, value } = e.target;

packages/react-native-web/src/exports/Pressable/index.js

@@ -15,7 +15,7 @@ import type { ViewProps } from '../View';
 
 import * as React from 'react';
 import { forwardRef, memo, useMemo, useState, useRef } from 'react';
-import setAndForwardRef from '../../modules/setAndForwardRef';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePressEvents from '../../hooks/usePressEvents';
 import View from '../View';
 
@@ -97,12 +97,7 @@ function Pressable(props: Props, forwardedRef): React.Node {
   const [pressed, setPressed] = useForceableState(testOnly_pressed === true);
 
   const hostRef = useRef(null);
-  const setRef = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      hostRef.current = hostNode;
-    }
-  });
+  const setRef = useMergeRefs(forwardedRef, hostRef);
 
   const pressConfig = useMemo(
     () => ({

packages/react-native-web/src/exports/Text/index.js

@@ -15,8 +15,8 @@ import { forwardRef, useContext, useRef } from 'react';
 import createElement from '../createElement';
 import css from '../StyleSheet/css';
 import pick from '../../modules/pick';
-import setAndForwardRef from '../../modules/setAndForwardRef';
 import useElementLayout from '../../hooks/useElementLayout';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePlatformMethods from '../../hooks/usePlatformMethods';
 import useResponderEvents from '../../hooks/useResponderEvents';
 import StyleSheet from '../StyleSheet';
@@ -100,12 +100,7 @@ const Text = forwardRef<TextProps, *>((props, forwardedRef) => {
 
   const hasTextAncestor = useContext(TextAncestorContext);
   const hostRef = useRef(null);
-  const setRef = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      hostRef.current = hostNode;
-    }
-  });
+  const setRef = useMergeRefs(forwardedRef, hostRef);
 
   const classList = [
     classes.text,

packages/react-native-web/src/exports/TextInput/__tests__/index-test.js

@@ -633,4 +633,18 @@ describe('components/TextInput', () => {
     const input = findInput(container);
     expect(input.value).toEqual(value);
   });
+
+  describe('imperative methods', () => {
+    test('node.clear()', () => {
+      const ref = React.createRef();
+      render(<TextInput ref={ref} />);
+      expect(typeof ref.current.clear).toBe('function');
+    });
+
+    test('node.isFocused()', () => {
+      const ref = React.createRef();
+      render(<TextInput ref={ref} />);
+      expect(typeof ref.current.isFocused).toBe('function');
+    });
+  });
 });

packages/react-native-web/src/exports/TextInput/index.js

@@ -10,13 +10,13 @@
 
 import type { TextInputProps } from './types';
 
-import { forwardRef, useRef } from 'react';
+import { forwardRef, useCallback, useMemo, useRef } from 'react';
 import createElement from '../createElement';
 import css from '../StyleSheet/css';
 import pick from '../../modules/pick';
-import setAndForwardRef from '../../modules/setAndForwardRef';
 import useElementLayout from '../../hooks/useElementLayout';
 import useLayoutEffect from '../../hooks/useLayoutEffect';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePlatformMethods from '../../hooks/usePlatformMethods';
 import useResponderEvents from '../../hooks/useResponderEvents';
 import StyleSheet from '../StyleSheet';
@@ -186,40 +186,10 @@ const TextInput = forwardRef<TextInputProps, *>((props, forwardedRef) => {
     type = 'password';
   }
 
-  const hostRef = useRef(null);
   const dimensions = useRef({ height: null, width: null });
-  const setRef = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      // TextInput needs to add more methods to the hostNode in addition to those
-      // added by `usePlatformMethods`. This is temporarily until an API like
-      // `TextInput.clear(hostRef)` is added to React Native.
-      if (hostNode != null) {
-        hostNode.clear = function() {
-          if (hostNode != null) {
-            hostNode.value = '';
-          }
-        };
-        hostNode.isFocused = function() {
-          return hostNode != null && TextInputState.currentlyFocusedField() === hostNode;
-        };
-      }
-      hostRef.current = hostNode;
-      if (hostRef.current != null) {
-        handleContentSizeChange();
-      }
-    }
-  });
+  const hostRef = useRef(null);
 
-  function handleBlur(e) {
-    TextInputState._currentlyFocusedNode = null;
-    if (onBlur) {
-      e.nativeEvent.text = e.target.value;
-      onBlur(e);
-    }
-  }
-
-  function handleContentSizeChange() {
+  const handleContentSizeChange = useCallback(() => {
     const node = hostRef.current;
     if (multiline && onContentSizeChange && node != null) {
       const newHeight = node.scrollHeight;
@@ -237,6 +207,36 @@ const TextInput = forwardRef<TextInputProps, *>((props, forwardedRef) => {
         });
       }
     }
+  }, [hostRef, multiline, onContentSizeChange]);
+
+  const imperativeRef = useMemo(
+    () => hostNode => {
+      // TextInput needs to add more methods to the hostNode in addition to those
+      // added by `usePlatformMethods`. This is temporarily until an API like
+      // `TextInput.clear(hostRef)` is added to React Native.
+      if (hostNode != null) {
+        hostNode.clear = function() {
+          if (hostNode != null) {
+            hostNode.value = '';
+          }
+        };
+        hostNode.isFocused = function() {
+          return hostNode != null && TextInputState.currentlyFocusedField() === hostNode;
+        };
+        handleContentSizeChange();
+      }
+    },
+    [handleContentSizeChange]
+  );
+
+  const setRef = useMergeRefs(forwardedRef, hostRef, imperativeRef);
+
+  function handleBlur(e) {
+    TextInputState._currentlyFocusedNode = null;
+    if (onBlur) {
+      e.nativeEvent.text = e.target.value;
+      onBlur(e);
+    }
   }
 
   function handleChange(e) {

packages/react-native-web/src/exports/TouchableHighlight/index.js

@@ -16,8 +16,8 @@ import type { ViewProps } from '../View';
 
 import * as React from 'react';
 import { useCallback, useMemo, useState, useRef } from 'react';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePressEvents from '../../hooks/usePressEvents';
-import setAndForwardRef from '../../modules/setAndForwardRef';
 import StyleSheet from '../StyleSheet';
 import View from '../View';
 
@@ -93,12 +93,7 @@ function TouchableHighlight(props: Props, forwardedRef): React.Node {
   } = props;
 
   const hostRef = useRef(null);
-  const setRef = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      hostRef.current = hostNode;
-    }
-  });
+  const setRef = useMergeRefs(forwardedRef, hostRef);
 
   const [extraStyles, setExtraStyles] = useState(
     testOnly_pressed === true ? createExtraStyles(activeOpacity, underlayColor) : null

packages/react-native-web/src/exports/TouchableOpacity/index.js

@@ -15,8 +15,8 @@ import type { ViewProps } from '../View';
 
 import * as React from 'react';
 import { useCallback, useMemo, useState, useRef } from 'react';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePressEvents from '../../hooks/usePressEvents';
-import setAndForwardRef from '../../modules/setAndForwardRef';
 import StyleSheet from '../StyleSheet';
 import View from '../View';
 
@@ -51,12 +51,7 @@ function TouchableOpacity(props: Props, forwardedRef): React.Node {
   } = props;
 
   const hostRef = useRef(null);
-  const setRef = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      hostRef.current = hostNode;
-    }
-  });
+  const setRef = useMergeRefs(forwardedRef, hostRef);
 
   const [duration, setDuration] = useState('0s');
   const [opacityOverride, setOpacityOverride] = useState(null);

packages/react-native-web/src/exports/TouchableWithoutFeedback/__tests__/index-test.js

@@ -0,0 +1,26 @@
+import React from 'react';
+import { render } from '@testing-library/react';
+import TouchableWithoutFeedback from '../';
+import View from '../../View';
+
+describe('components/TouchableWithoutFeedback', () => {
+  test('forwards ref', () => {
+    const ref = jest.fn();
+    render(
+      <TouchableWithoutFeedback ref={ref}>
+        <View />
+      </TouchableWithoutFeedback>
+    );
+    expect(ref).toHaveBeenCalled();
+  });
+
+  test('forwards ref of child', () => {
+    const ref = jest.fn();
+    render(
+      <TouchableWithoutFeedback>
+        <View ref={ref} />
+      </TouchableWithoutFeedback>
+    );
+    expect(ref).toHaveBeenCalled();
+  });
+});

packages/react-native-web/src/exports/TouchableWithoutFeedback/index.js

@@ -16,7 +16,7 @@ import type { ViewProps } from '../View';
 import * as React from 'react';
 import { useMemo, useRef } from 'react';
 import pick from '../../modules/pick';
-import setAndForwardRef from '../../modules/setAndForwardRef';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePressEvents from '../../hooks/usePressEvents';
 
 export type Props = $ReadOnly<{|
@@ -115,20 +115,7 @@ function TouchableWithoutFeedback(props: Props, forwardedRef): React.Node {
   supportedProps.accessible = accessible !== false;
   supportedProps.accessibilityState = { disabled, ...props.accessibilityState };
   supportedProps.focusable = focusable !== false && onPress !== undefined;
-  supportedProps.ref = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      const { ref } = element;
-      if (ref != null) {
-        if (typeof ref === 'function') {
-          ref(hostNode);
-        } else {
-          ref.current = hostNode;
-        }
-      }
-      hostRef.current = hostNode;
-    }
-  });
+  supportedProps.ref = useMergeRefs(forwardedRef, hostRef, element.ref);
 
   const elementProps = Object.assign(supportedProps, pressEventHandlers);
 

packages/react-native-web/src/exports/View/index.js

@@ -15,8 +15,8 @@ import { forwardRef, useContext, useRef } from 'react';
 import createElement from '../createElement';
 import css from '../StyleSheet/css';
 import pick from '../../modules/pick';
-import setAndForwardRef from '../../modules/setAndForwardRef';
 import useElementLayout from '../../hooks/useElementLayout';
+import useMergeRefs from '../../modules/useMergeRefs';
 import usePlatformMethods from '../../hooks/usePlatformMethods';
 import useResponderEvents from '../../hooks/useResponderEvents';
 import StyleSheet from '../StyleSheet';
@@ -102,12 +102,7 @@ const View = forwardRef<ViewProps, *>((props, forwardedRef) => {
 
   const hasTextAncestor = useContext(TextAncestorContext);
   const hostRef = useRef(null);
-  const setRef = setAndForwardRef({
-    getForwardedRef: () => forwardedRef,
-    setLocalRef: hostNode => {
-      hostRef.current = hostNode;
-    }
-  });
+  const setRef = useMergeRefs(forwardedRef, hostRef);
 
   const classList = [classes.view];
   const style = StyleSheet.compose(

packages/react-native-web/src/modules/mergeRefs/__tests__/index-test.js

@@ -0,0 +1,31 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import * as React from 'react';
+import mergeRefs from '..';
+import { render } from '@testing-library/react';
+
+describe('modules/mergeRefs', () => {
+  test('merges refs of different types', () => {
+    const ref = React.createRef(null);
+    let functionRefValue = null;
+    let hookRef;
+    function Component() {
+      const functionRef = x => {
+        functionRefValue = x;
+      };
+      hookRef = React.useRef(null);
+      return <div ref={mergeRefs(ref, hookRef, functionRef)} />;
+    }
+
+    render(<Component />);
+
+    expect(ref.current).not.toBe(null);
+    expect(hookRef.current).not.toBe(null);
+    expect(functionRefValue).not.toBe(null);
+  });
+});

packages/react-native-web/src/modules/mergeRefs/index.js

@@ -0,0 +1,33 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict
+ */
+
+import * as React from 'react';
+
+export default function mergeRefs(...args: $ReadOnlyArray<React.ElementRef<any>>) {
+  return function forwardRef(node: HTMLElement | null) {
+    args.forEach((ref: React.ElementRef<any>) => {
+      if (ref == null) {
+        return;
+      }
+      if (typeof ref === 'function') {
+        ref(node);
+        return;
+      }
+      if (typeof ref === 'object') {
+        ref.current = node;
+        return;
+      }
+      console.error(
+        `mergeRefs cannot handle Refs of type boolean, number or string, received ref ${String(
+          ref
+        )}`
+      );
+    });
+  };
+}

packages/react-native-web/src/modules/setAndForwardRef/index.js

@@ -1,58 +0,0 @@
-/**
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- * @flow
- */
-
-import * as React from 'react';
-
-type Args = $ReadOnly<{|
-  getForwardedRef: () => ?React.Ref<any>,
-  setLocalRef: (ref: React.ElementRef<any>) => mixed
-|}>;
-
-/**
- * This is a helper function for when a component needs to be able to forward a ref
- * to a child component, but still needs to have access to that component as part of
- * its implementation.
- *
- * Its main use case is in wrappers for native components.
- *
- * Usage:
- *
- *   function MyView(props) {
- *     const ref = useRef(null);
- *
- *     function setRef = setAndForwardRef({
- *       getForwardedRef: () => props.forwardedRef,
- *       setLocalRef: localRef => {
- *         ref.current = localRef;
- *       },
- *     });
- *
- *     return <View ref={setRef} />;
- *   }
- *
- *   const MyViewWithRef = React.forwardRef((props, ref) => (
- *     <MyView {...props} forwardedRef={ref} />
- *   ));
- */
-
-export default function setAndForwardRef({ getForwardedRef, setLocalRef }: Args) {
-  return function forwardRef(ref: React.ElementRef<any>) {
-    const forwardedRef = getForwardedRef();
-    setLocalRef(ref);
-
-    // Forward to user ref prop (if one has been specified)
-    if (typeof forwardedRef === 'function') {
-      // Handle function-based refs. String-based refs are handled as functions.
-      forwardedRef(ref);
-    } else if (typeof forwardedRef === 'object' && forwardedRef != null) {
-      // Handle createRef-based refs
-      forwardedRef.current = ref;
-    }
-  };
-}

packages/react-native-web/src/modules/useMergeRefs/__tests__/index-test.js

@@ -0,0 +1,87 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+import * as React from 'react';
+import { act } from 'react-dom/test-utils';
+import { cleanup, render } from '@testing-library/react';
+import useMergeRefs from '..';
+
+describe('modules/useMergeRefs', () => {
+  function TestComponent({ refs, ...rest }) {
+    const mergedRef = useMergeRefs(...refs);
+    return <div ref={mergedRef} {...rest} />;
+  }
+
+  afterEach(cleanup);
+
+  it('handles no refs', () => {
+    act(() => {
+      render(<TestComponent refs={[]} />);
+    });
+  });
+
+  test('merges any number of varying refs', () => {
+    const callbackRefs = Array(10).map(() => jest.fn());
+    const objectRefs = Array(10).map(() => ({ current: null }));
+    const nullRefs = Array(10).map(() => null);
+
+    act(() => {
+      render(<TestComponent refs={[...callbackRefs, ...objectRefs, ...nullRefs]} />);
+    });
+
+    callbackRefs.forEach(ref => {
+      expect(ref).toHaveBeenCalledTimes(1);
+    });
+
+    objectRefs.forEach(ref => {
+      expect(ref.current).toBeInstanceOf(HTMLButtonElement);
+    });
+  });
+
+  test('ref is called when ref changes', () => {
+    const ref = jest.fn();
+    const nextRef = jest.fn();
+    let rerender;
+
+    act(() => {
+      ({ rerender } = render(<TestComponent refs={[ref]} />));
+    });
+    expect(ref).toHaveBeenCalled();
+    act(() => {
+      rerender(<TestComponent refs={[nextRef]} />);
+    });
+    expect(nextRef).toHaveBeenCalled();
+  });
+
+  test('ref is not called for each rerender', () => {
+    const ref = jest.fn();
+    let rerender;
+
+    act(() => {
+      ({ rerender } = render(<TestComponent refs={[ref]} />));
+    });
+    expect(ref).toHaveBeenCalledTimes(1);
+    act(() => {
+      rerender(<TestComponent refs={[ref]} />);
+    });
+    expect(ref).toHaveBeenCalledTimes(1);
+  });
+
+  test('ref is not called for props changes', () => {
+    const ref = jest.fn();
+    let rerender;
+
+    act(() => {
+      ({ rerender } = render(<TestComponent id="foo" refs={[ref]} />));
+    });
+    expect(ref).toHaveBeenCalledTimes(1);
+    act(() => {
+      rerender(<TestComponent id="bar" refs={[ref]} />);
+    });
+    expect(ref).toHaveBeenCalledTimes(1);
+  });
+});

packages/react-native-web/src/modules/useMergeRefs/index.js

@@ -0,0 +1,21 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict-local
+ */
+
+import * as React from 'react';
+import mergeRefs from '../mergeRefs';
+
+export default function useMergeRefs(...args: $ReadOnlyArray<React.ElementRef<any>>) {
+  return React.useMemo(
+    () => mergeRefs(...args),
+    // Disable linter because args is always an array, and it is spread as
+    // arguments to mergeRefs correctly
+    // eslint-disable-next-line
+    [...args]
+  );
+}

packages/react-native-web/src/vendor/react-native/Animated/createAnimatedComponent.js

@@ -13,7 +13,7 @@ import { AnimatedEvent } from './AnimatedEvent';
 import AnimatedProps from './nodes/AnimatedProps';
 import React from 'react';
 import invariant from 'fbjs/lib/invariant';
-import setAndForwardRef from '../../../modules/setAndForwardRef';
+import mergeRefs from '../../../modules/mergeRefs';
 
 function createAnimatedComponent(Component: any, defaultProps: any): any {
   invariant(
@@ -140,26 +140,23 @@ function createAnimatedComponent(Component: any, defaultProps: any): any {
       }
     }
 
-    _setComponentRef = setAndForwardRef({
-      getForwardedRef: () => this.props.forwardedRef,
-      setLocalRef: ref => {
-        this._prevComponent = this._component;
-        this._component = ref;
+    _setComponentRef = mergeRefs(this.props.forwardedRef, (ref) => {
+      this._prevComponent = this._component;
+      this._component = ref;
 
-        // TODO: Delete this in a future release.
-        if (ref != null && ref.getNode == null) {
-          ref.getNode = () => {
-            console.warn(
-              '%s: Calling `getNode()` on the ref of an Animated component ' +
-                'is no longer necessary. You can now directly use the ref ' +
-                'instead. This method will be removed in a future release.',
-              ref.constructor.name ?? '<<anonymous>>',
-            );
-            return ref;
-          };
-        }
-      },
-    });
+      // TODO: Delete this in a future release.
+      if (ref != null && ref.getNode == null) {
+        ref.getNode = () => {
+          console.warn(
+            '%s: Calling `getNode()` on the ref of an Animated component ' +
+              'is no longer necessary. You can now directly use the ref ' +
+              'instead. This method will be removed in a future release.',
+            ref.constructor.name ?? '<<anonymous>>',
+          );
+          return ref;
+        };
+      }
+    })
 
     render() {
       const props = this._propsAnimated.__getValue();