fix(approve modal): break infinite loop from openNonce in effect deps
The ApproveAndSendDialog had a useEffect whose deps array included
`openNonce` — a state counter the effect itself incremented:
useEffect(() => {
if (!open) return;
setOpenNonce((n) => n + 1); // updates openNonce
// ... seed defaults ...
}, [open, openNonce, ...]); // openNonce is in deps
This produced Maximum update depth exceeded: the effect fires → state
updates → deps change → effect re-fires → loop, until React pulls the
plug.
The intent was 'seed once per open cycle, not on every claimerName /
repEmail update' (so async lookups arriving after the modal opened
didn't clobber operator edits). The state-counter approach was the
wrong mechanism for that — mutating a dep from inside the effect
*is* the loop.
Fix: replace openNonce (state) with seededFor (ref). A ref mutation
doesn't trigger re-render, so the loop is impossible. We still depend
on [open, quote.id] in the effect, and the ref's purpose is purely
'has this (open, quote.id) cycle been seeded yet?'. Reset to null on
open=false so the next open seeds fresh.
Dropped claimerName / repEmail from the deps too — they were the
reason openNonce existed in the first place (don't re-seed when they
update), and the ref now enforces that without the side-effect trap.
Tests: 229/230 vitest (the 1 failure is the pre-existing attachments
off-by-one, unrelated to this change).
This commit is contained in:
parent
9b2b0897a1
commit
a0332c4159
1 changed files with 23 additions and 10 deletions
|
|
@ -3065,15 +3065,23 @@ function ApproveAndSendDialog({
|
|||
/* eslint-enable react-hooks/set-state-in-effect */
|
||||
|
||||
// Initial form state — set when the modal opens, persisted when it
|
||||
// submits. We use a single open-counter so the effect runs ONCE per
|
||||
// open and not on every quote re-render.
|
||||
// submits. We seed ONCE per open cycle (not on every claimerName /
|
||||
// repEmail update) so async lookups that arrive after the modal
|
||||
// opens don't clobber the operator's in-progress edits.
|
||||
const [sendEmail, setSendEmail] = useState(true);
|
||||
const [cc, setCc] = useState("");
|
||||
const [bcc, setBcc] = useState("");
|
||||
const [subject, setSubject] = useState("");
|
||||
const [message, setMessage] = useState("");
|
||||
const [attachAuthLetters, setAttachAuthLetters] = useState(true);
|
||||
const [openNonce, setOpenNonce] = useState(0);
|
||||
// Tracks which (open, quote.id) cycle the form has been seeded for.
|
||||
// Reset to null whenever `open` is false so the next open seeds
|
||||
// fresh. We use a ref (not state) because mutating it shouldn't
|
||||
// re-trigger this effect — and critically, we don't include it in
|
||||
// the deps array (an earlier version used `openNonce` as a state
|
||||
// counter, but incrementing a state value inside an effect that's
|
||||
// dep'd on it produced an infinite loop).
|
||||
const seededFor = useRef<string | null>(null);
|
||||
// Track which attachments the operator wants to send. We pre-check
|
||||
// all of them (PDF + every outbound file); unchecking removes from
|
||||
// the outbound set.
|
||||
|
|
@ -3081,13 +3089,18 @@ function ApproveAndSendDialog({
|
|||
|
||||
/* eslint-disable react-hooks/set-state-in-effect, react-hooks/exhaustive-deps */
|
||||
useEffect(() => {
|
||||
if (!open) return;
|
||||
setOpenNonce((n) => n + 1);
|
||||
if (!open) {
|
||||
// Reset the seed marker so the next open seeds fresh defaults.
|
||||
seededFor.current = null;
|
||||
return;
|
||||
}
|
||||
// Seed exactly once per (open, quote.id) cycle. Subsequent prop
|
||||
// updates (claimerName arriving async, repEmail refreshing, etc.)
|
||||
// intentionally do NOT re-seed — the operator's edits win.
|
||||
if (seededFor.current === quote.id) return;
|
||||
seededFor.current = quote.id;
|
||||
// Seed defaults from the quote (persisted overrides win, then
|
||||
// fall back to customer/rep-derived values). We intentionally
|
||||
// don't depend on every `quote.*` field — we want to seed once
|
||||
// when the modal opens, not re-clobber the operator's edits
|
||||
// when something else updates the quote row.
|
||||
// fall back to customer/rep-derived values).
|
||||
const customer = quote.expand?.customer;
|
||||
const buyerEmail = (customer?.buyer_email || "").trim();
|
||||
setCc(quote.email_cc || buyerEmail);
|
||||
|
|
@ -3107,7 +3120,7 @@ function ApproveAndSendDialog({
|
|||
);
|
||||
setAttachAuthLetters(quote.attach_auth_letters !== false);
|
||||
setSendEmail(true);
|
||||
}, [open, openNonce, quote.id, claimerName, repEmail]);
|
||||
}, [open, quote.id]);
|
||||
/* eslint-enable react-hooks/set-state-in-effect, react-hooks/exhaustive-deps */
|
||||
|
||||
// Pre-select all outbound attachments when they change. We only
|
||||
|
|
|
|||
Loading…
Reference in a new issue