diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 148dc66..6c5753e 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -125,6 +125,17 @@ function App() { } }, []) + // Handle graph node click - memoized to prevent DependencyGraph re-renders + const handleGraphNodeClick = useCallback((nodeId: number) => { + const allFeatures = [ + ...(features?.pending ?? []), + ...(features?.in_progress ?? []), + ...(features?.done ?? []) + ] + const feature = allFeatures.find(f => f.id === nodeId) + if (feature) setSelectedFeature(feature) + }, [features]) + // Validate stored project exists (clear if project was deleted) useEffect(() => { if (selectedProject && projects && !projects.some(p => p.name === selectedProject)) { @@ -386,16 +397,7 @@ function App() { {graphData ? ( { - // Find the feature and open the modal - const allFeatures = [ - ...(features?.pending ?? []), - ...(features?.in_progress ?? []), - ...(features?.done ?? []) - ] - const feature = allFeatures.find(f => f.id === nodeId) - if (feature) setSelectedFeature(feature) - }} + onNodeClick={handleGraphNodeClick} /> ) : (
diff --git a/ui/src/components/DependencyGraph.tsx b/ui/src/components/DependencyGraph.tsx index de3931e..0649147 100644 --- a/ui/src/components/DependencyGraph.tsx +++ b/ui/src/components/DependencyGraph.tsx @@ -1,4 +1,5 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' +import { Component, useCallback, useEffect, useMemo, useRef, useState } from 'react' +import type { ErrorInfo, ReactNode } from 'react' import { ReactFlow, Background, @@ -14,7 +15,7 @@ import { Handle, } from '@xyflow/react' import dagre from 'dagre' -import { CheckCircle2, Circle, Loader2, AlertTriangle } from 'lucide-react' +import { CheckCircle2, Circle, Loader2, AlertTriangle, RefreshCw } from 'lucide-react' import type { DependencyGraph as DependencyGraphData, GraphNode } from '../lib/types' import '@xyflow/react/dist/style.css' @@ -27,6 +28,62 @@ interface DependencyGraphProps { onNodeClick?: (nodeId: number) => void } +// Error boundary to catch and recover from ReactFlow rendering errors +interface ErrorBoundaryProps { + children: ReactNode + onReset?: () => void +} + +interface ErrorBoundaryState { + hasError: boolean + error: Error | null +} + +class GraphErrorBoundary extends Component { + constructor(props: ErrorBoundaryProps) { + super(props) + this.state = { hasError: false, error: null } + } + + static getDerivedStateFromError(error: Error): ErrorBoundaryState { + return { hasError: true, error } + } + + componentDidCatch(error: Error, errorInfo: ErrorInfo) { + console.error('DependencyGraph error:', error, errorInfo) + } + + handleReset = () => { + this.setState({ hasError: false, error: null }) + this.props.onReset?.() + } + + render() { + if (this.state.hasError) { + return ( +
+
+ +
Graph rendering error
+
+ The dependency graph encountered an issue. +
+ +
+
+ ) + } + + return this.props.children + } +} + // Custom node component function FeatureNode({ data }: { data: GraphNode & { onClick?: () => void } }) { const statusColors = { @@ -127,10 +184,22 @@ function getLayoutedElements( return { nodes: layoutedNodes, edges } } -export function DependencyGraph({ graphData, onNodeClick }: DependencyGraphProps) { +function DependencyGraphInner({ graphData, onNodeClick }: DependencyGraphProps) { const [direction, setDirection] = useState<'TB' | 'LR'>('LR') + // Use ref for callback to avoid triggering re-renders when callback identity changes + const onNodeClickRef = useRef(onNodeClick) + useEffect(() => { + onNodeClickRef.current = onNodeClick + }, [onNodeClick]) + + // Create a stable click handler that uses the ref + const handleNodeClick = useCallback((nodeId: number) => { + onNodeClickRef.current?.(nodeId) + }, []) + // Convert graph data to React Flow format + // Only recalculate when graphData or direction changes (not when onNodeClick changes) const initialElements = useMemo(() => { const nodes: Node[] = graphData.nodes.map((node) => ({ id: String(node.id), @@ -138,7 +207,7 @@ export function DependencyGraph({ graphData, onNodeClick }: DependencyGraphProps position: { x: 0, y: 0 }, data: { ...node, - onClick: () => onNodeClick?.(node.id), + onClick: () => handleNodeClick(node.id), }, })) @@ -156,20 +225,36 @@ export function DependencyGraph({ graphData, onNodeClick }: DependencyGraphProps })) return getLayoutedElements(nodes, edges, direction) - }, [graphData, direction, onNodeClick]) + }, [graphData, direction, handleNodeClick]) const [nodes, setNodes, onNodesChange] = useNodesState(initialElements.nodes) const [edges, setEdges, onEdgesChange] = useEdgesState(initialElements.edges) - // Update layout when data or direction changes + // Update layout when initialElements changes + // Using a ref to track previous graph data to avoid unnecessary updates + const prevGraphDataRef = useRef('') + const prevDirectionRef = useRef<'TB' | 'LR'>(direction) + useEffect(() => { - const { nodes: layoutedNodes, edges: layoutedEdges } = getLayoutedElements( - initialElements.nodes, - initialElements.edges, - direction - ) - setNodes(layoutedNodes) - setEdges(layoutedEdges) + // Create a simple hash of the graph data to detect actual changes + const graphHash = JSON.stringify({ + nodes: graphData.nodes.map(n => ({ id: n.id, status: n.status })), + edges: graphData.edges, + }) + + // Only update if graph data or direction actually changed + if (graphHash !== prevGraphDataRef.current || direction !== prevDirectionRef.current) { + prevGraphDataRef.current = graphHash + prevDirectionRef.current = direction + + const { nodes: layoutedNodes, edges: layoutedEdges } = getLayoutedElements( + initialElements.nodes, + initialElements.edges, + direction + ) + setNodes(layoutedNodes) + setEdges(layoutedEdges) + } }, [graphData, direction, setNodes, setEdges, initialElements]) const onLayout = useCallback( @@ -287,3 +372,20 @@ export function DependencyGraph({ graphData, onNodeClick }: DependencyGraphProps
) } + +// Wrapper component with error boundary for stability +export function DependencyGraph({ graphData, onNodeClick }: DependencyGraphProps) { + // Use a key based on graph data length to force remount on structural changes + // This helps recover from corrupted ReactFlow state + const [resetKey, setResetKey] = useState(0) + + const handleReset = useCallback(() => { + setResetKey(k => k + 1) + }, []) + + return ( + + + + ) +}