fix(toast): memoize useToast factory to neutralize storm loop#601
Conversation
The ToastProvider value was an object literal recreated on every render. Any consumer that included `toast` in a useCallback or useEffect dep array would rebuild on every render of the provider, which renders every time addToast updates `toasts` state. If that consumer's effect called toast.error on failure (e.g. AdminAccessRequests.jsx loading /api/v1/pro/portal/admin/access-requests, which 403s when PRO is unlicensed), the loop self-amplifies: error → toast.error → provider re-render → new toast ref → useCallback rebuilds → useEffect re-fires → error → ... Wrapping the factory in useMemo([addToast]) makes the context value stable across re-renders. addToast itself is stable (useCallback with []), so the memo cache effectively pins the toast object for the lifetime of the provider. Independent fix; the underlying 403-on-every-PRO-route trigger is addressed separately in PR #600 (license cache schema PR-03). Closes cortex_observe #70. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Council Results0 tests 0 ✅ 0s ⏱️ Results for commit a316380. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Stabilizes the ToastProvider context value by memoizing the toast factory object so consumers can safely include toast in hook dependency arrays without triggering effect/callback rebuild loops on every toast render.
Changes:
- Add
useMemotoToast.jsximports. - Wrap the
toastobject literal inuseMemo([addToast])to preserve referential equality across provider re-renders. - Document the rationale (prevents “toast storm” loop when consumers call
toast.errorinside effects).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Wraps the
toastfactory object inToastProviderwithuseMemo([addToast])so the context value is stable across renders. Without it, every render ofToastProvider(which happens on everysetToasts(...)call) produces a new object literal, breaking referential equality for every consumer that depends ontoast.Closes
cortex_observe #70(filed againstAdminAccessRequests.jsxduring the 2026-05-10 incident).The loop this prevents
AdminAccessRequests.jsx:31-51:
```jsx
const fetchRequests = useCallback(async () => {
try {
const data = await api.get('/api/v1/pro/portal/admin/access-requests');
setRequests(data);
} catch (err) {
toast.error('Failed to load access requests');
} finally { setLoading(false); }
}, [api, filter, toast]);
useEffect(() => {
if (isPro && !flagsLoading) fetchRequests();
}, [isPro, flagsLoading, fetchRequests]);
```
If the fetch fails (e.g. PRO routes returning 403 `no_license` during a schema drift like Aeonyx #148), the chain is:
Visible as a pile-up of identical toasts plus continuous request load.
Independence from PR #600
This is an independent defensive fix. PR #600 fixes the underlying `license_gate` 403 chain that triggered the loop on the BLB3D VM. This PR ensures the same class of loop can't be triggered by any future 4xx on any admin page that calls `toast.error` inside a useEffect. They can land in either order.
Why useMemo is correct here
`addToast` is wrapped in `useCallback([], ...)` — its identity is stable across the lifetime of the provider. The memo cache therefore never invalidates (the dep array of `[addToast]` is effectively a no-op dep array), and the four method references inside the factory remain stable too.
Test plan
Out of scope
🤖 Generated with Claude Code
Agent-Session: 0f282673-19c5-4c14-b7ad-b397d94cc586