Created: 2026-03-01
Updated: 2026-03-01
This document tracks the iterative state management improvements applied to LimitedTimeSmithingCalculator during a code review session guided by the Vercel React Best Practices guidelines.
The component managed 10 form input fields (pity cap, current score, 4 ofuda ticket counts, 4 resource amounts), each with its own useState call. A useEffect persisted all inputs to localStorage reactively:
// 10 individual state declarations
const [pityCap, setPityCap] = useState<number | "">(() => initialInputs.pityCap);
const [currentScore, setCurrentScore] = useState<number | "">(() => initialInputs.currentScore);
// ...8 more
// Persistence effect with 12-item dependency array
useEffect(() => {
save({ pityCap: num(pityCap) || DEFAULT_INPUTS.pityCap, ... });
}, [save, pityCap, currentScore, umeCount, takeCount, matsuCount, fujiCount, charcoal, steel, coolant, whetstone]);
Rule: "Put Interaction Logic in Event Handlers" — side effects caused by user interaction should live in the event handler, not in a reactive effect.
The useEffect was replaced with a saveCurrentState helper called directly inside the input handler. Because React setState is asynchronous, the just-changed value hadn't committed to state yet at call time. An overrides parameter was introduced as a workaround to pass the new value explicitly:
function saveCurrentState(overrides: Partial<StoredInputs> = {}) {
save({ pityCap: num(pityCap) || DEFAULT_INPUTS.pityCap, ..., ...overrides });
}
function createNumberHandler(field: keyof StoredInputs, setter: (v: number | "") => void) {
return (e) => {
setter(numericValue);
saveCurrentState({ [field]: numericValue ?? 0 }); // overrides workaround
};
}
This improved explicitness but introduced an awkward pattern — the overrides spread exists solely because React state isn't synchronous.
Rule: "Use Functional setState Updates" — setState(prev => next) always has the correct current value, preventing stale closures.
During review, the core problem was identified: the overrides workaround exists entirely because individual setState calls don't commit immediately. The solution was to consolidate all 10 input fields into one useState<InputState> object and use a functional update, which gives direct access to next — a single source of truth that can be saved without any guesswork:
setInputs(prev => {
const next = { ...prev, [field]: parsed };
save(toStoredInputs(next)); // next is exactly what will be committed
return next;
});
useReducer Was Considered and RejectedThe initial question was whether to consolidate all useState calls into a useReducer. The analysis concluded no, for these reasons:
No performance benefit. React 18 automatically batches all setState calls within the same event handler into a single re-render. Whether handleReset calls 10 setters or one dispatch, the result is identical: one re-render.
The Vercel guidelines do not recommend it. Across all 57 rules in the guide, useReducer is not mentioned. There is no applicable rule that motivates the change.
Disproportionate boilerplate. A reducer requires a discriminated union of action types, a reducer function with switch cases, and updated call sites — roughly 30-40 lines of overhead for state transitions that are fundamentally simple (each field changes independently).
A partial useReducer — state machine boundary. A secondary suggestion was a "well-shaped" reducer with actions FIELD_CHANGE, CALCULATE_SUCCESS, CALCULATE_ERROR, RESET. While this is architecturally valid and removes the overrides workaround (because the reducer receives the next state atomically), it still requires full boilerplate. Since persistence is a side effect that must live outside the reducer anyway, the single-object useState approach achieves the same outcome with less ceremony.
// Mirrors StoredInputs but allows "" so fields can be cleared while typing
type InputState = { [K in keyof StoredInputs]: StoredInputs[K] | "" };
const DEFAULT_INPUT_STATE: InputState = { ...DEFAULT_INPUTS };
// Pure conversion for localStorage — coerces "" to 0, guards pityCap fallback
function toStoredInputs(state: InputState): StoredInputs { ... }
// Before: 10 lines
const [pityCap, setPityCap] = useState<number | "">(...);
// ...9 more
// After: 1 line
const [inputs, setInputs] = useState<InputState>(() => ({ ...initialInputs }));
createNumberHandler simplified// Before: required (field, setter) — called setter, then saveCurrentState with overrides
function createNumberHandler(field: keyof StoredInputs, setter: (v: number | "") => void)
// After: only field — functional update eliminates setter and overrides entirely
function createNumberHandler(field: keyof StoredInputs) {
return (e) => {
setInputs(prev => {
const next = { ...prev, [field]: parsed };
save(toStoredInputs(next));
return next;
});
};
}
handleReset simplified// Before: 10 individual setter calls
setPityCap(DEFAULT_INPUTS.pityCap);
setCurrentScore(DEFAULT_INPUTS.currentScore);
// ...8 more
// After: 1 call
setInputs({ ...DEFAULT_INPUT_STATE });
| Area | Before | After |
|---|---|---|
| State declarations | 10 useState calls |
1 useState<InputState> |
| Persistence mechanism | saveCurrentState(overrides) workaround |
Functional update — next is the truth |
createNumberHandler params |
(field, setter) |
(field) only |
handleReset |
10 setter calls | 1 setInputs call |
| Stale closure risk | Present — overrides compensated for it | Eliminated |
| Alignment with Vercel Rule 5.9 | No | Yes — functional setState |
Every JSX binding becomes inputs.pityCap instead of pityCap. This is slightly more verbose in the template. For 10 fields it adds minimal noise.
A single state update re-renders the component on every field change. This is identical to the previous behaviour — 10 individual useState calls would each trigger a re-render on their own field change regardless.
All 10 fields are in one object. If in the future some fields need to be conditionally shown or independently memoized, splitting them out again would require more work than they were in individual state. For this component's current structure this is not a concern.
toStoredInputs is a required conversion layer. InputState allows "" (for empty input boxes), but StoredInputs requires number (for JSON serialization). The conversion function is a small overhead that would not exist if the types were unified — but unifying them would mean storing "" in localStorage, which is undesirable.