Fix spawn-delete race and keep node deletion locked until synced

This commit is contained in:
Matthias
2026-04-01 11:22:11 +02:00
parent eb5ed06ced
commit 37a346a2b1
3 changed files with 95 additions and 8 deletions

View File

@@ -130,9 +130,8 @@ export function useCanvasDeleteHandlers({
...edgePromises, ...edgePromises,
]) ])
.then(() => { .then(() => {
for (const id of idsToDelete) { // Erfolg bedeutet hier nur: Mutation/Queue wurde angenommen.
deletingNodeIds.current.delete(id); // Den Delete-Lock erst lösen, wenn Convex-Snapshot die Node wirklich nicht mehr enthält.
}
}) })
.catch((error: unknown) => { .catch((error: unknown) => {
console.error("[Canvas] batch remove failed", error); console.error("[Canvas] batch remove failed", error);

View File

@@ -230,6 +230,7 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
const pendingEdgeSplitByClientRequestRef = useRef( const pendingEdgeSplitByClientRequestRef = useRef(
new Map<string, PendingEdgeSplit>(), new Map<string, PendingEdgeSplit>(),
); );
const pendingDeleteAfterCreateClientRequestIdsRef = useRef(new Set<string>());
/** Connection-Drop → neue Node: erlaubt Carry-over der Kante in der Rollback-Lücke (ohne Phantom nach Fehler). */ /** Connection-Drop → neue Node: erlaubt Carry-over der Kante in der Rollback-Lücke (ohne Phantom nach Fehler). */
const pendingConnectionCreatesRef = useRef(new Set<string>()); const pendingConnectionCreatesRef = useRef(new Set<string>());
/** Nach create+drag: Convex liefert oft noch Erstellkoordinaten, bis `moveNode` committed — bis dahin Position pinnen. */ /** Nach create+drag: Convex liefert oft noch Erstellkoordinaten, bis `moveNode` committed — bis dahin Position pinnen. */
@@ -663,6 +664,22 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
const optimisticNodeId = `${OPTIMISTIC_NODE_PREFIX}${clientRequestId}`; const optimisticNodeId = `${OPTIMISTIC_NODE_PREFIX}${clientRequestId}`;
const realNodeId = realId as string; const realNodeId = realId as string;
if (
pendingDeleteAfterCreateClientRequestIdsRef.current.has(clientRequestId)
) {
pendingDeleteAfterCreateClientRequestIdsRef.current.delete(clientRequestId);
removeOptimisticCreateLocally({
clientRequestId,
removeNode: true,
removeEdge: true,
});
deletingNodeIds.current.add(realNodeId);
await enqueueSyncMutationRef.current("batchRemoveNodes", {
nodeIds: [realId],
});
return;
}
setNodes((current) => setNodes((current) =>
current.map((node) => { current.map((node) => {
const nextParentId = const nextParentId =
@@ -712,7 +729,7 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
resolvedRealIdByClientRequestRef.current.set(clientRequestId, realId); resolvedRealIdByClientRequestRef.current.set(clientRequestId, realId);
await remapCanvasSyncNodeId(canvasId as string, optimisticNodeId, realNodeId); await remapCanvasSyncNodeId(canvasId as string, optimisticNodeId, realNodeId);
remapCanvasOpNodeId(canvasId as string, optimisticNodeId, realNodeId); remapCanvasOpNodeId(canvasId as string, optimisticNodeId, realNodeId);
}, [canvasId]); }, [canvasId, removeOptimisticCreateLocally]);
const runCreateNodeOnlineOnly = useCallback( const runCreateNodeOnlineOnly = useCallback(
async (args: Parameters<typeof createNode>[0]) => { async (args: Parameters<typeof createNode>[0]) => {
@@ -966,6 +983,10 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
removeEdge: true, removeEdge: true,
}); });
setEdgeSyncNonce((value) => value + 1); setEdgeSyncNonce((value) => value + 1);
} else if (op.type === "batchRemoveNodes") {
for (const nodeId of op.payload.nodeIds) {
deletingNodeIds.current.delete(nodeId as string);
}
} }
await ackCanvasSyncOp(op.id); await ackCanvasSyncOp(op.id);
resolveCanvasOp(canvasId as string, op.id); resolveCanvasOp(canvasId as string, op.id);
@@ -1104,6 +1125,14 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
.filter((id): id is string => id !== null); .filter((id): id is string => id !== null);
if (createClientRequestIds.length > 0) { if (createClientRequestIds.length > 0) {
if (isSyncOnline) {
for (const clientRequestId of createClientRequestIds) {
pendingDeleteAfterCreateClientRequestIdsRef.current.add(
clientRequestId,
);
}
}
const droppedSync = await dropCanvasSyncOpsByClientRequestIds( const droppedSync = await dropCanvasSyncOpsByClientRequestIds(
canvasId as string, canvasId as string,
createClientRequestIds, createClientRequestIds,
@@ -1146,6 +1175,7 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
[ [
canvasId, canvasId,
enqueueSyncMutation, enqueueSyncMutation,
isSyncOnline,
refreshPendingSyncCount, refreshPendingSyncCount,
removeOptimisticCreateLocally, removeOptimisticCreateLocally,
], ],
@@ -1325,6 +1355,31 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
if (isOptimisticNodeId(realId as string)) { if (isOptimisticNodeId(realId as string)) {
return; return;
} }
if (
pendingDeleteAfterCreateClientRequestIdsRef.current.has(clientRequestId)
) {
pendingDeleteAfterCreateClientRequestIdsRef.current.delete(
clientRequestId,
);
pendingMoveAfterCreateRef.current.delete(clientRequestId);
pendingEdgeSplitByClientRequestRef.current.delete(clientRequestId);
pendingConnectionCreatesRef.current.delete(clientRequestId);
resolvedRealIdByClientRequestRef.current.delete(clientRequestId);
const realNodeId = realId as string;
deletingNodeIds.current.add(realNodeId);
setNodes((current) =>
current.filter((node) => node.id !== realNodeId),
);
setEdges((current) =>
current.filter(
(edge) =>
edge.source !== realNodeId && edge.target !== realNodeId,
),
);
await runBatchRemoveNodesMutation({ nodeIds: [realId] });
return;
}
const optimisticNodeId = `${OPTIMISTIC_NODE_PREFIX}${clientRequestId}`; const optimisticNodeId = `${OPTIMISTIC_NODE_PREFIX}${clientRequestId}`;
setAssetBrowserTargetNodeId((current) => setAssetBrowserTargetNodeId((current) =>
current === optimisticNodeId ? (realId as string) : current, current === optimisticNodeId ? (realId as string) : current,
@@ -1426,7 +1481,12 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
}); });
} }
}, },
[canvasId, runMoveNodeMutation, runSplitEdgeAtExistingNodeMutation], [
canvasId,
runBatchRemoveNodesMutation,
runMoveNodeMutation,
runSplitEdgeAtExistingNodeMutation,
],
); );
syncPendingMoveForClientRequestRef.current = syncPendingMoveForClientRequest; syncPendingMoveForClientRequestRef.current = syncPendingMoveForClientRequest;

