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

fix: resolve stale closures in CandleChart click handler

Browse source 
Convert drawingState, selectedLineId, dragState, and annotations to
refs that stay in sync with state via useEffect. The chart click handler
(subscribeClick) captured stale closure values for these variables.
Now reads from refs (e.g. drawingStateRef.current) so the handler
always sees the latest state, and removed the stale state variables
from the useEffect dependency array.

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

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

@ -65,7 +65,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`)
- [ ] 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`)
- [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`)
## 8. Frontend — Memory Leaks & Performance

55
src/components/CandleChart.tsx
View file

@ -151,6 +151,18 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
const [selectedLineId, setSelectedLineId] = useState<number | null>(null);
const [dragState, setDragState] = useState<{ lineId: number; endpoint: 'p1' | 'p2' } | null>(null);
// Refs to avoid stale closures in chart click/mouse handlers
const drawingStateRef = useRef<DrawingState | null>(drawingState);
const selectedLineIdRef = useRef<number | null>(selectedLineId);
const dragStateRef = useRef<{ lineId: number; endpoint: 'p1' | 'p2' } | null>(dragState);
const annotationsRef = useRef<Annotation[]>(annotations);
// Keep refs in sync with state
useEffect(() => { drawingStateRef.current = drawingState; }, [drawingState]);
useEffect(() => { selectedLineIdRef.current = selectedLineId; }, [selectedLineId]);
useEffect(() => { dragStateRef.current = dragState; }, [dragState]);
useEffect(() => { annotationsRef.current = annotations; }, [annotations]);
// Track mounted state to avoid hydration mismatch
useEffect(() => {
setMounted(true);
@ -584,29 +596,29 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
if (time === null || price === null) return;
// Check if clicking on a selected line's endpoint handle to start dragging
if (selectedLineId !== null && !activeTool) {
const linePrimitive = linePrimitivesRef.current.get(selectedLineId);
if (selectedLineIdRef.current !== null && !activeTool) {
const linePrimitive = linePrimitivesRef.current.get(selectedLineIdRef.current);
if (linePrimitive) {
const p1 = linePrimitive.getP1();
const p2 = linePrimitive.getP2();
if (isNearEndpoint(timeCoordinate, priceCoordinate, p1.time, p1.price)) {
setDragState({ lineId: selectedLineId, endpoint: 'p1' });
setDragState({ lineId: selectedLineIdRef.current!, endpoint: 'p1' });
return;
}
if (isNearEndpoint(timeCoordinate, priceCoordinate, p2.time, p2.price)) {
setDragState({ lineId: selectedLineId, endpoint: 'p2' });
setDragState({ lineId: selectedLineIdRef.current!, endpoint: 'p2' });
return;
}
}
}
// If currently dragging, complete the drag operation
if (dragState) {
const linePrimitive = linePrimitivesRef.current.get(dragState.lineId);
if (dragStateRef.current) {
const linePrimitive = linePrimitivesRef.current.get(dragStateRef.current.lineId);
if (linePrimitive) {
const annotation = annotations.find(a => a.id === dragState.lineId);
const annotation = annotationsRef.current.find(a => a.id === dragStateRef.current!.lineId);
if (annotation) {
// Persist the updated geometry
const p1 = linePrimitive.getP1();
@ -638,7 +650,7 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
// Handle line and rectangle drawing
if (activeTool === 'line' || activeTool === 'rectangle') {
if (!drawingState) {
if (!drawingStateRef.current) {
// First click: record first point and show preview
const firstPoint = { time, price };
setDrawingState({ tool: activeTool, firstPoint });
@ -678,10 +690,11 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
const secondPoint = { time, price };
try {
const timestamp = typeof drawingState.firstPoint.time === 'string'
? Date.parse(drawingState.firstPoint.time) / 1000
: (drawingState.firstPoint.time as number);
const currentDrawing = drawingStateRef.current!;
const timestamp = typeof currentDrawing.firstPoint.time === 'string'
? Date.parse(currentDrawing.firstPoint.time) / 1000
: (currentDrawing.firstPoint.time as number);
const response = await fetch('/api/annotations', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
@ -691,10 +704,10 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
chart_id: activeChartId,
color: selectedColor,
geometry: {
startTime: typeof drawingState.firstPoint.time === 'string'
? Date.parse(drawingState.firstPoint.time) / 1000
: drawingState.firstPoint.time,
startPrice: drawingState.firstPoint.price,
startTime: typeof currentDrawing.firstPoint.time === 'string'
? Date.parse(currentDrawing.firstPoint.time) / 1000
: currentDrawing.firstPoint.time,
startPrice: currentDrawing.firstPoint.price,
endTime: typeof secondPoint.time === 'string'
? Date.parse(secondPoint.time) / 1000
: secondPoint.time,
@ -788,7 +801,7 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
if (response.ok) {
seriesRef.current!.detachPrimitive(deleteLineHit.primitive);
linePrimitivesRef.current.delete(deleteLineHit.id);
if (selectedLineId === deleteLineHit.id) {
if (selectedLineIdRef.current === deleteLineHit.id) {
setSelectedLineId(null);
}
await fetchAnnotations();
@ -838,7 +851,7 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
// Find annotation at this timestamp (within tolerance)
const tolerance = 60; // 60 seconds tolerance
const annotation = annotations.find(
const annotation = annotationsRef.current.find(
(a) =>
markerTypeNames.includes(a.label_type) &&
Math.abs(a.timestamp - timestamp) < tolerance
@ -876,7 +889,7 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
if (lineHit && activeTool !== 'delete') {
// Toggle selection
const newSelectedId = selectedLineId === lineHit.id ? null : lineHit.id;
const newSelectedId = selectedLineIdRef.current === lineHit.id ? null : lineHit.id;
setSelectedLineId(newSelectedId);
// Update all lines' selection state
@ -955,7 +968,7 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
// Find annotation at this timestamp (within tolerance)
const tolerance = 60; // 60 seconds tolerance
const annotation = annotations.find(
const annotation = annotationsRef.current.find(
(a) =>
markerTypeNames.includes(a.label_type) &&
Math.abs(a.timestamp - timestamp) < tolerance
@ -972,7 +985,7 @@ const CandleChart = forwardRef<CandleChartHandle, CandleChartProps>(
return () => {
chartRef.current?.unsubscribeClick(handleClick);
};
}, [activeTool, candles, annotations, annotationTypes, onAnnotationChange, predictionVisible, predictionSpans, predictionSummary, onPredictionClick, onPredictionDismiss, drawingState, selectedColor]);
}, [activeTool, candles, annotationTypes, onAnnotationChange, predictionVisible, predictionSpans, predictionSummary, onPredictionClick, onPredictionDismiss, selectedColor]);
// Handle crosshair move to update preview during drawing and dragging
useEffect(() => {