Fix useCallback dependency arrays (#261)

Expensify · Apr 3, 2024 · 2e3257f · 2e3257f
1 parent d7b611f
commit 2e3257f
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 47 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -9,6 +9,7 @@ module.exports = {
   },
   extends: [
     'expensify',
+    'plugin:react-hooks/recommended',
     'plugin:@typescript-eslint/recommended',
     'plugin:@typescript-eslint/stylistic',
     'plugin:import/typescript',

diff --git a/src/MarkdownTextInput.web.tsx b/src/MarkdownTextInput.web.tsx
@@ -207,7 +207,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
         parseText(divRef.current, divRef.current.innerText, newMarkdownStyle);
       }
       return newMarkdownStyle;
-    }, [markdownStyle]);
+    }, [markdownStyle, parseText]);
 
     const inputStyles = useMemo(
       () =>
@@ -228,7 +228,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
         const item = history.current.undo();
         return parseText(target, item ? item.text : null, processedMarkdownStyle, item ? item.cursorPosition : null, false).text;
       },
-      [processedMarkdownStyle],
+      [parseText, processedMarkdownStyle],
     );
 
     const redo = useCallback(
@@ -237,7 +237,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
         const item = history.current.redo();
         return parseText(target, item ? item.text : null, processedMarkdownStyle, item ? item.cursorPosition : null, false).text;
       },
-      [processedMarkdownStyle],
+      [parseText, processedMarkdownStyle],
     );
 
     // We have to process value property since contentEditable div adds one additional '\n' at the end of the text if we are entering new line
@@ -249,11 +249,52 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
     }, [value]);
 
     // Placeholder text color logic
-    const updateTextColor = useCallback((node: HTMLDivElement, text: string) => {
-      // eslint-disable-next-line no-param-reassign -- we need to change the style of the node, so we need to modify it
-      node.style.color = String(placeholder && (text === '' || text === '\n') ? placeholderTextColor : flattenedStyle.color || 'black');
+    const updateTextColor = useCallback(
+      (node: HTMLDivElement, text: string) => {
+        // eslint-disable-next-line no-param-reassign -- we need to change the style of the node, so we need to modify it
+        node.style.color = String(placeholder && (text === '' || text === '\n') ? placeholderTextColor : flattenedStyle.color || 'black');
+      },
+      [flattenedStyle.color, placeholder, placeholderTextColor],
+    );
+
+    const handleSelectionChange: ReactEventHandler<HTMLDivElement> = useCallback(
+      (event) => {
+        const e = event as unknown as NativeSyntheticEvent<TextInputSelectionChangeEventData>;
+        setEventProps(e);
+        if (onSelectionChange && contentSelection.current) {
+          e.nativeEvent.selection = contentSelection.current;
+          onSelectionChange(e);
+        }
+      },
+      [onSelectionChange, setEventProps],
+    );
+
+    const updateRefSelectionVariables = useCallback((newSelection: Selection) => {
+      const {start, end} = newSelection;
+      const markdownHTMLInput = divRef.current as HTMLInputElement;
+      markdownHTMLInput.selectionStart = start;
+      markdownHTMLInput.selectionEnd = end;
     }, []);
 
+    const updateSelection = useCallback(
+      (e: SyntheticEvent<HTMLDivElement> | null = null, predefinedSelection: Selection | null = null) => {
+        if (!divRef.current) {
+          return;
+        }
+        const newSelection = predefinedSelection || CursorUtils.getCurrentCursorPosition(divRef.current);
+
+        if (newSelection && (!contentSelection.current || contentSelection.current.start !== newSelection.start || contentSelection.current.end !== newSelection.end)) {
+          updateRefSelectionVariables(newSelection);
+          contentSelection.current = newSelection;
+
+          if (e) {
+            handleSelectionChange(e);
+          }
+        }
+      },
+      [handleSelectionChange, updateRefSelectionVariables],
+    );
+
     const handleOnChangeText = useCallback(
       (e: SyntheticEvent<HTMLDivElement>) => {
         if (!divRef.current || !(e.target instanceof HTMLElement)) {
@@ -292,44 +333,9 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
           onChangeText(normalizedText);
         }
       },
-      [multiline, onChange, onChangeText, setEventProps, processedMarkdownStyle],
+      [updateSelection, updateTextColor, onChange, onChangeText, undo, redo, parseText, processedMarkdownStyle, setEventProps],
     );
 
-    const handleSelectionChange: ReactEventHandler<HTMLDivElement> = useCallback(
-      (event) => {
-        const e = event as unknown as NativeSyntheticEvent<TextInputSelectionChangeEventData>;
-        setEventProps(e);
-        if (onSelectionChange && contentSelection.current) {
-          e.nativeEvent.selection = contentSelection.current;
-          onSelectionChange(e);
-        }
-      },
-      [onSelectionChange, setEventProps],
-    );
-
-    const updateRefSelectionVariables = useCallback((newSelection: Selection) => {
-      const {start, end} = newSelection;
-      const markdownHTMLInput = divRef.current as HTMLInputElement;
-      markdownHTMLInput.selectionStart = start;
-      markdownHTMLInput.selectionEnd = end;
-    }, []);
-
-    const updateSelection = useCallback((e: SyntheticEvent<HTMLDivElement> | null = null, predefinedSelection: Selection | null = null) => {
-      if (!divRef.current) {
-        return;
-      }
-      const newSelection = predefinedSelection || CursorUtils.getCurrentCursorPosition(divRef.current);
-
-      if (newSelection && (!contentSelection.current || contentSelection.current.start !== newSelection.start || contentSelection.current.end !== newSelection.end)) {
-        updateRefSelectionVariables(newSelection);
-        contentSelection.current = newSelection;
-
-        if (e) {
-          handleSelectionChange(e);
-        }
-      }
-    }, []);
-
     const handleKeyPress = useCallback(
       (e: KeyboardEvent<HTMLDivElement>) => {
         if (!divRef.current) {
@@ -388,7 +394,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
           }
         }
       },
-      [onKeyPress],
+      [multiline, blurOnSubmit, setEventProps, onKeyPress, updateSelection, handleOnChangeText, onSubmitEditing],
     );
 
     const handleFocus: FocusEventHandler<HTMLDivElement> = useCallback(
@@ -426,7 +432,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
           }
         }
       },
-      [clearTextOnFocus, multiline, onFocus, selectTextOnFocus, setEventProps],
+      [clearTextOnFocus, onFocus, selectTextOnFocus, setEventProps, updateSelection, value],
     );
 
     const handleBlur: FocusEventHandler<HTMLDivElement> = useCallback(
@@ -451,7 +457,7 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
         (e.target as HTMLInputElement).value = normalizeValue(divRef.current.innerText || '');
         onClick(e);
       },
-      [onClick],
+      [onClick, updateSelection],
     );
 
     const startComposition = useCallback(() => {
@@ -522,15 +528,15 @@ const MarkdownTextInput = React.forwardRef<TextInput, MarkdownTextInputProps>(
       if (autoFocus) {
         divRef.current.focus();
       }
-    }, []);
+    }, [autoFocus]);
 
     useEffect(() => {
       if (!divRef.current || !selection || (contentSelection.current && selection.start === contentSelection.current.start && selection.end === contentSelection.current.end)) {
         return;
       }
       CursorUtils.setCursorPosition(divRef.current, selection.start, selection.end);
       updateSelection(null, {start: selection.start, end: selection.end || selection.start});
-    }, [selection]);
+    }, [selection, updateSelection]);
 
     return (
       // eslint-disable-next-line jsx-a11y/no-static-element-interactions