refactor(frontend): triage the setState-in-effect sites (#99) #158

Merged
bermudalamb merged 1 commits from feature/99-setstate-triage into main 2026-08-24 12:19:51 -05:00
10 changed files with 44 additions and 8 deletions
Showing only changes of commit 45f1c77160 - Show all commits
+2
View File
@@ -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) {
+2
View File
@@ -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() {
+7
View File
@@ -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
+6
View File
@@ -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]);
+5
View File
@@ -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);
@@ -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 —
@@ -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]);
+3 -1
View File
@@ -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]);
+10 -7
View File
@@ -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<Status>('verifying');
const [error, setError] = useState<string | null>(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<Status>(token ? 'verifying' : 'failed');
const [error, setError] = useState<string | null>(
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(() => {
+2
View File
@@ -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;
}