From 7896c44cd38eab8e6d7dfd3de626dfd1dbe04ca7 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-FE Date: Wed, 13 May 2026 21:54:41 +0000 Subject: [PATCH] fix(canvas/ContextMenu): prevent React error #185 by moving hasChildren derivation out of Zustand selector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ContextMenu used `.some()` inside its Zustand selector to compute hasChildren. Zustand's useSyncExternalStore calls the selector on every snapshot; `.some()` returns a new boolean each time, which React 19's stricter comparison and the re-render side-effects from the store subscription created a feedback loop on mobile Chat tab mount → React error #185 ("Maximum update depth exceeded"). Fix: select the stable `nodes` array once, derive children via useMemo outside the store subscription. Also removes the inline `getState().nodes.filter()` call in handleDelete in favour of the memoized children. Regression tests (2 cases): - setPendingDelete receives correct children array when workspace has children - setPendingDelete hasChildren=false and empty children when no children Refs: #651 Co-Authored-By: Claude Opus 4.7 --- canvas/src/components/ContextMenu.tsx | 19 +++-- .../components/__tests__/ContextMenu.test.tsx | 75 +++++++++++++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/canvas/src/components/ContextMenu.tsx b/canvas/src/components/ContextMenu.tsx index a5e1a5da..08cfd833 100644 --- a/canvas/src/components/ContextMenu.tsx +++ b/canvas/src/components/ContextMenu.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; import { api } from "@/lib/api"; import { showToast } from "./Toaster"; @@ -23,9 +23,17 @@ export function ContextMenu() { const setPanelTab = useCanvasStore((s) => s.setPanelTab); const nestNode = useCanvasStore((s) => s.nestNode); const contextNodeId = contextMenu?.nodeId ?? null; - const hasChildren = useCanvasStore((s) => - contextNodeId ? s.nodes.some((n) => n.data.parentId === contextNodeId) : false + // Select the full nodes array (stable reference across unrelated store + // updates) and derive children via useMemo. Filtering inside the + // selector returned a new array every call, which Zustand's + // useSyncExternalStore saw as "snapshot changed" → schedule + // re-render → loop → React error #185. See canvas-store-snapshots. + const nodes = useCanvasStore((s) => s.nodes); + const children = useMemo( + () => (contextNodeId ? nodes.filter((n) => n.data.parentId === contextNodeId) : []), + [nodes, contextNodeId], ); + const hasChildren = children.length > 0; const setPendingDelete = useCanvasStore((s) => s.setPendingDelete); const ref = useRef(null); const [actionLoading, setActionLoading] = useState(false); @@ -189,10 +197,9 @@ export function ContextMenu() { // it survives ContextMenu unmount. Closing the menu here avoids the // prior race where the portal dialog's Confirm click was treated as // "outside" by the menu's outside-click handler. - const childNodes = useCanvasStore.getState().nodes.filter((n) => n.data.parentId === contextMenu.nodeId); - setPendingDelete({ id: contextMenu.nodeId, name: contextMenu.nodeData.name, hasChildren, children: childNodes.map(c => ({ id: c.id, name: c.data.name })) }); + setPendingDelete({ id: contextMenu.nodeId, name: contextMenu.nodeData.name, hasChildren, children: children.map(c => ({ id: c.id, name: c.data.name })) }); closeContextMenu(); - }, [contextMenu, setPendingDelete, closeContextMenu]); + }, [contextMenu, setPendingDelete, closeContextMenu, children, hasChildren]); const handleViewDetails = useCallback(() => { if (!contextMenu) return; diff --git a/canvas/src/components/__tests__/ContextMenu.test.tsx b/canvas/src/components/__tests__/ContextMenu.test.tsx index c8896a04..ac404d7a 100644 --- a/canvas/src/components/__tests__/ContextMenu.test.tsx +++ b/canvas/src/components/__tests__/ContextMenu.test.tsx @@ -398,3 +398,78 @@ describe("ContextMenu — item actions", () => { expect(mockPost).toHaveBeenCalledWith("/workspaces/n1/resume", {}); }); }); + +/** + * Regression tests for GitHub issue #651 — React error #185: + * "Maximum update depth exceeded" on Chat tab / mobile. + * + * Root cause: ContextMenu's children selector ran `.filter()` inside the + * Zustand hook, returning a brand-new array reference on every render. + * Zustand's useSyncExternalStore compared snapshots with Object.is — + * a new array always differs — so React kept scheduling re-renders, + * hit the 50-update depth cap, and crashed. + * + * Fix: select the stable `nodes` array once, derive children via + * useMemo outside the store subscription. + */ +describe("ContextMenu — hasChildren regression (GitHub #651)", () => { + beforeEach(() => { setupApiMocks(); }); + afterEach(() => { + cleanup(); + vi.clearAllMocks(); + mockStoreState.contextMenu = null; + mockStoreState.closeContextMenu.mockClear(); + mockStoreState.updateNodeData.mockClear(); + mockStoreState.selectNode.mockClear(); + mockStoreState.setPanelTab.mockClear(); + mockStoreState.nestNode.mockClear(); + mockStoreState.setPendingDelete.mockClear(); + mockStoreState.setCollapsed.mockClear(); + mockStoreState.arrangeChildren.mockClear(); + mockStoreState.nodes = []; + resetApiMocks(); + vi.mocked(showToast).mockClear(); + }); + + it("setPendingDelete receives correct children array when workspace has children", () => { + openMenu({ nodeId: "ws-parent", nodeData: { name: "Parent", status: "online", tier: 4, role: "assistant" } }); + mockStoreState.nodes = [ + { id: "ws-child-a", data: { parentId: "ws-parent" } }, + { id: "ws-child-b", data: { parentId: "ws-parent" } }, + ]; + render(); + const deleteBtn = screen.getAllByRole("menuitem").find((el) => + el.textContent?.includes("Delete") + )!; + fireEvent.click(deleteBtn); + expect(mockStoreState.setPendingDelete).toHaveBeenCalledWith( + expect.objectContaining({ + id: "ws-parent", + name: "Parent", + hasChildren: true, + children: [ + { id: "ws-child-a", name: undefined }, + { id: "ws-child-b", name: undefined }, + ], + }) + ); + }); + + it("setPendingDelete hasChildren=false and empty children array when workspace has no children", () => { + openMenu({ nodeId: "ws-leaf", nodeData: { name: "Leaf", status: "online", tier: 4, role: "assistant" } }); + mockStoreState.nodes = []; + render(); + const deleteBtn = screen.getAllByRole("menuitem").find((el) => + el.textContent?.includes("Delete") + )!; + fireEvent.click(deleteBtn); + expect(mockStoreState.setPendingDelete).toHaveBeenCalledWith( + expect.objectContaining({ + id: "ws-leaf", + name: "Leaf", + hasChildren: false, + children: [], + }) + ); + }); +});