diff --git a/openspec/changes/code-review-fix/tasks.md b/openspec/changes/code-review-fix/tasks.md index cd35588..f83bfc1 100644 --- a/openspec/changes/code-review-fix/tasks.md +++ b/openspec/changes/code-review-fix/tasks.md @@ -66,7 +66,7 @@ - [x] 7.1 `[opus]` Fix stale closure in `fetchPredictions`: extract `modelInfo` into a `useRef` that stays in sync with state, use ref in `generateCacheKey` (`src/app/page.tsx:489-552`) - [x] 7.2 `[opus]` Add AbortController to `fetchPredictions` and `handleFetchBatchPredictions`: store controller in ref, abort previous on new request, discard stale responses (`src/app/page.tsx:494-663`) - [x] 7.3 `[opus]` Fix stale closure in CandleChart click handler: convert `drawingState`, `selectedLineId`, `dragState`, `annotations` to refs, update refs alongside setState, read refs in handler (`src/components/CandleChart.tsx:572-971`) -- [ ] 7.4 `[opus]` Fix stale closure in SpanAnnotationManager keyboard handler: wrap `handleDeleteSpan` in `useCallback`, use ref for `selectedSpanId` (`src/components/SpanAnnotationManager.tsx:461-535`) +- [x] 7.4 `[opus]` Fix stale closure in SpanAnnotationManager keyboard handler: wrap `handleDeleteSpan` in `useCallback`, use ref for `selectedSpanId` (`src/components/SpanAnnotationManager.tsx:461-535`) ## 8. Frontend — Memory Leaks & Performance diff --git a/src/components/SpanAnnotationManager.tsx b/src/components/SpanAnnotationManager.tsx index 79c85bb..8382da7 100644 --- a/src/components/SpanAnnotationManager.tsx +++ b/src/components/SpanAnnotationManager.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useEffect, useState, useRef } from 'react'; +import { useEffect, useState, useRef, useCallback } from 'react'; import { IChartApi, ISeriesApi, Time } from 'lightweight-charts'; import { SpanRectanglePrimitive, SpanData } from './SpanRectanglePrimitive'; import SpanPopover from './SpanPopover'; @@ -73,6 +73,12 @@ export default function SpanAnnotationManager({ const [popoverOpen, setPopoverOpen] = useState(false); const [editingSpan, setEditingSpan] = useState(null); const primitivesRef = useRef>(new Map()); + const selectedSpanIdRef = useRef(selectedSpanId); + + // Keep selectedSpanIdRef in sync with prop + useEffect(() => { + selectedSpanIdRef.current = selectedSpanId; + }, [selectedSpanId]); // Find nearest candle to a timestamp const findNearestCandle = (timestamp: number): Candle | null => { @@ -347,7 +353,7 @@ export default function SpanAnnotationManager({ }, [activeTool, interactionState, previewPrimitive, series]); // Handle span deletion - const handleDeleteSpan = async (spanId: number) => { + const handleDeleteSpan = useCallback(async (spanId: number) => { try { const response = await fetch(`/api/span-annotations/${spanId}`, { method: 'DELETE', @@ -355,7 +361,7 @@ export default function SpanAnnotationManager({ if (response.ok) { onSpanAnnotationsChange(); - if (selectedSpanId === spanId) { + if (selectedSpanIdRef.current === spanId) { onSelectedSpanChange(null); } } else { @@ -364,7 +370,7 @@ export default function SpanAnnotationManager({ } catch (error) { console.error('Error deleting span annotation:', error); } - }; + }, [onSpanAnnotationsChange, onSelectedSpanChange]); // Handle popover save const handlePopoverSave = async (data: { @@ -461,15 +467,16 @@ export default function SpanAnnotationManager({ useEffect(() => { const handleKeyDown = async (e: KeyboardEvent) => { // Delete/Backspace: delete selected span - if ((e.key === 'Delete' || e.key === 'Backspace') && selectedSpanId !== null) { + const currentSelectedSpanId = selectedSpanIdRef.current; + if ((e.key === 'Delete' || e.key === 'Backspace') && currentSelectedSpanId !== null) { e.preventDefault(); - handleDeleteSpan(selectedSpanId); + handleDeleteSpan(currentSelectedSpanId); return; } // Enter: open edit popover for selected span - if (e.key === 'Enter' && selectedSpanId !== null) { - const span = spanAnnotations.find((s) => s.id === selectedSpanId); + if (e.key === 'Enter' && currentSelectedSpanId !== null) { + const span = spanAnnotations.find((s) => s.id === currentSelectedSpanId); if (span) { setEditingSpan(span); setPopoverOpen(true); @@ -532,7 +539,7 @@ export default function SpanAnnotationManager({ window.addEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown); - }, [selectedSpanId, spanAnnotations, activeTool, interactionState, startCandle, cursorCandle, spanLabelTypes, activeChartId, previewPrimitive, series, onSpanAnnotationsChange]); + }, [handleDeleteSpan, spanAnnotations, activeTool, interactionState, startCandle, cursorCandle, spanLabelTypes, activeChartId, previewPrimitive, series, onSpanAnnotationsChange]); // Handle double-click to edit span useEffect(() => {