Fence + wall panels: stable handler refs via nodeRef to fix slider-drag loop
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
6a4de8cff5
commit
71b211de97
@@ -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<FenceNode>) => {
|
||||
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(() => {
|
||||
|
||||
@@ -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<WallNode>) => {
|
||||
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(() => {
|
||||
|
||||
Reference in New Issue
Block a user