fix: undo/redo history index divergence when exceeding 50 entries
Fixes two bugs identified by Devin Review on PR #1: 1. History index goes out of bounds (50) when entries exceed the 50-entry cap, causing the first undo to be a silent no-op. The shift() removed an entry but the index still incremented past the array bounds. 2. pushHistory uses stale historyIndex closure value inside setHistory's functional updater, causing entries to be silently dropped when multiple pushHistory calls are batched by React. Fix: Combine history entries and index into a single useReducer state so both are always updated atomically. Add 'reset' action for new transaction tab creation. Co-Authored-By: Nakamoto, S <defi@defi-oracle.io>
This commit is contained in:
77
src/App.tsx
77
src/App.tsx
@@ -1,4 +1,4 @@
|
||||
import { useState, useEffect, useCallback, useRef } from 'react';
|
||||
import { useState, useEffect, useCallback, useRef, useReducer } from 'react';
|
||||
import { addEdge, applyNodeChanges, applyEdgeChanges, type Node, type Edge, type Connection, type NodeChange, type EdgeChange } from '@xyflow/react';
|
||||
import TitleBar from './components/TitleBar';
|
||||
import ActivityBar from './components/ActivityBar';
|
||||
@@ -66,44 +66,68 @@ export default function App() {
|
||||
}));
|
||||
}, [activeTransactionId]);
|
||||
|
||||
// Undo/redo
|
||||
const [history, setHistory] = useState<HistoryEntry[]>([{ nodes: [], edges: [] }]);
|
||||
const [historyIndex, setHistoryIndex] = useState(0);
|
||||
// Undo/redo — combined into single state to prevent index/entries divergence
|
||||
type HistoryState = { entries: HistoryEntry[]; index: number };
|
||||
type HistoryAction =
|
||||
| { type: 'push'; nodes: Node[]; edges: Edge[] }
|
||||
| { type: 'undo' }
|
||||
| { type: 'redo' }
|
||||
| { type: 'reset' };
|
||||
|
||||
const historyReducer = useCallback((state: HistoryState, action: HistoryAction): HistoryState => {
|
||||
switch (action.type) {
|
||||
case 'push': {
|
||||
const trimmed = state.entries.slice(0, state.index + 1);
|
||||
const entry = { nodes: JSON.parse(JSON.stringify(action.nodes)), edges: JSON.parse(JSON.stringify(action.edges)) };
|
||||
const next = [...trimmed, entry];
|
||||
let newIndex = trimmed.length;
|
||||
if (next.length > 50) {
|
||||
next.shift();
|
||||
newIndex = next.length - 1;
|
||||
}
|
||||
return { entries: next, index: newIndex };
|
||||
}
|
||||
case 'undo': {
|
||||
if (state.index <= 0) return state;
|
||||
return { ...state, index: state.index - 1 };
|
||||
}
|
||||
case 'redo': {
|
||||
if (state.index >= state.entries.length - 1) return state;
|
||||
return { ...state, index: state.index + 1 };
|
||||
}
|
||||
case 'reset': {
|
||||
return { entries: [{ nodes: [], edges: [] }], index: 0 };
|
||||
}
|
||||
}
|
||||
}, []);
|
||||
|
||||
const [historyState, dispatchHistory] = useReducer(historyReducer, { entries: [{ nodes: [], edges: [] }], index: 0 });
|
||||
const history = historyState.entries;
|
||||
const historyIndex = historyState.index;
|
||||
const skipHistoryRef = useRef(false);
|
||||
|
||||
const pushHistory = useCallback((n: Node[], e: Edge[]) => {
|
||||
if (skipHistoryRef.current) { skipHistoryRef.current = false; return; }
|
||||
setHistory(prev => {
|
||||
const trimmed = prev.slice(0, historyIndex + 1);
|
||||
const entry = { nodes: JSON.parse(JSON.stringify(n)), edges: JSON.parse(JSON.stringify(e)) };
|
||||
const next = [...trimmed, entry];
|
||||
if (next.length > 50) next.shift();
|
||||
return next;
|
||||
});
|
||||
setHistoryIndex(prev => Math.min(prev + 1, 50));
|
||||
}, [historyIndex]);
|
||||
dispatchHistory({ type: 'push', nodes: n, edges: e });
|
||||
}, []);
|
||||
|
||||
const undo = useCallback(() => {
|
||||
if (historyIndex <= 0) return;
|
||||
const newIndex = historyIndex - 1;
|
||||
const entry = history[newIndex];
|
||||
if (!entry) return;
|
||||
const entry = historyState.entries[historyState.index - 1];
|
||||
if (!entry || historyState.index <= 0) return;
|
||||
skipHistoryRef.current = true;
|
||||
setHistoryIndex(newIndex);
|
||||
dispatchHistory({ type: 'undo' });
|
||||
setNodes(JSON.parse(JSON.stringify(entry.nodes)));
|
||||
setEdges(JSON.parse(JSON.stringify(entry.edges)));
|
||||
}, [historyIndex, history, setNodes, setEdges]);
|
||||
}, [historyState, setNodes, setEdges]);
|
||||
|
||||
const redo = useCallback(() => {
|
||||
if (historyIndex >= history.length - 1) return;
|
||||
const newIndex = historyIndex + 1;
|
||||
const entry = history[newIndex];
|
||||
if (!entry) return;
|
||||
const entry = historyState.entries[historyState.index + 1];
|
||||
if (!entry || historyState.index >= historyState.entries.length - 1) return;
|
||||
skipHistoryRef.current = true;
|
||||
setHistoryIndex(newIndex);
|
||||
dispatchHistory({ type: 'redo' });
|
||||
setNodes(JSON.parse(JSON.stringify(entry.nodes)));
|
||||
setEdges(JSON.parse(JSON.stringify(entry.edges)));
|
||||
}, [historyIndex, history, setNodes, setEdges]);
|
||||
}, [historyState, setNodes, setEdges]);
|
||||
|
||||
// Selected nodes
|
||||
const [selectedNodeIds, setSelectedNodeIds] = useState<Set<string>>(new Set());
|
||||
@@ -359,8 +383,7 @@ export default function App() {
|
||||
const id = `tx-${Date.now()}`;
|
||||
setTransactionTabs(prev => [...prev, { id, name: `Transaction ${prev.length + 1}`, nodes: [], edges: [] }]);
|
||||
setActiveTransactionId(id);
|
||||
setHistory([{ nodes: [], edges: [] }]);
|
||||
setHistoryIndex(0);
|
||||
dispatchHistory({ type: 'reset' });
|
||||
addTerminalEntry('info', 'system', 'New transaction tab created');
|
||||
}, [addTerminalEntry]);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user