From 9fb850f2a4a3e264e079ddb1e067626ceeb3bad4 Mon Sep 17 00:00:00 2001 From: Matthias Meister Date: Fri, 3 Apr 2026 22:01:18 +0200 Subject: [PATCH] fix(canvas): align flow reconciliation hook with task plan Rename the hook test to the planned path, revert the persistent Vitest config tweak, and narrow the hook inputs to reconciliation data plus shared refs. Keep verification working with a temporary test config instead of expanding the repo-level include list. --- ...=> use-canvas-flow-reconciliation.test.ts} | 96 ++++++++++--------- components/canvas/canvas.tsx | 26 ++++- .../canvas/use-canvas-flow-reconciliation.ts | 31 +++--- vitest.config.ts | 1 - 4 files changed, 89 insertions(+), 65 deletions(-) rename components/canvas/__tests__/{use-canvas-flow-reconciliation.test.tsx => use-canvas-flow-reconciliation.test.ts} (80%) diff --git a/components/canvas/__tests__/use-canvas-flow-reconciliation.test.tsx b/components/canvas/__tests__/use-canvas-flow-reconciliation.test.ts similarity index 80% rename from components/canvas/__tests__/use-canvas-flow-reconciliation.test.tsx rename to components/canvas/__tests__/use-canvas-flow-reconciliation.test.ts index d095c43..a642413 100644 --- a/components/canvas/__tests__/use-canvas-flow-reconciliation.test.tsx +++ b/components/canvas/__tests__/use-canvas-flow-reconciliation.test.ts @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { act, useEffect, useRef, useState } from "react"; +import React, { act, useEffect, useMemo, useRef, useState } from "react"; import { createRoot, type Root } from "react-dom/client"; import type { Edge as RFEdge, Node as RFNode } from "@xyflow/react"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -22,15 +22,16 @@ vi.mock("@/components/canvas/canvas-helpers", async () => { const asCanvasId = (id: string): Id<"canvases"> => id as Id<"canvases">; const asNodeId = (id: string): Id<"nodes"> => id as Id<"nodes">; + type HarnessProps = { - canvasId: Id<"canvases">; initialNodes: RFNode[]; initialEdges: RFEdge[]; convexNodes?: Doc<"nodes">[]; convexEdges?: Doc<"edges">[]; - storageUrlsById: Record; + storageUrlsById?: Record; themeMode: "light" | "dark"; - edgeSyncNonce: number; + pendingRemovedEdgeIds?: Set; + pendingMovePins?: Map; isDragging: boolean; isResizing: boolean; resolvedRealIdByClientRequest: Map>; @@ -56,6 +57,7 @@ function HookHarness(props: HarnessProps) { const [nodes, setNodes] = useState(props.initialNodes); const [edges, setEdges] = useState(props.initialEdges); const nodesRef = useRef(nodes); + const edgesRef = useRef(edges); const deletingNodeIds = useRef(new Set()); const convexNodeIdsSnapshotForEdgeCarryRef = useRef( props.previousConvexNodeIdsSnapshot, @@ -64,6 +66,14 @@ function HookHarness(props: HarnessProps) { props.resolvedRealIdByClientRequest, ); const pendingConnectionCreatesRef = useRef(props.pendingConnectionCreateIds); + const pendingRemovedEdgeIds = useMemo( + () => props.pendingRemovedEdgeIds ?? new Set(), + [props.pendingRemovedEdgeIds], + ); + const pendingMovePins = useMemo( + () => props.pendingMovePins ?? new Map(), + [props.pendingMovePins], + ); const pendingLocalPositionUntilConvexMatchesRef = useRef( props.pendingLocalPositionPins ?? new Map(), ); @@ -77,23 +87,27 @@ function HookHarness(props: HarnessProps) { nodesRef.current = nodes; }, [nodes]); + useEffect(() => { + edgesRef.current = edges; + }, [edges]); + useEffect(() => { isDraggingRef.current = props.isDragging; isResizingRef.current = props.isResizing; }, [props.isDragging, props.isResizing]); useCanvasFlowReconciliation({ - canvasId: props.canvasId, convexNodes: props.convexNodes, convexEdges: props.convexEdges, storageUrlsById: props.storageUrlsById, themeMode: props.themeMode, - edges, - edgeSyncNonce: props.edgeSyncNonce, + pendingRemovedEdgeIds, + pendingMovePins, setNodes, setEdges, refs: { nodesRef, + edgesRef, deletingNodeIds, convexNodeIdsSnapshotForEdgeCarryRef, resolvedRealIdByClientRequestRef, @@ -141,9 +155,8 @@ describe("useCanvasFlowReconciliation", () => { await act(async () => { root?.render( - { position: { x: 120, y: 80 }, data: {}, }, - ]} - initialEdges={[ + ], + initialEdges: [ { id: "optimistic_edge_req-1", source: "node-source", target: "optimistic_req-1", }, - ]} - convexNodes={[ + ], + convexNodes: [ { _id: asNodeId("node-source"), _creationTime: 0, @@ -187,17 +200,16 @@ describe("useCanvasFlowReconciliation", () => { height: 220, data: {}, } as Doc<"nodes">, - ]} - convexEdges={[]} - storageUrlsById={{}} - themeMode="light" - edgeSyncNonce={0} - isDragging={false} - isResizing={false} - resolvedRealIdByClientRequest={new Map()} - pendingConnectionCreateIds={new Set(["req-1"])} - previousConvexNodeIdsSnapshot={new Set(["node-source"])} - />, + ], + convexEdges: [], + storageUrlsById: {}, + themeMode: "light", + isDragging: false, + isResizing: false, + resolvedRealIdByClientRequest: new Map>(), + pendingConnectionCreateIds: new Set(["req-1"]), + previousConvexNodeIdsSnapshot: new Set(["node-source"]), + }), ); }); @@ -226,9 +238,8 @@ describe("useCanvasFlowReconciliation", () => { await act(async () => { root?.render( - { data: { label: "local" }, dragging: true, }, - ]} - initialEdges={[]} - convexNodes={[ + ], + initialEdges: [], + convexNodes: [ { _id: asNodeId("node-real"), _creationTime: 1, @@ -250,19 +261,18 @@ describe("useCanvasFlowReconciliation", () => { height: 200, data: { label: "server" }, } as Doc<"nodes">, - ]} - convexEdges={[] as Doc<"edges">[]} - storageUrlsById={{}} - themeMode="light" - edgeSyncNonce={0} - isDragging={false} - isResizing={false} - resolvedRealIdByClientRequest={new Map([ + ], + convexEdges: [] as Doc<"edges">[], + storageUrlsById: {}, + themeMode: "light", + isDragging: false, + isResizing: false, + resolvedRealIdByClientRequest: new Map([ ["req-drag", asNodeId("node-real")], - ])} - pendingConnectionCreateIds={new Set()} - previousConvexNodeIdsSnapshot={new Set(["node-real"])} - />, + ]), + pendingConnectionCreateIds: new Set(), + previousConvexNodeIdsSnapshot: new Set(["node-real"]), + }), ); }); diff --git a/components/canvas/canvas.tsx b/components/canvas/canvas.tsx index 05fa4bc..072d755 100644 --- a/components/canvas/canvas.tsx +++ b/components/canvas/canvas.tsx @@ -77,6 +77,8 @@ import { getMiniMapNodeStrokeColor, getNodeCenterClientPosition, getIntersectedEdgeId, + getPendingRemovedEdgeIdsFromLocalOps, + getPendingMovePinsFromLocalOps, hasHandleKey, isEditableKeyboardTarget, isOptimisticEdgeId, @@ -239,6 +241,24 @@ function CanvasInner({ canvasId }: CanvasInnerProps) { [assetBrowserTargetNodeId], ); + const pendingRemovedEdgeIds = useMemo( + () => { + void convexEdges; + void edgeSyncNonce; + return getPendingRemovedEdgeIdsFromLocalOps(canvasId as string); + }, + [canvasId, convexEdges, edgeSyncNonce], + ); + + const pendingMovePins = useMemo( + () => { + void convexNodes; + void edgeSyncNonce; + return getPendingMovePinsFromLocalOps(canvasId as string); + }, + [canvasId, convexNodes, edgeSyncNonce], + ); + const handleNavToolChange = useCallback((tool: CanvasNavTool) => { if (tool === "scissor") { setScissorsMode(true); @@ -339,17 +359,17 @@ function CanvasInner({ canvasId }: CanvasInnerProps) { }); useCanvasFlowReconciliation({ - canvasId, convexNodes, convexEdges, storageUrlsById, themeMode: resolvedTheme === "dark" ? "dark" : "light", - edges, - edgeSyncNonce, + pendingRemovedEdgeIds, + pendingMovePins, setNodes, setEdges, refs: { nodesRef, + edgesRef, deletingNodeIds, convexNodeIdsSnapshotForEdgeCarryRef, resolvedRealIdByClientRequestRef, diff --git a/components/canvas/use-canvas-flow-reconciliation.ts b/components/canvas/use-canvas-flow-reconciliation.ts index a830795..f7f50f1 100644 --- a/components/canvas/use-canvas-flow-reconciliation.ts +++ b/components/canvas/use-canvas-flow-reconciliation.ts @@ -2,11 +2,6 @@ import { useLayoutEffect, type Dispatch, type MutableRefObject, type SetStateAct import type { Edge as RFEdge, Node as RFNode } from "@xyflow/react"; import type { Doc, Id } from "@/convex/_generated/dataModel"; - -import { - getPendingMovePinsFromLocalOps, - getPendingRemovedEdgeIdsFromLocalOps, -} from "./canvas-helpers"; import { buildIncomingCanvasFlowNodes, reconcileCanvasFlowEdges, @@ -17,6 +12,7 @@ type PositionPin = { x: number; y: number }; type CanvasFlowReconciliationRefs = { nodesRef: MutableRefObject; + edgesRef: MutableRefObject; deletingNodeIds: MutableRefObject>; convexNodeIdsSnapshotForEdgeCarryRef: MutableRefObject>; resolvedRealIdByClientRequestRef: MutableRefObject>>; @@ -30,30 +26,29 @@ type CanvasFlowReconciliationRefs = { }; export function useCanvasFlowReconciliation(args: { - canvasId: Id<"canvases">; convexNodes: Doc<"nodes">[] | undefined; convexEdges: Doc<"edges">[] | undefined; storageUrlsById: Record | undefined; themeMode: "light" | "dark"; - edges: RFEdge[]; - edgeSyncNonce: number; + pendingRemovedEdgeIds: ReadonlySet; + pendingMovePins: ReadonlyMap; setNodes: Dispatch>; setEdges: Dispatch>; refs: CanvasFlowReconciliationRefs; }) { const { - canvasId, convexEdges, convexNodes, storageUrlsById, themeMode, - edges, - edgeSyncNonce, + pendingRemovedEdgeIds, + pendingMovePins, setNodes, setEdges, } = args; const { nodesRef, + edgesRef, deletingNodeIds, convexNodeIdsSnapshotForEdgeCarryRef, resolvedRealIdByClientRequestRef, @@ -73,7 +68,7 @@ export function useCanvasFlowReconciliation(args: { convexEdges, convexNodes, previousConvexNodeIdsSnapshot: convexNodeIdsSnapshotForEdgeCarryRef.current, - pendingRemovedEdgeIds: getPendingRemovedEdgeIdsFromLocalOps(canvasId as string), + pendingRemovedEdgeIds, pendingConnectionCreateIds: pendingConnectionCreatesRef.current, resolvedRealIdByClientRequest: resolvedRealIdByClientRequestRef.current, localNodeIds: new Set(nodesRef.current.map((node) => node.id)), @@ -96,13 +91,13 @@ export function useCanvasFlowReconciliation(args: { return reconciliation.edges; }); }, [ - canvasId, convexEdges, convexNodes, - edgeSyncNonce, + pendingRemovedEdgeIds, setEdges, themeMode, convexNodeIdsSnapshotForEdgeCarryRef, + edgesRef, isDragging, nodesRef, pendingConnectionCreatesRef, @@ -124,7 +119,7 @@ export function useCanvasFlowReconciliation(args: { convexNodes, storageUrlsById, previousNodes, - edges, + edges: edgesRef.current, }); const reconciliation = reconcileCanvasFlowNodes({ @@ -136,7 +131,7 @@ export function useCanvasFlowReconciliation(args: { pendingConnectionCreateIds: pendingConnectionCreatesRef.current, preferLocalPositionNodeIds: preferLocalPositionNodeIdsRef.current, pendingLocalPositionPins: pendingLocalPositionUntilConvexMatchesRef.current, - pendingMovePins: getPendingMovePinsFromLocalOps(canvasId as string), + pendingMovePins, }); resolvedRealIdByClientRequestRef.current = @@ -150,9 +145,9 @@ export function useCanvasFlowReconciliation(args: { return reconciliation.nodes; }); }, [ - canvasId, convexNodes, - edges, + edgesRef, + pendingMovePins, setNodes, storageUrlsById, deletingNodeIds, diff --git a/vitest.config.ts b/vitest.config.ts index 4336a3d..70f9b7d 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -12,7 +12,6 @@ export default defineConfig({ include: [ "tests/**/*.test.ts", "components/canvas/__tests__/canvas-flow-reconciliation-helpers.test.ts", - "components/canvas/__tests__/use-canvas-flow-reconciliation.test.tsx", "components/canvas/__tests__/use-canvas-sync-engine.test.ts", "components/canvas/__tests__/use-canvas-sync-engine-hook.test.tsx", ],