diff --git a/frontend/src/admin/Categories.tsx b/frontend/src/admin/Categories.tsx index 329f6ad..3583b93 100644 --- a/frontend/src/admin/Categories.tsx +++ b/frontend/src/admin/Categories.tsx @@ -78,6 +78,8 @@ export default function Categories() { .finally(() => setLoading(false)); } + // Load-on-mount. `load` sets a pending flag before fetching. + // eslint-disable-next-line react-hooks/set-state-in-effect useEffect(() => { void load(); }, []); function openNew(parent: number | null) { diff --git a/frontend/src/admin/Tags.tsx b/frontend/src/admin/Tags.tsx index 1c476fd..55f6d4d 100644 --- a/frontend/src/admin/Tags.tsx +++ b/frontend/src/admin/Tags.tsx @@ -36,6 +36,8 @@ export default function Tags() { .finally(() => setLoading(false)); } + // Load-on-mount. `load` sets a pending flag before fetching. + // eslint-disable-next-line react-hooks/set-state-in-effect useEffect(() => { void load(); }, []); function openNew() { diff --git a/frontend/src/cart/Cart.tsx b/frontend/src/cart/Cart.tsx index 2460073..e231c45 100644 --- a/frontend/src/cart/Cart.tsx +++ b/frontend/src/cart/Cart.tsx @@ -67,6 +67,9 @@ export default function Cart() { .finally(() => setLoading(false)); } + // Load-on-mount once the session resolves. `loadAll` sets a pending flag + // first, which is what the rule sees. + // eslint-disable-next-line react-hooks/set-state-in-effect useEffect(() => { if (customer) loadAll(); }, [customer]); // Only ticks while there is something to count down, so an empty cart does @@ -80,6 +83,10 @@ export default function Cart() { // the release rather than sitting at "expiring…" until the page is reloaded. useEffect(() => { if (!lapsed) return; + // Refetching from the server on a tick, which is synchronisation rather + // than derived state — the release happens server-side and the client + // has no way to know when. + // eslint-disable-next-line react-hooks/set-state-in-effect loadAll(); // The header badge counts held items too, so it goes stale in exactly the // same way. Safe as a dependency: CartContext memoizes it with an empty diff --git a/frontend/src/cart/CartContext.tsx b/frontend/src/cart/CartContext.tsx index 9d97fb2..dac4702 100644 --- a/frontend/src/cart/CartContext.tsx +++ b/frontend/src/cart/CartContext.tsx @@ -24,10 +24,16 @@ export function CartProvider({ children }: Readonly<{ children: React.ReactNode .catch(() => setItems([])); }, []); + // Synchronising with the session, which is an external system, so an effect + // is the right tool. Deriving it instead would push "signed out" onto every + // consumer of this context, and remounting on a `key` is more indirection + // than the problem deserves. Decided in #99 rather than left to be + // re-investigated. useEffect(() => { if (customer) { refresh(); } else { + // eslint-disable-next-line react-hooks/set-state-in-effect setItems([]); } }, [customer, refresh]); diff --git a/frontend/src/cart/useNow.ts b/frontend/src/cart/useNow.ts index 838fadd..8db7a6f 100644 --- a/frontend/src/cart/useNow.ts +++ b/frontend/src/cart/useNow.ts @@ -25,6 +25,11 @@ export function useNow(intervalMs: number, active: boolean): number { if (!active) return; // Set immediately as well as on the interval: mounting with `active` already // true would otherwise show a value up to intervalMs stale. + // Subscribing to the clock, which is an external system — the case the + // rule's own documentation names as correct. Set once here as well as on + // the interval so a mount with `active` already true is not up to + // intervalMs stale. + // eslint-disable-next-line react-hooks/set-state-in-effect setNow(Date.now()); const timer = setInterval(() => setNow(Date.now()), intervalMs); return () => clearInterval(timer); diff --git a/frontend/src/customer/CustomerAuthContext.tsx b/frontend/src/customer/CustomerAuthContext.tsx index 8bc6ced..0a2b8d5 100755 --- a/frontend/src/customer/CustomerAuthContext.tsx +++ b/frontend/src/customer/CustomerAuthContext.tsx @@ -33,6 +33,10 @@ export function CustomerAuthProvider({ children }: Readonly<{ children: React.Re .finally(() => setLoading(false)); }, []); + // `refresh` sets a pending flag before fetching. The rule cannot tell a + // fetch with a spinner from a value that was already knowable, and this is + // the former. + // eslint-disable-next-line react-hooks/set-state-in-effect useEffect(() => { refresh(); }, [refresh]); // Logging out has to clear the context, not just call the endpoint — diff --git a/frontend/src/customer/FavoritesContext.tsx b/frontend/src/customer/FavoritesContext.tsx index 2e756e1..a998d05 100644 --- a/frontend/src/customer/FavoritesContext.tsx +++ b/frontend/src/customer/FavoritesContext.tsx @@ -31,10 +31,13 @@ export function FavoritesProvider({ children }: Readonly<{ children: React.React .catch(() => setFavorites([])); }, []); + // Same decision as CartProvider: clearing on sign-out is synchronisation + // with the session rather than derived state. See #99. useEffect(() => { if (customer) { refresh(); } else { + // eslint-disable-next-line react-hooks/set-state-in-effect setFavorites([]); } }, [customer, refresh]); diff --git a/frontend/src/customer/Orders.tsx b/frontend/src/customer/Orders.tsx index 8322bbf..1bd22ea 100644 --- a/frontend/src/customer/Orders.tsx +++ b/frontend/src/customer/Orders.tsx @@ -115,7 +115,9 @@ export default function Orders() { } }, []); - useEffect(() => { + // Load-on-mount once the session resolves, with a pending flag. + useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect if (customer) void load(); }, [customer, load]); diff --git a/frontend/src/customer/VerifyEmail.tsx b/frontend/src/customer/VerifyEmail.tsx index 85bf41a..5056f37 100644 --- a/frontend/src/customer/VerifyEmail.tsx +++ b/frontend/src/customer/VerifyEmail.tsx @@ -15,8 +15,14 @@ type Status = 'verifying' | 'verified' | 'failed'; export default function VerifyEmail() { const [searchParams] = useSearchParams(); const token = searchParams.get('token'); - const [status, setStatus] = useState('verifying'); - const [error, setError] = useState(null); + // Derived from the URL rather than discovered in an effect. Whether the link + // carries a token is knowable during the first render, so routing it through + // an effect made this component render once as a spinner in a state that was + // never true — a link with no token was never "verifying". See #99. + const [status, setStatus] = useState(token ? 'verifying' : 'failed'); + const [error, setError] = useState( + token ? null : 'This link is missing its verification token.' + ); const { refresh } = useCustomerAuth(); // The backend deletes the token once it succeeds, so a second request for the @@ -29,11 +35,8 @@ export default function VerifyEmail() { if (requested.current) return; requested.current = true; - if (!token) { - setStatus('failed'); - setError('This link is missing its verification token.'); - return; - } + // Nothing to do: the missing-token case is already the initial state. + if (!token) return; verifyEmail(token) .then(() => { diff --git a/frontend/src/useCatalogue.ts b/frontend/src/useCatalogue.ts index 0793b8c..6654e0c 100644 --- a/frontend/src/useCatalogue.ts +++ b/frontend/src/useCatalogue.ts @@ -86,6 +86,8 @@ export function useCatalogue(filters: ItemFilters, onAuthRequired: () => void): useEffect(() => { if (awaitingAuth) { + // A pending flag while the session resolves, not a value already known. + // eslint-disable-next-line react-hooks/set-state-in-effect setLoading(true); return; }