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
69 lines
2.3 KiB
TypeScript
Executable File
69 lines
2.3 KiB
TypeScript
Executable File
import React, { createContext, useContext, useEffect, useMemo, useState, useCallback } from 'react';
|
|
import { Customer, fetchMe, logoutCustomer } from './customerApi';
|
|
|
|
interface CustomerAuthValue {
|
|
customer: Customer | null;
|
|
loading: boolean;
|
|
refresh: () => void;
|
|
logout: () => Promise<void>;
|
|
}
|
|
|
|
const CustomerAuthContext = createContext<CustomerAuthValue>({
|
|
customer: null,
|
|
loading: true,
|
|
refresh: () => {},
|
|
logout: async () => {}
|
|
});
|
|
|
|
export function useCustomerAuth() {
|
|
return useContext(CustomerAuthContext);
|
|
}
|
|
|
|
export function CustomerAuthProvider({ children }: Readonly<{ children: React.ReactNode }>) {
|
|
const [customer, setCustomer] = useState<Customer | null>(null);
|
|
const [loading, setLoading] = useState(true);
|
|
|
|
const refresh = useCallback(() => {
|
|
setLoading(true);
|
|
// A rejection here previously left `loading` true forever, which renders
|
|
// as a permanent spinner rather than a signed-out page.
|
|
void fetchMe()
|
|
.then(c => setCustomer(c))
|
|
.catch(() => setCustomer(null))
|
|
.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 —
|
|
// otherwise `customer` stays set and the header keeps offering "My Account"
|
|
// until something happens to refetch. Clearing here rather than calling
|
|
// refresh() avoids a window where the session is gone but the UI still shows
|
|
// the customer as signed in.
|
|
//
|
|
// Rejects if the server did not accept the logout, leaving the customer
|
|
// signed in, because the session cookie is still valid in that case.
|
|
const logout = useCallback(async () => {
|
|
await logoutCustomer();
|
|
setCustomer(null);
|
|
setLoading(false);
|
|
}, []);
|
|
|
|
// Memoized because this is the outermost provider: an unmemoized value makes
|
|
// every signed-in-aware component in the tree re-render on any parent render.
|
|
const value = useMemo(
|
|
() => ({ customer, loading, refresh, logout }),
|
|
[customer, loading, refresh, logout]
|
|
);
|
|
|
|
return (
|
|
<CustomerAuthContext.Provider value={value}>
|
|
{children}
|
|
</CustomerAuthContext.Provider>
|
|
);
|
|
}
|