Fix react-hooks/exhaustive-deps error on TransactionsTable.jsx (#4268)

* Fix react-hooks/exhaustive-deps error on TransactionsTable.jsx

* Release notes

* Fix lint

eslint.config.mjs

@@ -792,9 +792,6 @@ export default [
       'packages/desktop-client/src/components/spreadsheet/useSheetValue.ts',
       'packages/desktop-client/src/components/table.tsx',
       'packages/desktop-client/src/components/transactions/TransactionList.jsx',
-      'packages/desktop-client/src/components/transactions/TransactionsTable.jsx',
-      'packages/desktop-client/src/components/transactions/TransactionsTable.test.jsx',
-      // 'packages/desktop-client/src/hooks/useAccounts.ts',
       'packages/desktop-client/src/hooks/useCategories.ts',
     ],
 

packages/desktop-client/src/components/transactions/TransactionsTable.jsx

@@ -7,7 +7,6 @@ import React, {
   useRef,
   useMemo,
   useCallback,
-  useLayoutEffect,
   useEffect,
 } from 'react';
 import { useHotkeys } from 'react-hotkeys-hook';
@@ -1856,28 +1855,35 @@ function TransactionTableInner({
     setScrollWidth(!width ? 0 : width);
   }
 
+  const {
+    onCloseAddTransaction: onCloseAddTransactionProp,
+    onNavigateToTransferAccount: onNavigateToTransferAccountProp,
+    onNavigateToSchedule: onNavigateToScheduleProp,
+    onNotesTagClick: onNotesTagClickProp,
+  } = props;
+
   const onNavigateToTransferAccount = useCallback(
     accountId => {
-      props.onCloseAddTransaction();
-      props.onNavigateToTransferAccount(accountId);
+      onCloseAddTransactionProp();
+      onNavigateToTransferAccountProp(accountId);
     },
-    [props.onCloseAddTransaction, props.onNavigateToTransferAccount],
+    [onCloseAddTransactionProp, onNavigateToTransferAccountProp],
   );
 
   const onNavigateToSchedule = useCallback(
     scheduleId => {
-      props.onCloseAddTransaction();
-      props.onNavigateToSchedule(scheduleId);
+      onCloseAddTransactionProp();
+      onNavigateToScheduleProp(scheduleId);
     },
-    [props.onCloseAddTransaction, props.onNavigateToSchedule],
+    [onCloseAddTransactionProp, onNavigateToScheduleProp],
   );
 
   const onNotesTagClick = useCallback(
     noteTag => {
-      props.onCloseAddTransaction();
-      props.onNotesTagClick(noteTag);
+      onCloseAddTransactionProp();
+      onNotesTagClickProp(noteTag);
     },
-    [props.onCloseAddTransaction, props.onNotesTagClick],
+    [onCloseAddTransactionProp, onNotesTagClickProp],
   );
 
   useEffect(() => {
@@ -2113,6 +2119,7 @@ export const TransactionTable = forwardRef((props, ref) => {
   const [newTransactions, setNewTransactions] = useState(null);
   const [prevIsAdding, setPrevIsAdding] = useState(false);
   const splitsExpanded = useSplitsExpanded();
+  const splitsExpandedDispatch = splitsExpanded.dispatch;
   const prevSplitsExpanded = useRef(null);
 
   const tableRef = useRef(null);
@@ -2191,6 +2198,8 @@ export const TransactionTable = forwardRef((props, ref) => {
     );
   }, [props.transactions, props.payees, props.accounts]);
 
+  const hasPrevSplitsExpanded = prevSplitsExpanded.current;
+
   useEffect(() => {
     // If it's anchored that means we've also disabled animations. To
     // reduce the chance for side effect collision, only do this if
@@ -2199,7 +2208,7 @@ export const TransactionTable = forwardRef((props, ref) => {
       tableRef.current.unanchor();
       tableRef.current.setRowAnimation(true);
     }
-  }, [prevSplitsExpanded.current]);
+  }, [hasPrevSplitsExpanded]);
 
   const newNavigator = useTableNavigator(
     newTransactions,
@@ -2217,14 +2226,12 @@ export const TransactionTable = forwardRef((props, ref) => {
   const [_, forceRerender] = useState({});
   const selectedItems = useSelectedItems();
 
-  useLayoutEffect(() => {
-    latestState.current = {
-      newTransactions,
-      newNavigator,
-      tableNavigator,
-      transactions: props.transactions,
-    };
-  });
+  latestState.current = {
+    newTransactions,
+    newNavigator,
+    tableNavigator,
+    transactions: props.transactions,
+  };
 
   // Derive new transactions from the `isAdding` prop
   if (prevIsAdding !== props.isAdding) {
@@ -2239,32 +2246,30 @@ export const TransactionTable = forwardRef((props, ref) => {
     setPrevIsAdding(props.isAdding);
   }
 
-  useEffect(() => {
-    if (shouldAdd.current) {
-      if (newTransactions[0].account == null) {
-        dispatch(
-          addNotification({
-            type: 'error',
-            message: 'Account is a required field',
-          }),
-        );
-        newNavigator.onEdit('temp', 'account');
-      } else {
-        const transactions = latestState.current.newTransactions;
-        const lastDate = transactions.length > 0 ? transactions[0].date : null;
-        setNewTransactions(
-          makeTemporaryTransactions(
-            props.currentAccountId,
-            props.currentCategoryId,
-            lastDate,
-          ),
-        );
-        newNavigator.onEdit('temp', 'date');
-        props.onAdd(transactions);
-      }
-      shouldAdd.current = false;
+  if (shouldAdd.current) {
+    if (newTransactions[0].account == null) {
+      dispatch(
+        addNotification({
+          type: 'error',
+          message: 'Account is a required field',
+        }),
+      );
+      newNavigator.onEdit('temp', 'account');
+    } else {
+      const transactions = latestState.current.newTransactions;
+      const lastDate = transactions.length > 0 ? transactions[0].date : null;
+      setNewTransactions(
+        makeTemporaryTransactions(
+          props.currentAccountId,
+          props.currentCategoryId,
+          lastDate,
+        ),
+      );
+      newNavigator.onEdit('temp', 'date');
+      props.onAdd(transactions);
     }
-  });
+    shouldAdd.current = false;
+  }
 
   useEffect(() => {
     if (savePending.current && afterSaveFunc.current) {
@@ -2273,7 +2278,7 @@ export const TransactionTable = forwardRef((props, ref) => {
     }
 
     savePending.current = false;
-  }, [newTransactions, props.transactions]);
+  }, [newTransactions, props, props.transactions]);
 
   function getFieldsNewTransaction(item) {
     const fields = [
@@ -2410,7 +2415,20 @@ export const TransactionTable = forwardRef((props, ref) => {
     // effect we want to run. We have to wait for all updates to be
     // committed (the input could still be saving a value).
     forceRerender({});
-  }, [props.onAdd, newNavigator.onEdit]);
+  }, []);
+
+  const {
+    onSave: onSaveProp,
+    onApplyRules: onApplyRulesProp,
+    onBatchDelete,
+    onBatchDuplicate,
+    onBatchLinkSchedule,
+    onBatchUnlinkSchedule,
+    onCreateRule: onCreateRuleProp,
+    onScheduleAction: onScheduleActionProp,
+    onMakeAsNonSplitTransactions: onMakeAsNonSplitTransactionsProp,
+    onSplit: onSplitProp,
+  } = props;
 
   const onSave = useCallback(
     async (transaction, subtransactions = null, updatedFieldName = null) => {
@@ -2421,8 +2439,8 @@ export const TransactionTable = forwardRef((props, ref) => {
         : transaction;
 
       if (isTemporaryId(transaction.id)) {
-        if (props.onApplyRules) {
-          groupedTransaction = await props.onApplyRules(
+        if (onApplyRulesProp) {
+          groupedTransaction = await onApplyRulesProp(
             groupedTransaction,
             updatedFieldName,
           );
@@ -2437,48 +2455,69 @@ export const TransactionTable = forwardRef((props, ref) => {
           ),
         );
       } else {
-        props.onSave(groupedTransaction);
+        onSaveProp(groupedTransaction);
       }
     },
-    [props.onSave],
+    [onSaveProp, onApplyRulesProp],
   );
 
-  const onDelete = useCallback(id => {
-    const temporary = isTemporaryId(id);
+  const onDelete = useCallback(
+    id => {
+      const temporary = isTemporaryId(id);
 
-    if (temporary) {
-      const newTrans = latestState.current.newTransactions;
+      if (temporary) {
+        const newTrans = latestState.current.newTransactions;
 
-      if (id === newTrans[0].id) {
-        // You can never delete the parent new transaction
-        return;
+        if (id === newTrans[0].id) {
+          // You can never delete the parent new transaction
+          return;
+        }
+
+        setNewTransactions(deleteTransaction(newTrans, id).data);
+      } else {
+        onBatchDelete([id]);
       }
+    },
+    [onBatchDelete],
+  );
 
-      setNewTransactions(deleteTransaction(newTrans, id).data);
-    } else {
-      props.onBatchDelete([id]);
-    }
-  }, []);
+  const onDuplicate = useCallback(
+    id => {
+      onBatchDuplicate([id]);
+    },
+    [onBatchDuplicate],
+  );
 
-  const onDuplicate = useCallback(id => {
-    props.onBatchDuplicate([id]);
-  }, []);
-
-  const onLinkSchedule = useCallback(id => {
-    props.onBatchLinkSchedule([id]);
-  }, []);
-  const onUnlinkSchedule = useCallback(id => {
-    props.onBatchUnlinkSchedule([id]);
-  }, []);
-  const onCreateRule = useCallback(id => {
-    props.onCreateRule([id]);
-  }, []);
-  const onScheduleAction = useCallback((action, id) => {
-    props.onScheduleAction(action, [id]);
-  }, []);
-  const onMakeAsNonSplitTransactions = useCallback(id => {
-    props.onMakeAsNonSplitTransactions([id]);
-  }, []);
+  const onLinkSchedule = useCallback(
+    id => {
+      onBatchLinkSchedule([id]);
+    },
+    [onBatchLinkSchedule],
+  );
+  const onUnlinkSchedule = useCallback(
+    id => {
+      onBatchUnlinkSchedule([id]);
+    },
+    [onBatchUnlinkSchedule],
+  );
+  const onCreateRule = useCallback(
+    id => {
+      onCreateRuleProp([id]);
+    },
+    [onCreateRuleProp],
+  );
+  const onScheduleAction = useCallback(
+    (action, id) => {
+      onScheduleActionProp(action, [id]);
+    },
+    [onScheduleActionProp],
+  );
+  const onMakeAsNonSplitTransactions = useCallback(
+    id => {
+      onMakeAsNonSplitTransactionsProp([id]);
+    },
+    [onMakeAsNonSplitTransactionsProp],
+  );
 
   const onSplit = useMemo(() => {
     return id => {
@@ -2501,9 +2540,9 @@ export const TransactionTable = forwardRef((props, ref) => {
         }
       } else {
         const trans = latestState.current.transactions.find(t => t.id === id);
-        const newId = props.onSplit(id);
+        const newId = onSplitProp(id);
 
-        splitsExpanded.dispatch({ type: 'open-split', id: trans.id });
+        splitsExpandedDispatch({ type: 'open-split', id: trans.id });
 
         const { tableNavigator } = latestState.current;
         if (trans.amount === null) {
@@ -2513,12 +2552,19 @@ export const TransactionTable = forwardRef((props, ref) => {
         }
       }
     };
-  }, [props.onSplit, splitsExpanded.dispatch]);
+  }, [onSplitProp, splitsExpandedDispatch]);
+
+  const { onAddSplit: onAddSplitProp } = props;
 
   const onAddSplit = useCallback(
     id => {
+      const {
+        tableNavigator,
+        newNavigator,
+        newTransactions: newTrans,
+      } = latestState.current;
+
       if (isTemporaryId(id)) {
-        const newTrans = latestState.current.newTransactions;
         const { data, diff } = addSplitTransaction(newTrans, id);
         setNewTransactions(data);
         newNavigator.onEdit(
@@ -2526,19 +2572,19 @@ export const TransactionTable = forwardRef((props, ref) => {
           latestState.current.newNavigator.focusedField,
         );
       } else {
-        const newId = props.onAddSplit(id);
+        const newId = onAddSplitProp(id);
         tableNavigator.onEdit(
           newId,
           latestState.current.tableNavigator.focusedField,
         );
       }
     },
-    [props.onAddSplit],
+    [onAddSplitProp],
   );
 
   const onDistributeRemainder = useCallback(
     async id => {
-      const { transactions, tableNavigator, newTransactions } =
+      const { transactions, newNavigator, tableNavigator, newTransactions } =
         latestState.current;
 
       const targetTransactions = isTemporaryId(id)
@@ -2592,7 +2638,7 @@ export const TransactionTable = forwardRef((props, ref) => {
         });
       }
     },
-    [latestState],
+    [onSave],
   );
 
   function onCloseAddTransaction() {
@@ -2606,8 +2652,8 @@ export const TransactionTable = forwardRef((props, ref) => {
   }
 
   const onToggleSplit = useCallback(
-    id => splitsExpanded.dispatch({ type: 'toggle-split', id }),
-    [splitsExpanded.dispatch],
+    id => splitsExpandedDispatch({ type: 'toggle-split', id }),
+    [splitsExpandedDispatch],
   );
 
   return (

packages/desktop-client/src/components/transactions/TransactionsTable.test.tsx

@@ -130,13 +130,14 @@ type LiveTransactionTableProps = {
 };
 
 function LiveTransactionTable(props: LiveTransactionTableProps) {
-  const [transactions, setTransactions] = useState(props.transactions);
+  const { transactions: transactionsProp, onTransactionsChange } = props;
+
+  const [transactions, setTransactions] = useState(transactionsProp);
 
   useEffect(() => {
-    if (transactions === props.transactions) return;
-    props.onTransactionsChange?.(transactions);
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [transactions]);
+    if (transactions === transactionsProp) return;
+    onTransactionsChange?.(transactions);
+  }, [transactions, transactionsProp, onTransactionsChange]);
 
   const onSplit = (id: string) => {
     const { data, diff } = splitTransaction(transactions, id);

upcoming-release-notes/4268.md

@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [joel-jeremy]
+---
+
+Fix react-hooks/exhaustive-deps error on TransactionsTable.jsx