From 666f56bff318549b62ae5f68f0a046ef8d81c545 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Sat, 12 Nov 2022 09:17:00 -0800 Subject: [PATCH] TextInput: Switch to `useMergeRefs` Summary: Switches `TextInput` from `setAndForwardRefs` to `useMergeRefs` (and `useCallback`). Instead of creating a new `_setNativeRef` function on every render, this will now only create a new ref function when either `mostRecentEventCount` changes, `viewCommands` (i.e. `props.multiline` on iOS) changes, or when `props.forwardedRef` changes. When a text input is being edited, `mostRecentEventCount` will probably change. But when an unfocused `TextInput` is being updated because a parent is being updated, we will now no longer unnecessarily create a new `ref`. The observable behavior of this is that any `ref` supplied onto `TextInput` will now be invoked less frequently. Changelog: [General][Changed] Any `ref` set on `TextInput` will now be updated less frequently (when the underlying `ref` has not changed). Reviewed By: sammy-SC Differential Revision: D41191439 fbshipit-source-id: c69a061317c51ad6c6ca8acd116539e0f24a1d08 --- Libraries/Components/TextInput/TextInput.js | 140 ++++++++++---------- 1 file changed, 72 insertions(+), 68 deletions(-) diff --git a/Libraries/Components/TextInput/TextInput.js b/Libraries/Components/TextInput/TextInput.js index 768943604b86e7..7c64112f2db9e0 100644 --- a/Libraries/Components/TextInput/TextInput.js +++ b/Libraries/Components/TextInput/TextInput.js @@ -27,13 +27,12 @@ import StyleSheet, { import Text from '../../Text/Text'; import TextAncestor from '../../Text/TextAncestor'; import Platform from '../../Utilities/Platform'; -import setAndForwardRef from '../../Utilities/setAndForwardRef'; +import useMergeRefs from '../../Utilities/useMergeRefs'; import TextInputState from './TextInputState'; import invariant from 'invariant'; import nullthrows from 'nullthrows'; import * as React from 'react'; - -const {useLayoutEffect, useRef, useState} = React; +import {useCallback, useLayoutEffect, useRef, useState} from 'react'; type ReactRefSetter = {current: null | T, ...} | ((ref: null | T) => mixed); type TextInputInstance = React.ElementRef> & { @@ -1184,71 +1183,74 @@ function InternalTextInput(props: Props): React.Node { } }, [inputRef]); - const setLocalRef = (ref: TextInputInstance | null) => { - inputRef.current = ref; - - /* - Hi reader from the future. I'm sorry for this. - - This is a hack. Ideally we would forwardRef to the underlying - host component. However, since TextInput has it's own methods that can be - called as well, if we used the standard forwardRef then these - methods wouldn't be accessible and thus be a breaking change. - - We have a couple of options of how to handle this: - - Return a new ref with everything we methods from both. This is problematic - because we need React to also know it is a host component which requires - internals of the class implementation of the ref. - - Break the API and have some other way to call one set of the methods or - the other. This is our long term approach as we want to eventually - get the methods on host components off the ref. So instead of calling - ref.measure() you might call ReactNative.measure(ref). This would hopefully - let the ref for TextInput then have the methods like `.clear`. Or we do it - the other way and make it TextInput.clear(textInputRef) which would be fine - too. Either way though is a breaking change that is longer term. - - Mutate this ref. :( Gross, but accomplishes what we need in the meantime - before we can get to the long term breaking change. - */ - if (ref != null) { - // $FlowFixMe[incompatible-use] - See the explanation above. - Object.assign(ref, { - clear(): void { - if (inputRef.current != null) { - viewCommands.setTextAndSelection( - inputRef.current, - mostRecentEventCount, - '', - 0, - 0, - ); - } - }, - // TODO: Fix this returning true on null === null, when no input is focused - isFocused(): boolean { - return TextInputState.currentlyFocusedInput() === inputRef.current; - }, - getNativeRef(): ?React.ElementRef> { - return inputRef.current; - }, - setSelection(start: number, end: number): void { - if (inputRef.current != null) { - viewCommands.setTextAndSelection( - inputRef.current, - mostRecentEventCount, - null, - start, - end, - ); - } - }, - }); - } - }; + const setLocalRef = useCallback( + (instance: TextInputInstance | null) => { + inputRef.current = instance; + + /* + Hi reader from the future. I'm sorry for this. + + This is a hack. Ideally we would forwardRef to the underlying + host component. However, since TextInput has it's own methods that can be + called as well, if we used the standard forwardRef then these + methods wouldn't be accessible and thus be a breaking change. + + We have a couple of options of how to handle this: + - Return a new ref with everything we methods from both. This is problematic + because we need React to also know it is a host component which requires + internals of the class implementation of the ref. + - Break the API and have some other way to call one set of the methods or + the other. This is our long term approach as we want to eventually + get the methods on host components off the ref. So instead of calling + ref.measure() you might call ReactNative.measure(ref). This would hopefully + let the ref for TextInput then have the methods like `.clear`. Or we do it + the other way and make it TextInput.clear(textInputRef) which would be fine + too. Either way though is a breaking change that is longer term. + - Mutate this ref. :( Gross, but accomplishes what we need in the meantime + before we can get to the long term breaking change. + */ + if (instance != null) { + // $FlowFixMe[incompatible-use] - See the explanation above. + Object.assign(instance, { + clear(): void { + if (inputRef.current != null) { + viewCommands.setTextAndSelection( + inputRef.current, + mostRecentEventCount, + '', + 0, + 0, + ); + } + }, + // TODO: Fix this returning true on null === null, when no input is focused + isFocused(): boolean { + return TextInputState.currentlyFocusedInput() === inputRef.current; + }, + getNativeRef(): ?React.ElementRef> { + return inputRef.current; + }, + setSelection(start: number, end: number): void { + if (inputRef.current != null) { + viewCommands.setTextAndSelection( + inputRef.current, + mostRecentEventCount, + null, + start, + end, + ); + } + }, + }); + } + }, + [mostRecentEventCount, viewCommands], + ); - const _setNativeRef = setAndForwardRef({ - getForwardedRef: () => props.forwardedRef, + const ref = useMergeRefs( setLocalRef, - }); + props.forwardedRef, + ); const _onChange = (event: ChangeEvent) => { const currentText = event.nativeEvent.text; @@ -1411,7 +1413,8 @@ function InternalTextInput(props: Props): React.Node { textInput = (