refactor(frontend): triage the setState-in-effect sites (#99) #158
@@ -78,6 +78,8 @@ export default function Categories() {
|
|||||||
.finally(() => setLoading(false));
|
.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(); }, []);
|
useEffect(() => { void load(); }, []);
|
||||||
|
|
||||||
function openNew(parent: number | null) {
|
function openNew(parent: number | null) {
|
||||||
|
|||||||
@@ -36,6 +36,8 @@ export default function Tags() {
|
|||||||
.finally(() => setLoading(false));
|
.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(); }, []);
|
useEffect(() => { void load(); }, []);
|
||||||
|
|
||||||
function openNew() {
|
function openNew() {
|
||||||
|
|||||||
@@ -67,6 +67,9 @@ export default function Cart() {
|
|||||||
.finally(() => setLoading(false));
|
.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]);
|
useEffect(() => { if (customer) loadAll(); }, [customer]);
|
||||||
|
|
||||||
// Only ticks while there is something to count down, so an empty cart does
|
// 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.
|
// the release rather than sitting at "expiring…" until the page is reloaded.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!lapsed) return;
|
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();
|
loadAll();
|
||||||
// The header badge counts held items too, so it goes stale in exactly the
|
// 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
|
// same way. Safe as a dependency: CartContext memoizes it with an empty
|
||||||
|
|||||||
@@ -24,10 +24,16 @@ export function CartProvider({ children }: Readonly<{ children: React.ReactNode
|
|||||||
.catch(() => setItems([]));
|
.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(() => {
|
useEffect(() => {
|
||||||
if (customer) {
|
if (customer) {
|
||||||
refresh();
|
refresh();
|
||||||
} else {
|
} else {
|
||||||
|
// eslint-disable-next-line react-hooks/set-state-in-effect
|
||||||
setItems([]);
|
setItems([]);
|
||||||
}
|
}
|
||||||
}, [customer, refresh]);
|
}, [customer, refresh]);
|
||||||
|
|||||||
@@ -25,6 +25,11 @@ export function useNow(intervalMs: number, active: boolean): number {
|
|||||||
if (!active) return;
|
if (!active) return;
|
||||||
// Set immediately as well as on the interval: mounting with `active` already
|
// Set immediately as well as on the interval: mounting with `active` already
|
||||||
// true would otherwise show a value up to intervalMs stale.
|
// 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());
|
setNow(Date.now());
|
||||||
const timer = setInterval(() => setNow(Date.now()), intervalMs);
|
const timer = setInterval(() => setNow(Date.now()), intervalMs);
|
||||||
return () => clearInterval(timer);
|
return () => clearInterval(timer);
|
||||||
|
|||||||
@@ -33,6 +33,10 @@ export function CustomerAuthProvider({ children }: Readonly<{ children: React.Re
|
|||||||
.finally(() => setLoading(false));
|
.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]);
|
useEffect(() => { refresh(); }, [refresh]);
|
||||||
|
|
||||||
// Logging out has to clear the context, not just call the endpoint —
|
// 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([]));
|
.catch(() => setFavorites([]));
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
// Same decision as CartProvider: clearing on sign-out is synchronisation
|
||||||
|
// with the session rather than derived state. See #99.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (customer) {
|
if (customer) {
|
||||||
refresh();
|
refresh();
|
||||||
} else {
|
} else {
|
||||||
|
// eslint-disable-next-line react-hooks/set-state-in-effect
|
||||||
setFavorites([]);
|
setFavorites([]);
|
||||||
}
|
}
|
||||||
}, [customer, refresh]);
|
}, [customer, refresh]);
|
||||||
|
|||||||
@@ -115,7 +115,9 @@ export default function Orders() {
|
|||||||
}
|
}
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
// Load-on-mount once the session resolves, with a pending flag.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
// eslint-disable-next-line react-hooks/set-state-in-effect
|
||||||
if (customer) void load();
|
if (customer) void load();
|
||||||
}, [customer, load]);
|
}, [customer, load]);
|
||||||
|
|
||||||
|
|||||||
@@ -15,8 +15,14 @@ type Status = 'verifying' | 'verified' | 'failed';
|
|||||||
export default function VerifyEmail() {
|
export default function VerifyEmail() {
|
||||||
const [searchParams] = useSearchParams();
|
const [searchParams] = useSearchParams();
|
||||||
const token = searchParams.get('token');
|
const token = searchParams.get('token');
|
||||||
const [status, setStatus] = useState<Status>('verifying');
|
// Derived from the URL rather than discovered in an effect. Whether the link
|
||||||
const [error, setError] = useState<string | null>(null);
|
// 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();
|
const { refresh } = useCustomerAuth();
|
||||||
|
|
||||||
// The backend deletes the token once it succeeds, so a second request for the
|
// 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;
|
if (requested.current) return;
|
||||||
requested.current = true;
|
requested.current = true;
|
||||||
|
|
||||||
if (!token) {
|
// Nothing to do: the missing-token case is already the initial state.
|
||||||
setStatus('failed');
|
if (!token) return;
|
||||||
setError('This link is missing its verification token.');
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
verifyEmail(token)
|
verifyEmail(token)
|
||||||
.then(() => {
|
.then(() => {
|
||||||
|
|||||||
@@ -86,6 +86,8 @@ export function useCatalogue(filters: ItemFilters, onAuthRequired: () => void):
|
|||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (awaitingAuth) {
|
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);
|
setLoading(true);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user