trentuna/candle-annotator
8
0
Fork 0
Code Issues Pull requests Releases Packages Activity Actions

fix: replace useState with useRef for preview primitive to prevent memory leak

Browse source 
The preview primitive in SpanAnnotationManager was stored in useState,
causing unnecessary re-renders and potential memory leaks since the
primitive was not cleaned up on unmount. Changed to useRef, updated all
read/write sites, removed from dependency arrays, and added unmount
cleanup effect that detaches the primitive from the series.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Marko Djordjevic 2026-02-18 15:18:48 +01:00
parent e5b0cc2540
commit 0cd7a34c99
2 changed files with 35 additions and 21 deletions
Download patch file Download diff file Expand all files Collapse all files

2
openspec/changes/code-review-fix/tasks.md
View file

@ -70,7 +70,7 @@
## 8. Frontend — Memory Leaks & Performance ## 8. Frontend — Memory Leaks & Performance
- [ ] 8.1 `[opus]` Fix SpanAnnotationManager preview primitive memory leak: replace `useState` with `useRef` for preview primitive, add cleanup on unmount (`src/components/SpanAnnotationManager.tsx:265-324`) - [x] 8.1 `[opus]` Fix SpanAnnotationManager preview primitive memory leak: replace `useState` with `useRef` for preview primitive, add cleanup on unmount (`src/components/SpanAnnotationManager.tsx:265-324`)
- [ ] 8.2 `[sonnet]` Use `chart.applyOptions()` for theme changes instead of re-creating chart (`src/components/CandleChart.tsx:333`) - [ ] 8.2 `[sonnet]` Use `chart.applyOptions()` for theme changes instead of re-creating chart (`src/components/CandleChart.tsx:333`)
- [ ] 8.3 `[sonnet]` Remove `fitContent()` from reconciliation effect, only call on initial load (`src/components/SpanAnnotationManager.tsx:160`) - [ ] 8.3 `[sonnet]` Remove `fitContent()` from reconciliation effect, only call on initial load (`src/components/SpanAnnotationManager.tsx:160`)
- [ ] 8.4 `[opus]` Implement incremental primitive updates in SpanAnnotationManager: update only selection state on selection change, full reconciliation only on annotation list changes (`src/components/SpanAnnotationManager.tsx:104-161`) - [ ] 8.4 `[opus]` Implement incremental primitive updates in SpanAnnotationManager: update only selection state on selection change, full reconciliation only on annotation list changes (`src/components/SpanAnnotationManager.tsx:104-161`)

54
src/components/SpanAnnotationManager.tsx
View file

