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

fix: resolve stale closure in SpanAnnotationManager keyboard handler

Browse source 
Use selectedSpanIdRef to avoid capturing stale selectedSpanId in the
keyboard event handler closure. Wrap handleDeleteSpan in useCallback
with stable dependencies (reads selectedSpanIdRef.current instead of
the prop directly). Remove selectedSpanId from the useEffect dependency
array since the ref is used instead.

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

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

@ -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

25
src/components/SpanAnnotationManager.tsx
View file

@ -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<SpanAnnotation | null>(null);
const primitivesRef = useRef<Map<number, SpanRectanglePrimitive>>(new Map());
const selectedSpanIdRef = useRef<number | null>(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(() => {