From a0332c41592d19968a562e93c6bb4c3fa181962f Mon Sep 17 00:00:00 2001 From: SJCoetz Date: Mon, 29 Jun 2026 23:20:16 +0200 Subject: [PATCH] fix(approve modal): break infinite loop from openNonce in effect deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- src/app/(app)/quotes/[id]/page.tsx | 33 +++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/app/(app)/quotes/[id]/page.tsx b/src/app/(app)/quotes/[id]/page.tsx index 4959d2c..6faee6b 100644 --- a/src/app/(app)/quotes/[id]/page.tsx +++ b/src/app/(app)/quotes/[id]/page.tsx @@ -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(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