From 80d66538c25c81fd6dfbcf61ce6d994cacf27e46 Mon Sep 17 00:00:00 2001 From: Andre Padez Date: Mon, 10 Aug 2026 21:51:04 +0100 Subject: [PATCH] fix the render loop I was warned about in the file I edited MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit React #185, maximum update depth, and the page with it. usePublishChatTabName named useGlobal setter as an effect dependency. useGlobal rebuilds that setter every render, so the effect re-ran every render, set global state, and rendered again. The publisher directly above it in the same file documents this exact hazard — I copied the shape and not the reason. Now through a ref, depending on the string alone, identical to usePublishPageTitle. Also stabilised setPaneTarget with useCallback. It is handed to every pane as onChange and a pane puts it in a context others read, so a fresh identity each render is the same loop waiting for the first consumer that depends on it. The active tab key is read through a ref so it never has to be a dependency. Co-Authored-By: Claude Opus 5 --- .../src/apps/ChatHistory/ChatTabs.tsx | 25 +++++++++++++------ src/workspaces/officerdev/src/page-title.ts | 16 +++++++----- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/workspaces/officerdev/src/apps/ChatHistory/ChatTabs.tsx b/src/workspaces/officerdev/src/apps/ChatHistory/ChatTabs.tsx index 896c2d28..2dbefff3 100644 --- a/src/workspaces/officerdev/src/apps/ChatHistory/ChatTabs.tsx +++ b/src/workspaces/officerdev/src/apps/ChatHistory/ChatTabs.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { Columns2, Plus, X } from 'lucide-react'; import { connectionLabel } from 'hooks/connections'; import { usePublishChatTabName } from '../../page-title'; @@ -63,6 +63,9 @@ export const ChatTabs = () => { const [renamingKey, setRenamingKey] = useState(null); const [renameValue, setRenameValue] = useState(''); const restored = useRef(false); + // Read inside the stable callback above, so it never has to be a dependency. + const activeKeyRef = useRef(activeKey); + activeKeyRef.current = activeKey; // First render picks the first tab; afterwards the user owns it. useEffect(() => { @@ -132,12 +135,20 @@ export const ChatTabs = () => { tab.panes.length <= 1 ? tab : { ...tab, panes: tab.panes.filter((pane) => pane.key !== paneKey) }, ); - const setPaneTarget = (paneKey: string, target: SelectedSession | null) => - active && - update(active.key, (tab) => ({ - ...tab, - panes: tab.panes.map((pane) => (pane.key === paneKey ? { ...pane, target } : pane)), - })); + // Stable across renders on purpose. It is handed to every pane as `onChange`, and a pane passes it + // into a context that other components read — an identity that changed every render would make any + // effect depending on it re-run forever, which is the render loop this file already caused once. + const setPaneTarget = useCallback( + (paneKey: string, target: SelectedSession | null) => + setTabs((prev) => + prev.map((tab) => + tab.key !== activeKeyRef.current + ? tab + : { ...tab, panes: tab.panes.map((pane) => (pane.key === paneKey ? { ...pane, target } : pane)) }, + ), + ), + [], + ); if (!active) return null; diff --git a/src/workspaces/officerdev/src/page-title.ts b/src/workspaces/officerdev/src/page-title.ts index 8f46a446..fa148fb2 100644 --- a/src/workspaces/officerdev/src/page-title.ts +++ b/src/workspaces/officerdev/src/page-title.ts @@ -76,12 +76,16 @@ export const useChatTabName = () => useGlobal(CHAT_TAB_NAME, null */ export function usePublishChatTabName(name: string | null) { const [, setName] = useGlobal(CHAT_TAB_NAME, null); - const nameRef = useRef(name); - nameRef.current = name; + + // Through a ref, and depending on the STRING only — exactly as `usePublishPageTitle` does, for the + // reason documented there: `useGlobal` rebuilds its setter every render, so naming it as a dependency + // re-runs this effect on every render, which sets global state, which renders again. That is React + // error #185, and it took the whole page down until the deps were narrowed to the value itself. + const setNameRef = useRef(setName); + setNameRef.current = setName; useEffect(() => { - setName(nameRef.current); - }, [name, setName]); - - useEffect(() => () => setName(null), [setName]); + setNameRef.current(name); + return () => setNameRef.current(null); + }, [name]); }