compose layout writes against current state, not a captured tree
Every mutation in WorkspaceView computed its new tree from the `layout` its callback closed over, and two of the paths are not immediate: the resize debounce fires 500 ms after the drag began, and a window resize fires onLayout on every group at once. So the later write was computed from a tree that predated the earlier one and silently undid it — remove a panel just after dragging a splitter and it came back. Worse now that panel identity lives in the layout: the resurrected tree carries an older `config`, so a panel that was just given an agent's name reverts to anonymous and the agent stops being addressable through it. All eight now pass an updater to setValue, which composes against the current cache. The debounce timer also had no cleanup at all, so it outlived the component. It now flushes on unmount rather than dropping — with an updater the early write is correct, and dropping would lose a splitter drag made just before navigating away, which the no-cleanup version did at least persist. Neither file is prettier-clean at HEAD, so neither was formatted; the new code is written to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,4 @@
|
|||||||
import { useCallback, useRef } from 'react';
|
import { useCallback, useEffect, useRef } from 'react';
|
||||||
import { ResizablePanel, ResizablePanelGroup, ResizableHandle } from '@/components/ui/resizable';
|
import { ResizablePanel, ResizablePanelGroup, ResizableHandle } from '@/components/ui/resizable';
|
||||||
import type { LayoutNode, AppRegistryMap, PanelComponents } from './types';
|
import type { LayoutNode, AppRegistryMap, PanelComponents } from './types';
|
||||||
import { countPanels } from './layout-utils';
|
import { countPanels } from './layout-utils';
|
||||||
@@ -105,9 +105,26 @@ const LayoutNodeRenderer = ({
|
|||||||
onRemove,
|
onRemove,
|
||||||
onResized,
|
onResized,
|
||||||
}: LayoutNodeRendererProps) => {
|
}: LayoutNodeRendererProps) => {
|
||||||
const debounceRef = useRef<ReturnType<typeof setTimeout>>(null);
|
const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||||
|
const pendingRef = useRef<(() => void) | null>(null);
|
||||||
const mountedRef = useRef(false);
|
const mountedRef = useRef(false);
|
||||||
|
|
||||||
|
// The timer had no cleanup at all, so it outlived the component and fired against whatever it had
|
||||||
|
// captured. Flush rather than drop: `onResized` composes against current state now (WorkspaceView passes
|
||||||
|
// an updater), so running it early is correct — and dropping it would lose a splitter drag made just
|
||||||
|
// before navigating away, which the old no-cleanup version did at least persist.
|
||||||
|
useEffect(
|
||||||
|
() => () => {
|
||||||
|
if (!debounceRef.current) return;
|
||||||
|
clearTimeout(debounceRef.current);
|
||||||
|
debounceRef.current = null;
|
||||||
|
const pending = pendingRef.current;
|
||||||
|
pendingRef.current = null;
|
||||||
|
pending?.();
|
||||||
|
},
|
||||||
|
[],
|
||||||
|
);
|
||||||
|
|
||||||
const handleLayout = useCallback(
|
const handleLayout = useCallback(
|
||||||
(sizes: number[]) => {
|
(sizes: number[]) => {
|
||||||
if (node.type !== 'group') return;
|
if (node.type !== 'group') return;
|
||||||
@@ -116,8 +133,13 @@ const LayoutNodeRenderer = ({
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (debounceRef.current) clearTimeout(debounceRef.current);
|
if (debounceRef.current) clearTimeout(debounceRef.current);
|
||||||
|
const groupId = node.id;
|
||||||
|
pendingRef.current = () => onResized(groupId, sizes);
|
||||||
debounceRef.current = setTimeout(() => {
|
debounceRef.current = setTimeout(() => {
|
||||||
onResized(node.id, sizes);
|
debounceRef.current = null;
|
||||||
|
const pending = pendingRef.current;
|
||||||
|
pendingRef.current = null;
|
||||||
|
pending?.();
|
||||||
}, 500);
|
}, 500);
|
||||||
},
|
},
|
||||||
[node, onResized],
|
[node, onResized],
|
||||||
|
|||||||
@@ -57,39 +57,51 @@ export const WorkspaceView = ({ workspace, locked, cwd = '~', root, initialFileP
|
|||||||
}
|
}
|
||||||
}, [maximizedPanelId]);
|
}, [maximizedPanelId]);
|
||||||
|
|
||||||
|
// Every mutation goes through `onLayoutChange` as an UPDATER, never as a computed tree.
|
||||||
|
//
|
||||||
|
// Each of these used to close over `layout` as it was when the callback was made, and two of the paths
|
||||||
|
// are not immediate: the resize debounce fires 500 ms after the drag began, and a window resize fires
|
||||||
|
// `onLayout` on every group at once. So the second write was computed from a tree that predated the
|
||||||
|
// first and silently undid it — remove a panel just after dragging a splitter and it came back. That is
|
||||||
|
// worse than it sounds now that panel identity lives in the layout: the resurrected tree carries an
|
||||||
|
// older `config`, so a panel that was just given an agent's name reverts to anonymous, and the agent it
|
||||||
|
// addresses is no longer reachable through it.
|
||||||
|
//
|
||||||
|
// The guards below still test the rendered `layout` — they only decide whether a write is worth making,
|
||||||
|
// and being one render stale there costs a redundant no-op write at worst.
|
||||||
const handleSetApp = useCallback(
|
const handleSetApp = useCallback(
|
||||||
(panelId: string, appType: string | null) => {
|
(panelId: string, appType: string | null) => {
|
||||||
onLayoutChange(setApp(layout, panelId, appType));
|
onLayoutChange((prev) => setApp(prev, panelId, appType));
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[onLayoutChange],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleSplit = useCallback(
|
const handleSplit = useCallback(
|
||||||
(panelId: string, direction: 'horizontal' | 'vertical') => {
|
(panelId: string, direction: 'horizontal' | 'vertical') => {
|
||||||
onLayoutChange(splitPanel(layout, panelId, direction));
|
onLayoutChange((prev) => splitPanel(prev, panelId, direction));
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[onLayoutChange],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleRemove = useCallback(
|
const handleRemove = useCallback(
|
||||||
(panelId: string) => {
|
(panelId: string) => {
|
||||||
if (countPanels(layout) <= 1) return;
|
if (countPanels(layout) <= 1) return;
|
||||||
onLayoutChange(removePanel(layout, panelId));
|
onLayoutChange((prev) => (countPanels(prev) <= 1 ? prev : removePanel(prev, panelId)));
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[layout, onLayoutChange],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleResized = useCallback(
|
const handleResized = useCallback(
|
||||||
(groupId: string, sizes: number[]) => {
|
(groupId: string, sizes: number[]) => {
|
||||||
onLayoutChange(updateSizes(layout, groupId, sizes));
|
onLayoutChange((prev) => updateSizes(prev, groupId, sizes));
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[onLayoutChange],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleSetZoom = useCallback(
|
const handleSetZoom = useCallback(
|
||||||
(panelId: string, zoom: number) => {
|
(panelId: string, zoom: number) => {
|
||||||
const next = setZoom(layout, panelId, zoom);
|
if (setZoom(layout, panelId, zoom) === layout) return;
|
||||||
if (next !== layout) onLayoutChange(next);
|
onLayoutChange((prev) => setZoom(prev, panelId, zoom));
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[layout, onLayoutChange],
|
||||||
);
|
);
|
||||||
@@ -99,26 +111,26 @@ export const WorkspaceView = ({ workspace, locked, cwd = '~', root, initialFileP
|
|||||||
|
|
||||||
const handleSetPanelConfig = useCallback(
|
const handleSetPanelConfig = useCallback(
|
||||||
(panelId: string, config: PanelConfig | undefined) => {
|
(panelId: string, config: PanelConfig | undefined) => {
|
||||||
const next = setPanelConfig(layout, panelId, config);
|
if (setPanelConfig(layout, panelId, config) === layout) return;
|
||||||
if (next !== layout) onLayoutChange(next);
|
onLayoutChange((prev) => setPanelConfig(prev, panelId, config));
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[layout, onLayoutChange],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleSwap = useCallback(
|
const handleSwap = useCallback(
|
||||||
(sourceId: string, targetId: string) => {
|
(sourceId: string, targetId: string) => {
|
||||||
onLayoutChange(swapPanels(layout, sourceId, targetId));
|
onLayoutChange((prev) => swapPanels(prev, sourceId, targetId));
|
||||||
setSwapSourceId(null);
|
setSwapSourceId(null);
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[onLayoutChange],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleMove = useCallback(
|
const handleMove = useCallback(
|
||||||
(sourceId: string, targetId: string, position: DropPosition) => {
|
(sourceId: string, targetId: string, position: DropPosition) => {
|
||||||
onLayoutChange(movePanel(layout, sourceId, targetId, position));
|
onLayoutChange((prev) => movePanel(prev, sourceId, targetId, position));
|
||||||
setDragSourceId(null);
|
setDragSourceId(null);
|
||||||
},
|
},
|
||||||
[layout, onLayoutChange],
|
[onLayoutChange],
|
||||||
);
|
);
|
||||||
|
|
||||||
const startDrag = useCallback(
|
const startDrag = useCallback(
|
||||||
|
|||||||
Reference in New Issue
Block a user