-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Move pendingNewChat navigation logic from useEffect to event handler #1016
Copy link
Copy link
Closed
Labels
Description
Description
In LayoutDataProvider.tsx, the "new chat" navigation is split across an event handler and a useEffect via a pendingNewChat boolean flag. The handler sets the flag (line 553), and a separate effect watches it plus params?.chat_id to complete the navigation (lines 261-267). This is the classic "event → setState → effect" anti-pattern that React docs warn against.
Vercel React Best Practices Rule: rerender-move-effect-to-event (5.7)
File to change
surfsense_web/components/layout/providers/LayoutDataProvider.tsx
What to do
-
Remove the
pendingNewChatstate and its useEffect (lines 259-268). -
Refactor
handleNewChat(lines 544-558) to handle the full navigation in one flow:
const handleNewChat = useCallback(() => {
const isOutOfSync = currentThreadState.id !== null && !params?.chat_id;
if (isOutOfSync) {
resetCurrentThread();
router.replace(`/dashboard/${searchSpaceId}/new-chat/${currentThreadState.id}`);
// Allow router to sync, then navigate to fresh new-chat
setTimeout(() => {
router.push(`/dashboard/${searchSpaceId}/new-chat`);
}, 0);
} else {
router.push(`/dashboard/${searchSpaceId}/new-chat`);
}
}, [router, searchSpaceId, currentThreadState.id, params?.chat_id, resetCurrentThread]);- Remove
pendingNewChatandsetPendingNewChatfrom the component entirely.
Acceptance criteria
- "New Chat" button still works from any state (fresh page, existing chat, out-of-sync URL)
- No
pendingNewChatstate or associated useEffect exists - Navigation flow is traceable in a single function
Reactions are currently unavailable