@ -69,7 +69,7 @@ export default function SpanAnnotationManager({
const [startCandle, setStartCandle] = useState<Candle | null>(null); const [startCandle, setStartCandle] = useState<Candle | null>(null);
const [endCandle, setEndCandle] = useState<Candle | null>(null); const [endCandle, setEndCandle] = useState<Candle | null>(null);
const [cursorCandle, setCursorCandle] = useState<Candle | null>(null); const [cursorCandle, setCursorCandle] = useState<Candle | null>(null);
const [previewPrimitive, setPreviewPrimitive] = useState<SpanRectanglePrimitive | null>(null); const previewPrimitiveRef = useRef<SpanRectanglePrimitive | null>(null);
const [popoverOpen, setPopoverOpen] = useState(false); const [popoverOpen, setPopoverOpen] = useState(false);
const [editingSpan, setEditingSpan] = useState<SpanAnnotation | null>(null); const [editingSpan, setEditingSpan] = useState<SpanAnnotation | null>(null);
const primitivesRef = useRef<Map<number, SpanRectanglePrimitive>>(new Map()); const primitivesRef = useRef<Map<number, SpanRectanglePrimitive>>(new Map());
@ -80,6 +80,20 @@ export default function SpanAnnotationManager({
selectedSpanIdRef.current = selectedSpanId; selectedSpanIdRef.current = selectedSpanId;
}, [selectedSpanId]); }, [selectedSpanId]);
// Cleanup preview primitive on unmount
useEffect(() => {
return () => {
if (previewPrimitiveRef.current && series) {
try {
series.detachPrimitive(previewPrimitiveRef.current);
} catch {
// series may already be disposed
}
previewPrimitiveRef.current = null;
}
};
}, [series]);
// Find nearest candle to a timestamp // Find nearest candle to a timestamp
const findNearestCandle = (timestamp: number): Candle | null => { const findNearestCandle = (timestamp: number): Candle | null => {
if (candles.length === 0) return null; if (candles.length === 0) return null;
@ -171,9 +185,9 @@ export default function SpanAnnotationManager({
if (!chart || !series) return; if (!chart || !series) return;
// Clean up preview if tool changes away from span // Clean up preview if tool changes away from span
if (activeTool !== 'span' && previewPrimitive) { if (activeTool !== 'span' && previewPrimitiveRef.current) {
series.detachPrimitive(previewPrimitive); series.detachPrimitive(previewPrimitiveRef.current);
setPreviewPrimitive(null); previewPrimitiveRef.current = null;
setInteractionState('idle'); setInteractionState('idle');
setStartCandle(null); setStartCandle(null);
setEndCandle(null); setEndCandle(null);
@ -222,9 +236,9 @@ export default function SpanAnnotationManager({
setPopoverOpen(true); setPopoverOpen(true);
// Clean up preview primitive // Clean up preview primitive
if (previewPrimitive) { if (previewPrimitiveRef.current) {
series.detachPrimitive(previewPrimitive); series.detachPrimitive(previewPrimitiveRef.current);
setPreviewPrimitive(null); previewPrimitiveRef.current = null;
} }
} }
} else if (activeTool === 'delete') { } else if (activeTool === 'delete') {
@ -266,7 +280,7 @@ export default function SpanAnnotationManager({
return () => { return () => {
chart.unsubscribeClick(handleClick); chart.unsubscribeClick(handleClick);
}; };
}, [chart, series, activeTool, interactionState, candles, previewPrimitive, selectedSpanId, onSelectedSpanChange]); }, [chart, series, activeTool, interactionState, candles, selectedSpanId, onSelectedSpanChange]);
// Handle mouse move for preview // Handle mouse move for preview
useEffect(() => { useEffect(() => {
@ -308,8 +322,8 @@ export default function SpanAnnotationManager({
}; };
// Remove old preview primitive // Remove old preview primitive
if (previewPrimitive) { if (previewPrimitiveRef.current) {
series.detachPrimitive(previewPrimitive); series.detachPrimitive(previewPrimitiveRef.current);
} }
// Create new preview primitive // Create new preview primitive
@ -319,7 +333,7 @@ export default function SpanAnnotationManager({
}); });
series.attachPrimitive(newPreview); series.attachPrimitive(newPreview);
setPreviewPrimitive(newPreview); previewPrimitiveRef.current = newPreview;
}; };
chart.subscribeCrosshairMove(handleMouseMove); chart.subscribeCrosshairMove(handleMouseMove);
@ -327,7 +341,7 @@ export default function SpanAnnotationManager({
return () => { return () => {
chart.unsubscribeCrosshairMove(handleMouseMove); chart.unsubscribeCrosshairMove(handleMouseMove);
}; };
}, [chart, series, activeTool, interactionState, startCandle, candles, previewPrimitive]); }, [chart, series, activeTool, interactionState, startCandle, candles]);
// Handle Escape key to cancel span selection // Handle Escape key to cancel span selection
useEffect(() => { useEffect(() => {
@ -340,9 +354,9 @@ export default function SpanAnnotationManager({
setEndCandle(null); setEndCandle(null);
// Clean up preview // Clean up preview
if (previewPrimitive && series) { if (previewPrimitiveRef.current && series) {
series.detachPrimitive(previewPrimitive); series.detachPrimitive(previewPrimitiveRef.current);
setPreviewPrimitive(null); previewPrimitiveRef.current = null;
} }
} }
} }
@ -350,7 +364,7 @@ export default function SpanAnnotationManager({
window.addEventListener('keydown', handleKeyDown); window.addEventListener('keydown', handleKeyDown);
return () => window.removeEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown);
}, [activeTool, interactionState, previewPrimitive, series]); }, [activeTool, interactionState, series]);
// Handle span deletion // Handle span deletion
const handleDeleteSpan = useCallback(async (spanId: number) => { const handleDeleteSpan = useCallback(async (spanId: number) => {
@ -522,9 +536,9 @@ export default function SpanAnnotationManager({
onSpanAnnotationsChange(); onSpanAnnotationsChange();
// Clean up preview and reset state // Clean up preview and reset state
if (previewPrimitive && series) { if (previewPrimitiveRef.current && series) {
series.detachPrimitive(previewPrimitive); series.detachPrimitive(previewPrimitiveRef.current);
setPreviewPrimitive(null); previewPrimitiveRef.current = null;
} }
setInteractionState('idle'); setInteractionState('idle');
setStartCandle(null); setStartCandle(null);
@ -539,7 +553,7 @@ export default function SpanAnnotationManager({
window.addEventListener('keydown', handleKeyDown); window.addEventListener('keydown', handleKeyDown);
return () => window.removeEventListener('keydown', handleKeyDown); return () => window.removeEventListener('keydown', handleKeyDown);
}, [handleDeleteSpan, spanAnnotations, activeTool, interactionState, startCandle, cursorCandle, spanLabelTypes, activeChartId, previewPrimitive, series, onSpanAnnotationsChange]); }, [handleDeleteSpan, spanAnnotations, activeTool, interactionState, startCandle, cursorCandle, spanLabelTypes, activeChartId, series, onSpanAnnotationsChange]);
// Handle double-click to edit span // Handle double-click to edit span
useEffect(() => { useEffect(() => {