bermudalamb
45f1c77160
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
40 lines
1.8 KiB
TypeScript
40 lines
1.8 KiB
TypeScript
import { useEffect, useState } from 'react';
|
|
|
|
/**
|
|
* The current time, as state that advances on an interval.
|
|
*
|
|
* Returns a value rather than merely forcing a re-render, and that is the point.
|
|
* A component that calls `Date.now()` while rendering produces output that
|
|
* depends on the clock, which React is entitled to assume it does not — the
|
|
* `react-hooks/purity` rule says so, and it was the only instance in this
|
|
* codebase precisely because it was the only place doing it. Reading `now` from
|
|
* state makes render a function of its inputs again.
|
|
*
|
|
* It also fixes the defect underneath that rule. Nothing scheduled a re-render,
|
|
* so the cart's countdown was a still photograph of the moment the page loaded,
|
|
* and the red warning in the final stretch could only appear by accident — the
|
|
* component had already rendered before the final stretch began.
|
|
*
|
|
* `active` exists so an idle page is not waking React forever: an empty cart has
|
|
* nothing to count down, so it does not tick at all.
|
|
*/
|
|
export function useNow(intervalMs: number, active: boolean): number {
|
|
const [now, setNow] = useState(() => Date.now());
|
|
|
|
useEffect(() => {
|
|
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);
|
|
}, [intervalMs, active]);
|
|
|
|
return now;
|
|
}
|