View File

@@ -1,7 +1,14 @@
"use client"; "use client";
import type { ReactNode } from "react"; import type { ReactNode } from "react";
import { NodeResizeControl, NodeToolbar, Position, useNodeId, useReactFlow } from "@xyflow/react"; import {
getConnectedEdges,
NodeResizeControl,
NodeToolbar,
Position,
useNodeId,
useReactFlow,
} from "@xyflow/react";
import { Trash2, Copy } from "lucide-react"; import { Trash2, Copy } from "lucide-react";
import { useCanvasPlacement } from "@/components/canvas/canvas-placement-context"; import { useCanvasPlacement } from "@/components/canvas/canvas-placement-context";
import { NodeErrorBoundary } from "./node-error-boundary"; import { NodeErrorBoundary } from "./node-error-boundary";
@@ -46,11 +53,32 @@ const INTERNAL_FIELDS = new Set([
function NodeToolbarActions() { function NodeToolbarActions() {
const nodeId = useNodeId(); const nodeId = useNodeId();
const { deleteElements, getNode, getNodes, setNodes } = useReactFlow(); const { deleteElements, getNode, getNodes, getEdges, setNodes } = useReactFlow();
const { createNodeWithIntersection } = useCanvasPlacement(); const { createNodeWithIntersection } = useCanvasPlacement();
const handleDelete = () => { const handleDelete = () => {
if (!nodeId) return; if (!nodeId) return;
void deleteElements({ nodes: [{ id: nodeId }] }); const node = getNode(nodeId);
const resolvedNode =
node ??
(() => {
const selectedNodes = getNodes().filter((candidate) => candidate.selected);
if (selectedNodes.length !== 1) return undefined;
return selectedNodes[0];
})();
const targetNodeId = resolvedNode?.id ?? nodeId;
const connectedEdges = resolvedNode
? getConnectedEdges([resolvedNode], getEdges())
: [];
void deleteElements({
nodes: [{ id: targetNodeId }],
edges: connectedEdges.map((edge) => ({ id: edge.id })),
}).catch((error: unknown) => {
console.error("[NodeToolbar] deleteElements failed", {
nodeId: targetNodeId,
error: String(error),
});
});
}; };
const handleDuplicate = () => { const handleDuplicate = () => {