Eleven warnings that looked alike and were not. This is a decision per site rather than eleven fixes, which is what the issue asked for — some of these would be made worse by "fixing" them. One was a real defect. VerifyEmail routed a fact through an effect that was already knowable during render: whether the link carries a token comes from the URL. The component therefore rendered once as a spinner in a state that was never true — a link with no token was never "verifying". Both pieces of state now derive their initial value from the token, and the effect's missing-token branch becomes an early return, so the failure is what the first render shows. Two are a defensible reset. CartProvider and FavoritesProvider clear their collection when the customer becomes null, which is synchronisation with the session rather than derived state. Deriving instead would push "signed out" onto every consumer of those contexts, and remounting on a `key` is more indirection than the problem deserves. Decided and written down rather than left for the next reader to re-investigate. Eight are legitimate and flagged conservatively. Six are a pending flag before a fetch — the rule cannot tell a spinner from a value that was already known. Cart's lapsed-item refetch is synchronisation with a server-side release the client cannot observe. useNow subscribes to the clock, which is the case the rule's own documentation names as correct. Each of the ten that stay carries the reason and a targeted disable, so lint drops from thirteen warnings to two — and the two left are the unrelated no-alphabetical-sort pair. Suppressing per site rather than switching the rule off keeps it live for new code, which is where the next VerifyEmail would be caught. The trap #60 recorded caught this, in a variant it does not describe. Placing the disable above `useEffect(` works only for a single-line effect: where the effect spans several lines the flagged line is the setState inside the body, so the directive covered nothing and produced both an unused-disable warning and the original one. Three sites were wrong that way on the first attempt. Confirmed fixed by the absence of "Unused eslint-disable directive" from the output — a disable that covers nothing reports itself, which is what makes this checkable rather than assumed. Verified: tsc clean over src and tests, and the specs covering the changed behaviour pass — verify-email, auth, favorites, orders, cart-countdown, resend-verification, 32 of 33 with the one failure passing 10/10 in a serial re-run. Closes #99
127 lines
5.1 KiB
TypeScript
127 lines
5.1 KiB
TypeScript
import { useCallback, useEffect, useState } from 'react';
|
|
import { Item, FilterOptions, fetchItems, fetchFilterOptions } from './api';
|
|
import { ItemFilters, filtersFromSearchParams, filtersToSearchParams } from './filters';
|
|
import { useCustomerAuth } from './customer/CustomerAuthContext';
|
|
|
|
/**
|
|
* Fetching the catalogue for a set of filters.
|
|
*
|
|
* This was sixty lines sharing scope with the page that renders a header, a
|
|
* footer, a filter drawer and an auth modal — six pieces of state, four
|
|
* callbacks and two effects, none of which are about laying anything out. #81
|
|
* took the rendering half out by extracting `Catalogue`; this is the state half.
|
|
*
|
|
* The page keeps the URL as the source of filter truth, because that genuinely
|
|
* belongs to it: a reload, a shared link and the back button all have to restore
|
|
* the same view. What moves here is the request, the debounce, and the session
|
|
* negotiation the request depends on.
|
|
*/
|
|
|
|
// Dragging the price slider fires a change per pixel; without this every one
|
|
// would become its own request.
|
|
const FILTER_DEBOUNCE_MS = 250;
|
|
|
|
export interface Catalogue {
|
|
items: Item[];
|
|
loading: boolean;
|
|
/** A failed request, which must never be rendered as an empty catalogue. */
|
|
failed: boolean;
|
|
options: FilterOptions | null;
|
|
/** The favorites filter is set, the session has resolved, and nobody is signed in. */
|
|
needsFavoritesAuth: boolean;
|
|
/** Refetch both the items and the filter bounds — after a cart change, say. */
|
|
reload: () => void;
|
|
retry: () => void;
|
|
/**
|
|
* A stable identity for the current filter set.
|
|
*
|
|
* Returned rather than recomputed by the caller because it is already derived
|
|
* here, and the page needs exactly this value: it resets the catalogue's error
|
|
* boundary, so changing the filters gives a crashed grid another chance
|
|
* without a reload.
|
|
*/
|
|
filterKey: string;
|
|
}
|
|
|
|
/**
|
|
* @param onAuthRequired Called when the favorites filter is asked for by someone
|
|
* signed out. The hook decides the moment; the page decides what to do about it,
|
|
* because the prompt is a modal the page owns. Must be stable, or it re-triggers
|
|
* the effect below on every render.
|
|
*/
|
|
export function useCatalogue(filters: ItemFilters, onAuthRequired: () => void): Catalogue {
|
|
const [items, setItems] = useState<Item[]>([]);
|
|
const [loading, setLoading] = useState(true);
|
|
const [failed, setFailed] = useState(false);
|
|
const [options, setOptions] = useState<FilterOptions | null>(null);
|
|
|
|
const { customer, loading: authLoading } = useCustomerAuth();
|
|
|
|
// "Only my favorites" needs to know who is asking. Until the session has
|
|
// resolved we hold rather than guess: firing the request early would 401 and
|
|
// show the outage banner to someone who is in fact signed in.
|
|
const awaitingAuth = filters.favoritesOnly && authLoading;
|
|
const needsFavoritesAuth = filters.favoritesOnly && !authLoading && !customer;
|
|
|
|
// Serialised, then parsed back inside `load`. The round trip looks redundant
|
|
// and is not: depending on a *string* means `load` stays referentially stable
|
|
// while the filters are value-equal, and `load` is what the debounce effect
|
|
// below depends on. Depending on the object instead would give a new `load`
|
|
// every render, restarting the debounce each time and firing a request per
|
|
// keystroke.
|
|
const filterKey = filtersToSearchParams(filters).toString();
|
|
|
|
const load = useCallback(() => {
|
|
return fetchItems(filtersFromSearchParams(new URLSearchParams(filterKey)))
|
|
.then((loaded) => {
|
|
setItems(loaded);
|
|
setFailed(false);
|
|
})
|
|
// A failed request must never fall through to the empty state: telling a
|
|
// customer "no items yet" when the server is broken hides the outage and
|
|
// reads as an empty shop.
|
|
.catch(() => setFailed(true))
|
|
.finally(() => setLoading(false));
|
|
}, [filterKey]);
|
|
|
|
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;
|
|
}
|
|
// Prompt instead of requesting. The server would answer 401, and rendering
|
|
// that as "no items match these filters" would tell a signed-out visitor
|
|
// they have no favorites rather than that we do not know who they are.
|
|
if (needsFavoritesAuth) {
|
|
setItems([]);
|
|
setFailed(false);
|
|
setLoading(false);
|
|
onAuthRequired();
|
|
return;
|
|
}
|
|
setLoading(true);
|
|
const timer = setTimeout(() => void load(), FILTER_DEBOUNCE_MS);
|
|
return () => clearTimeout(timer);
|
|
}, [load, awaitingAuth, needsFavoritesAuth, onAuthRequired]);
|
|
|
|
useEffect(() => {
|
|
fetchFilterOptions().then(setOptions).catch(() => setOptions(null));
|
|
}, []);
|
|
|
|
// Adding to cart flips an item to reserved, and the filter options' price
|
|
// bounds shift as inventory changes.
|
|
const reload = useCallback(() => {
|
|
void load();
|
|
fetchFilterOptions().then(setOptions).catch(() => undefined);
|
|
}, [load]);
|
|
|
|
const retry = useCallback(() => {
|
|
setLoading(true);
|
|
void load();
|
|
}, [load]);
|
|
|
|
return { items, loading, failed, options, needsFavoritesAuth, reload, retry, filterKey };
|
|
}
|