Fix render loop in <Modal>, clean up ref merger code

(cherry picked from commit 893a77a)

ts/components/Input.tsx

@@ -15,7 +15,7 @@ import classNames from 'classnames';
 import * as grapheme from '../util/grapheme';
 import { LocalizerType } from '../types/Util';
 import { getClassNamesFor } from '../util/getClassNamesFor';
-import { multiRef } from '../util/multiRef';
+import { refMerger } from '../util/refMerger';
 
 export type PropsType = {
   countLength?: (value: string) => number;
@@ -173,7 +173,7 @@ export const Input = forwardRef<
       onKeyDown: handleKeyDown,
       onPaste: handlePaste,
       placeholder,
-      ref: multiRef<HTMLInputElement | HTMLTextAreaElement | null>(
+      ref: refMerger<HTMLInputElement | HTMLTextAreaElement | null>(
         ref,
         innerRef
       ),

ts/components/Modal.tsx

@@ -11,6 +11,7 @@ import { ModalHost } from './ModalHost';
 import { Theme } from '../util/theme';
 import { getClassNamesFor } from '../util/getClassNamesFor';
 import { useHasWrapped } from '../hooks/useHasWrapped';
+import { useRefMerger } from '../hooks/useRefMerger';
 
 type PropsType = {
   children: ReactNode;
@@ -38,6 +39,9 @@ export function Modal({
   theme,
 }: Readonly<PropsType>): ReactElement {
   const modalRef = useRef<HTMLDivElement | null>(null);
+
+  const refMerger = useRefMerger();
+
   const bodyRef = useRef<HTMLDivElement | null>(null);
   const [scrolled, setScrolled] = useState(false);
   const [hasOverflow, setHasOverflow] = useState(false);
@@ -112,10 +116,7 @@ export function Modal({
                 const scrollTop = bodyRef.current?.scrollTop || 0;
                 setScrolled(scrollTop > 2);
               }}
-              ref={bodyEl => {
-                measureRef(bodyEl);
-                bodyRef.current = bodyEl;
-              }}
+              ref={refMerger(measureRef, bodyRef)}
             >
               {children}
             </div>

ts/components/Tooltip.tsx

@@ -6,7 +6,7 @@ import classNames from 'classnames';
 import { noop } from 'lodash';
 import { Manager, Reference, Popper } from 'react-popper';
 import { Theme, themeClassName } from '../util/theme';
-import { multiRef } from '../util/multiRef';
+import { refMerger } from '../util/refMerger';
 import { offsetDistanceModifier } from '../util/popperUtil';
 
 type EventWrapperPropsType = {
@@ -52,7 +52,7 @@ const TooltipEventWrapper = React.forwardRef<
     <span
       onFocus={on}
       onBlur={off}
-      ref={multiRef<HTMLSpanElement>(ref, wrapperRef)}
+      ref={refMerger<HTMLSpanElement>(ref, wrapperRef)}
     >
       {children}
     </span>

ts/components/_util.ts

@@ -1,30 +1,6 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import { MutableRefObject, Ref } from 'react';
-import { isFunction } from 'lodash';
-import memoizee from 'memoizee';
-
 export function cleanId(id: string): string {
   return id.replace(/[^\u0020-\u007e\u00a0-\u00ff]/g, '_');
 }
-
-// Memoizee makes this difficult.
-// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-export const createRefMerger = () =>
-  memoizee(
-    <T>(...refs: Array<Ref<T>>) => {
-      return (t: T) => {
-        refs.forEach(r => {
-          if (isFunction(r)) {
-            r(t);
-          } else if (r) {
-            // Using a MutableRefObject as intended
-            // eslint-disable-next-line no-param-reassign
-            (r as MutableRefObject<T>).current = t;
-          }
-        });
-      };
-    },
-    { length: false, max: 1 }
-  );

ts/components/conversation/Message.tsx

@@ -62,7 +62,7 @@ import {
   ConversationColorType,
   CustomColorType,
 } from '../../types/Colors';
-import { createRefMerger } from '../_util';
+import { createRefMerger } from '../../util/refMerger';
 import { emojiToData } from '../emoji/lib';
 import type { SmartReactionPicker } from '../../state/smart/ReactionPicker';
 import { getCustomColorStyle } from '../../util/getCustomColorStyle';

ts/components/conversation/conversation-details/AddGroupMembersModal/ChooseGroupMembersModal.tsx

@@ -13,6 +13,8 @@ import Measure, { MeasuredComponentProps } from 'react-measure';
 import { LocalizerType } from '../../../../types/Util';
 import { assert } from '../../../../util/assert';
 import { getOwn } from '../../../../util/getOwn';
+import { refMerger } from '../../../../util/refMerger';
+import { useRestoreFocus } from '../../../../hooks/useRestoreFocus';
 import { missingCaseError } from '../../../../util/missingCaseError';
 import { filterAndSortConversationsByTitle } from '../../../../util/filterAndSortConversations';
 import { ConversationType } from '../../../../state/ducks/conversations';
@@ -58,6 +60,8 @@ export const ChooseGroupMembersModal: FunctionComponent<PropsType> = ({
   setSearchTerm,
   toggleSelectedContact,
 }) => {
+  const [focusRef] = useRestoreFocus();
+
   const inputRef = useRef<null | HTMLInputElement>(null);
 
   const numberOfContactsAlreadyInGroup = conversationIdsAlreadyInGroup.size;
@@ -142,7 +146,7 @@ export const ChooseGroupMembersModal: FunctionComponent<PropsType> = ({
               confirmAdds();
             }
           }}
-          ref={inputRef}
+          ref={refMerger<HTMLInputElement>(inputRef, focusRef)}
           value={searchTerm}
         />
         {Boolean(selectedContacts.length) && (

ts/hooks/useRefMerger.ts

@@ -0,0 +1,8 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { useMemo } from 'react';
+import { createRefMerger } from '../util/refMerger';
+
+export const useRefMerger = (): ReturnType<typeof createRefMerger> =>
+  useMemo(createRefMerger, []);

ts/util/multiRef.ts → ts/util/refMerger.ts

@@ -1,9 +1,18 @@
 // Copyright 2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
-import { Ref } from 'react';
+import type { MutableRefObject, Ref } from 'react';
+import memoizee from 'memoizee';
 
-export function multiRef<T>(...refs: Array<Ref<T>>): (topLevelRef: T) => void {
+/**
+ * Merges multiple refs.
+ *
+ * Returns a new function each time, which may cause unnecessary re-renders. Try
+ * `createRefMerger` if you want to cache the function.
+ */
+export function refMerger<T>(
+  ...refs: Array<Ref<unknown>>
+): (topLevelRef: T) => void {
   return (el: T) => {
     refs.forEach(ref => {
       // This is a simplified version of [what React does][0] to set a ref.
@@ -15,8 +24,12 @@ export function multiRef<T>(...refs: Array<Ref<T>>): (topLevelRef: T) => void {
         //   not be `readonly`. That's why we do this cast. See [the React source][1].
         // [1]: https://github.com/facebook/react/blob/29b7b775f2ecf878eaf605be959d959030598b07/packages/shared/ReactTypes.js#L78-L80
         // eslint-disable-next-line no-param-reassign
-        (ref as React.MutableRefObject<T>).current = el;
+        (ref as MutableRefObject<T>).current = el;
       }
     });
   };
 }
+
+export function createRefMerger(): typeof refMerger {
+  return memoizee(refMerger, { length: false, max: 1 });
+}