batch3 T6 review fixes: remove rollback closure, flush-error resync
- remove: capture snapshot inside setPanes functional updater to avoid stale-closure rollback under concurrent renders - flushPendingState: call refresh() on PATCH failure so server truth and optimistic local state can't silently diverge - Drop body.state! non-null assertion via narrowed local Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -19,21 +19,6 @@ export function usePanes(sessionId: string | undefined): {
|
||||
const pendingState = useRef<Map<string, PaneState>>(new Map());
|
||||
const debounceTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
|
||||
const flushPendingState = useCallback(() => {
|
||||
if (debounceTimer.current !== null) {
|
||||
clearTimeout(debounceTimer.current);
|
||||
debounceTimer.current = null;
|
||||
}
|
||||
const snapshot = new Map(pendingState.current);
|
||||
pendingState.current.clear();
|
||||
for (const [id, state] of snapshot) {
|
||||
// fire-and-forget; caller surface handles errors via hook error state
|
||||
void api.panes.update(id, { state }).catch((err) => {
|
||||
setError(err instanceof Error ? err.message : 'pane operation failed');
|
||||
});
|
||||
}
|
||||
}, []);
|
||||
|
||||
const refresh = useCallback(async () => {
|
||||
if (!sessionId) {
|
||||
setPanes(null);
|
||||
@@ -51,6 +36,23 @@ export function usePanes(sessionId: string | undefined): {
|
||||
}
|
||||
}, [sessionId]);
|
||||
|
||||
const flushPendingState = useCallback(async () => {
|
||||
if (debounceTimer.current !== null) {
|
||||
clearTimeout(debounceTimer.current);
|
||||
debounceTimer.current = null;
|
||||
}
|
||||
const updates = Array.from(pendingState.current.entries());
|
||||
pendingState.current.clear();
|
||||
if (updates.length === 0) return;
|
||||
try {
|
||||
await Promise.all(updates.map(([id, state]) => api.panes.update(id, { state })));
|
||||
} catch (err) {
|
||||
setError(err instanceof Error ? err.message : 'pane state PATCH failed');
|
||||
// server truth may diverge from optimistic local state; resync
|
||||
void refresh();
|
||||
}
|
||||
}, [refresh]);
|
||||
|
||||
// Fetch on mount / sessionId change; preserve previous list while reloading
|
||||
// (loading=true but panes stays non-null after first fetch to avoid flash)
|
||||
useEffect(() => {
|
||||
@@ -99,7 +101,8 @@ export function usePanes(sessionId: string | undefined): {
|
||||
});
|
||||
|
||||
// Coalesce: last state wins within debounce window
|
||||
pendingState.current.set(id, body.state!);
|
||||
const nextState = body.state;
|
||||
pendingState.current.set(id, nextState);
|
||||
|
||||
if (debounceTimer.current !== null) {
|
||||
clearTimeout(debounceTimer.current);
|
||||
@@ -124,21 +127,24 @@ export function usePanes(sessionId: string | undefined): {
|
||||
|
||||
const remove = useCallback(
|
||||
async (id: string): Promise<void> => {
|
||||
// Optimistic remove
|
||||
const previous = panes;
|
||||
setPanes((prev) => (prev ? prev.filter((p) => p.id !== id) : prev));
|
||||
// Optimistic remove — capture snapshot inside functional updater to avoid stale closure
|
||||
let snapshot: Pane[] | null = null;
|
||||
setPanes((prev) => {
|
||||
snapshot = prev;
|
||||
return prev ? prev.filter((p) => p.id !== id) : prev;
|
||||
});
|
||||
|
||||
try {
|
||||
await api.panes.remove(id);
|
||||
await refresh();
|
||||
} catch (err) {
|
||||
// Rollback
|
||||
setPanes(previous);
|
||||
// Rollback to the truly-most-recent value captured above
|
||||
setPanes(snapshot);
|
||||
setError(err instanceof Error ? err.message : 'pane operation failed');
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
[panes, refresh]
|
||||
[refresh]
|
||||
);
|
||||
|
||||
return { panes, loading, error, refresh, create, update, remove };
|
||||
|
||||
Reference in New Issue
Block a user