diff --git a/minibank-fraud-web/src/HistoryScreen.tsx b/minibank-fraud-web/src/HistoryScreen.tsx index bb8badd..de3a9cb 100644 --- a/minibank-fraud-web/src/HistoryScreen.tsx +++ b/minibank-fraud-web/src/HistoryScreen.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState, type ReactNode } from 'react'; +import { useCallback, useEffect, useRef, useState, type ReactNode } from 'react'; import { fetchMyTransfers, formatMoney, type HistoryItem, type Page } from './api'; import ErrorBox from './ErrorBox'; import TableFrame from './TableFrame'; @@ -224,20 +224,20 @@ export default function HistoryScreen() { the loader below is called from the effect and from the retry beside its own error box. */ const alive = useRef(true); - useEffect(() => { - alive.current = true; - void loadFirst(); - return () => { - alive.current = false; - }; - }, []); - /** * The first page, and the way back from a first page that failed. The retry is the same call * rather than a reload of the browser tab: the screen is reached from the rail, and pressing * the entry somebody is already standing on mounts nothing. + * + * useCallback with an empty list, and the list is empty because it is true rather than to + * silence anything: everything this closes over is stable across renders - the state setters, + * the ref, the page size and the fetch - so the function never has to be rebuilt. That is what + * lets the effect below name it and still run once, which is the whole point. Declared above + * the effect and not below it: a name in a dependency array is read while the component + * renders, and a const read before its own line is a ReferenceError rather than a hoisted + * function. */ - async function loadFirst() { + const loadFirst = useCallback(async () => { try { setLoading(true); setError(null); @@ -251,7 +251,15 @@ export default function HistoryScreen() { } finally { if (alive.current) setLoading(false); } - } + }, []); + + useEffect(() => { + alive.current = true; + void loadFirst(); + return () => { + alive.current = false; + }; + }, [loadFirst]); /** * The next page, appended under the rows already on screen. diff --git a/minibank-fraud-web/src/NewPaymentScreen.tsx b/minibank-fraud-web/src/NewPaymentScreen.tsx index 9ced491..e531809 100644 --- a/minibank-fraud-web/src/NewPaymentScreen.tsx +++ b/minibank-fraud-web/src/NewPaymentScreen.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { createPayment, fetchMyBeneficiaries, @@ -176,14 +176,6 @@ export default function NewPaymentScreen({ parked }: { parked: boolean }) { */ const quoteToken = useRef(0); - useEffect(() => { - alive.current = true; - void loadAccounts(); - void loadBeneficiaries(); - return () => { - alive.current = false; - }; - }, []); /** * The accounts, without which there is no form at all. @@ -191,7 +183,13 @@ export default function NewPaymentScreen({ parked }: { parked: boolean }) { * Its failure is the one on this screen that leaves nothing to do, and it is a read that costs * nothing to ask twice, which is exactly the case a retry is for. */ - async function loadAccounts() { + /* + * useCallback with an empty list, and the list is empty because it is true rather than to + * silence anything: everything this closes over is stable across renders - the state setters, + * the ref and the fetch - so the function never has to be rebuilt, and the effect below can + * name it and still run once. + */ + const loadAccounts = useCallback(async () => { try { setLoadingAccounts(true); setAccountsError(null); @@ -210,7 +208,7 @@ export default function NewPaymentScreen({ parked }: { parked: boolean }) { } finally { if (alive.current) setLoadingAccounts(false); } - } + }, []); /** * The address book, fetched beside the accounts and gating nothing. @@ -218,7 +216,7 @@ export default function NewPaymentScreen({ parked }: { parked: boolean }) { * A slow list of payees must not delay a payment, so this has no loading line of its own: the * row simply is not there until the names arrive. */ - async function loadBeneficiaries() { + const loadBeneficiaries = useCallback(async () => { try { const saved = await fetchMyBeneficiaries(); if (!alive.current) return; @@ -229,7 +227,18 @@ export default function NewPaymentScreen({ parked }: { parked: boolean }) { setBeneficiaries([]); setBeneficiariesFailed(true); } - } + }, []); + + /* Declared after both loaders and not before them: a name in a dependency array is read while + the component renders, and a const read before its own line is a ReferenceError. */ + useEffect(() => { + alive.current = true; + void loadAccounts(); + void loadBeneficiaries(); + return () => { + alive.current = false; + }; + }, [loadAccounts, loadBeneficiaries]); /** * A name was chosen, or the choice was given up. diff --git a/minibank-fraud-web/src/WaitingAuthorizationsScreen.tsx b/minibank-fraud-web/src/WaitingAuthorizationsScreen.tsx index a1926ba..7d0ee35 100644 --- a/minibank-fraud-web/src/WaitingAuthorizationsScreen.tsx +++ b/minibank-fraud-web/src/WaitingAuthorizationsScreen.tsx @@ -283,6 +283,23 @@ export default function WaitingAuthorizationsScreen() { useEffect(() => { void loadList(); + /* + * THE ONE EFFECT IN THIS WINDOW THAT IS SILENCED RATHER THAN ANSWERED, and the difference from + * the other three is in what the loader reads. + * + * The history screen, the payment form and the desk all wrap their loader in useCallback and + * name it here, honestly: two of them close over nothing that changes between renders, and the + * desk's closes over exactly the pair its effect already watched. loadList is not like them. It + * reads items.length, to ask for a page as large as the list already on screen, and selectedId, + * to drop a selection that has left it. An honest useCallback would therefore be rebuilt on + * every change to either, and an effect naming it would re-read the whole list every time a + * customer picked a payment. + * + * Keeping it mount-only through a callback means reading those two through refs instead, which + * is a second copy of two pieces of state kept in step by hand for the sake of a lint rule. The + * array is empty because the effect is a mount, and that is said here rather than worked around. + */ + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); /* The deadline of the payment on screen, which is what the tick below is started for. */