From 71b211de97662dd9132bbd821ebc1ad777898756 Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Fri, 15 May 2026 12:20:01 -0400 Subject: [PATCH] Fence + wall panels: stable handler refs via nodeRef to fix slider-drag loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User report: dragging the Length slider on a fence triggered "Maximum update depth exceeded" in updateNodesAction. Root cause: the panels' `handleUpdate` / `handleUpdateLength` useCallback deps included the subscribed `node` and `updateNode` references. On every store tick during slider drag (one per pointermove), Zustand notified subscribers → panel re-rendered → new `node` ref → new handler refs → SliderControl re-rendered with new onChange prop → its `useCallback([..., onChange])` for handleLabelPointerMove rebuilt while pointer capture was active. Combined with float drift in `getWallCurveLength` recomputing per render, React eventually flagged the cascade as a componentWillUpdate / componentDidUpdate loop. Fix: - Mirror `node` into a `nodeRef` updated on every render. Handlers read from `nodeRef.current` instead of closing over `node`. - Drop the subscribed `updateNode` dep: use `useScene.getState(). updateNode(...)` inside the handler. Same pattern ParametricInspector already uses for its registry-driven inspector. - Drop the now-redundant `useScene.getState().dirtyNodes.add(id)` call — updateNode's RAF markDirty already covers it. Net effect: handler refs are stable across slider drags (only change when `selectedId` changes). SliderControl's pointer listeners no longer churn mid-drag. Cycle broken. Same fix applied to wall-panel.tsx prophylactically — it has the identical pattern and would exhibit the same loop under the right float-drift / drag conditions. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/components/ui/panels/fence-panel.tsx | 30 ++++++++++++------- .../src/components/ui/panels/wall-panel.tsx | 30 ++++++++++++------- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/packages/editor/src/components/ui/panels/fence-panel.tsx b/packages/editor/src/components/ui/panels/fence-panel.tsx index f269cba5..8d8d3531 100644 --- a/packages/editor/src/components/ui/panels/fence-panel.tsx +++ b/packages/editor/src/components/ui/panels/fence-panel.tsx @@ -14,7 +14,7 @@ import { import { useViewer } from '@pascal-app/viewer' import { Move, Spline } from 'lucide-react' -import { useCallback } from 'react' +import { useCallback, useRef } from 'react' import { sfxEmitter } from '../../../lib/sfx-bus' import useEditor from '../../../store/use-editor' @@ -44,7 +44,6 @@ export function FencePanel() { const selectedId = useViewer((s) => s.selection.selectedIds[0]) const selectedCount = useViewer((s) => s.selection.selectedIds.length) const setSelection = useViewer((s) => s.setSelection) - const updateNode = useScene((s) => s.updateNode) const setMovingNode = useEditor((s) => s.setMovingNode) const setCurvingFence = useEditor((s) => s.setCurvingFence) @@ -52,34 +51,43 @@ export function FencePanel() { selectedId ? (s.nodes[selectedId as AnyNode['id']] as FenceNode | undefined) : undefined, ) + // Mirror the latest node into a ref so the slider handlers below have + // stable identities across re-renders. Without this, every store tick + // (one per pointermove during a slider drag) rebuilt the handler + // refs, which destabilised SliderControl's pointer-capture listeners + // and combined with float drift in `getWallCurveLength` produced a + // "Maximum update depth exceeded" cascade. + const nodeRef = useRef(node) + nodeRef.current = node + const handleUpdate = useCallback( (updates: Partial) => { if (!selectedId) return - updateNode(selectedId as AnyNode['id'], updates) - useScene.getState().dirtyNodes.add(selectedId as AnyNodeId) + useScene.getState().updateNode(selectedId as AnyNode['id'], updates) }, - [selectedId, updateNode], + [selectedId], ) const handleUpdateLength = useCallback( (newLength: number) => { - if (!node || newLength <= 0) return + const n = nodeRef.current + if (!n || newLength <= 0) return - const dx = node.end[0] - node.start[0] - const dz = node.end[1] - node.start[1] + const dx = n.end[0] - n.start[0] + const dz = n.end[1] - n.start[1] const currentLength = Math.sqrt(dx * dx + dz * dz) if (currentLength === 0) return const dirX = dx / currentLength const dirZ = dz / currentLength const newEnd: [number, number] = [ - node.start[0] + dirX * newLength, - node.start[1] + dirZ * newLength, + n.start[0] + dirX * newLength, + n.start[1] + dirZ * newLength, ] handleUpdate({ end: newEnd }) }, - [node, handleUpdate], + [handleUpdate], ) const handleClose = useCallback(() => { diff --git a/packages/editor/src/components/ui/panels/wall-panel.tsx b/packages/editor/src/components/ui/panels/wall-panel.tsx index c1fae2e7..c1f803ff 100644 --- a/packages/editor/src/components/ui/panels/wall-panel.tsx +++ b/packages/editor/src/components/ui/panels/wall-panel.tsx @@ -12,7 +12,7 @@ import { } from '@pascal-app/core' import { useViewer } from '@pascal-app/viewer' import { Move, Spline } from 'lucide-react' -import { useCallback } from 'react' +import { useCallback, useRef } from 'react' import { sfxEmitter } from '../../../lib/sfx-bus' import useEditor from '../../../store/use-editor' import { ActionButton, ActionGroup } from '../controls/action-button' @@ -23,7 +23,6 @@ import { PanelWrapper } from './panel-wrapper' export function WallPanel() { const selectedId = useViewer((s) => s.selection.selectedIds[0]) const setSelection = useViewer((s) => s.setSelection) - const updateNode = useScene((s) => s.updateNode) const setMovingNode = useEditor((s) => s.setMovingNode) const setCurvingWall = useEditor((s) => s.setCurvingWall) @@ -47,21 +46,30 @@ export function WallPanel() { }) }) + // Mirror the latest node into a ref so the slider handlers below have + // stable identities across re-renders. Without this, every store tick + // (one per pointermove during a slider drag) rebuilt the handler + // refs, destabilising SliderControl's pointer-capture listeners and + // combining with float drift in `getWallCurveLength` produced a + // "Maximum update depth exceeded" cascade. Same fix in fence-panel.tsx. + const nodeRef = useRef(node) + nodeRef.current = node + const handleUpdate = useCallback( (updates: Partial) => { if (!selectedId) return - updateNode(selectedId as AnyNode['id'], updates) - useScene.getState().dirtyNodes.add(selectedId as AnyNodeId) + useScene.getState().updateNode(selectedId as AnyNode['id'], updates) }, - [selectedId, updateNode], + [selectedId], ) const handleUpdateLength = useCallback( (newLength: number) => { - if (!node || newLength <= 0) return + const n = nodeRef.current + if (!n || newLength <= 0) return - const dx = node.end[0] - node.start[0] - const dz = node.end[1] - node.start[1] + const dx = n.end[0] - n.start[0] + const dz = n.end[1] - n.start[1] const currentLength = Math.sqrt(dx * dx + dz * dz) if (currentLength === 0) return @@ -70,13 +78,13 @@ export function WallPanel() { const dirZ = dz / currentLength const newEnd: [number, number] = [ - node.start[0] + dirX * newLength, - node.start[1] + dirZ * newLength, + n.start[0] + dirX * newLength, + n.start[1] + dirZ * newLength, ] handleUpdate({ end: newEnd }) }, - [node, handleUpdate], + [handleUpdate], ) const handleClose = useCallback(() => {