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.
This commit is contained in:
2026-04-03 22:01:18 +02:00
parent d1c14c93e5
commit 9fb850f2a4
4 changed files with 89 additions and 65 deletions

View File

@@ -1,6 +1,6 @@
// @vitest-environment jsdom // @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 { createRoot, type Root } from "react-dom/client";
import type { Edge as RFEdge, Node as RFNode } from "@xyflow/react"; import type { Edge as RFEdge, Node as RFNode } from "@xyflow/react";
import { afterEach, describe, expect, it, vi } from "vitest"; 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 asCanvasId = (id: string): Id<"canvases"> => id as Id<"canvases">;
const asNodeId = (id: string): Id<"nodes"> => id as Id<"nodes">; const asNodeId = (id: string): Id<"nodes"> => id as Id<"nodes">;
type HarnessProps = { type HarnessProps = {
canvasId: Id<"canvases">;
initialNodes: RFNode[]; initialNodes: RFNode[];
initialEdges: RFEdge[]; initialEdges: RFEdge[];
convexNodes?: Doc<"nodes">[]; convexNodes?: Doc<"nodes">[];
convexEdges?: Doc<"edges">[]; convexEdges?: Doc<"edges">[];
storageUrlsById: Record<string, string>; storageUrlsById?: Record<string, string | undefined>;
themeMode: "light" | "dark"; themeMode: "light" | "dark";
edgeSyncNonce: number; pendingRemovedEdgeIds?: Set<string>;
pendingMovePins?: Map<string, { x: number; y: number }>;
isDragging: boolean; isDragging: boolean;
isResizing: boolean; isResizing: boolean;
resolvedRealIdByClientRequest: Map<string, Id<"nodes">>; resolvedRealIdByClientRequest: Map<string, Id<"nodes">>;
@@ -56,6 +57,7 @@ function HookHarness(props: HarnessProps) {
const [nodes, setNodes] = useState<RFNode[]>(props.initialNodes); const [nodes, setNodes] = useState<RFNode[]>(props.initialNodes);
const [edges, setEdges] = useState<RFEdge[]>(props.initialEdges); const [edges, setEdges] = useState<RFEdge[]>(props.initialEdges);
const nodesRef = useRef(nodes); const nodesRef = useRef(nodes);
const edgesRef = useRef(edges);
const deletingNodeIds = useRef(new Set<string>()); const deletingNodeIds = useRef(new Set<string>());
const convexNodeIdsSnapshotForEdgeCarryRef = useRef( const convexNodeIdsSnapshotForEdgeCarryRef = useRef(
props.previousConvexNodeIdsSnapshot, props.previousConvexNodeIdsSnapshot,
@@ -64,6 +66,14 @@ function HookHarness(props: HarnessProps) {
props.resolvedRealIdByClientRequest, props.resolvedRealIdByClientRequest,
); );
const pendingConnectionCreatesRef = useRef(props.pendingConnectionCreateIds); const pendingConnectionCreatesRef = useRef(props.pendingConnectionCreateIds);
const pendingRemovedEdgeIds = useMemo(
() => props.pendingRemovedEdgeIds ?? new Set<string>(),
[props.pendingRemovedEdgeIds],
);
const pendingMovePins = useMemo(
() => props.pendingMovePins ?? new Map<string, { x: number; y: number }>(),
[props.pendingMovePins],
);
const pendingLocalPositionUntilConvexMatchesRef = useRef( const pendingLocalPositionUntilConvexMatchesRef = useRef(
props.pendingLocalPositionPins ?? new Map<string, { x: number; y: number }>(), props.pendingLocalPositionPins ?? new Map<string, { x: number; y: number }>(),
); );
@@ -77,23 +87,27 @@ function HookHarness(props: HarnessProps) {
nodesRef.current = nodes; nodesRef.current = nodes;
}, [nodes]); }, [nodes]);
useEffect(() => {
edgesRef.current = edges;
}, [edges]);
useEffect(() => { useEffect(() => {
isDraggingRef.current = props.isDragging; isDraggingRef.current = props.isDragging;
isResizingRef.current = props.isResizing; isResizingRef.current = props.isResizing;
}, [props.isDragging, props.isResizing]); }, [props.isDragging, props.isResizing]);
useCanvasFlowReconciliation({ useCanvasFlowReconciliation({
canvasId: props.canvasId,
convexNodes: props.convexNodes, convexNodes: props.convexNodes,
convexEdges: props.convexEdges, convexEdges: props.convexEdges,
storageUrlsById: props.storageUrlsById, storageUrlsById: props.storageUrlsById,
themeMode: props.themeMode, themeMode: props.themeMode,
edges, pendingRemovedEdgeIds,
edgeSyncNonce: props.edgeSyncNonce, pendingMovePins,
setNodes, setNodes,
setEdges, setEdges,
refs: { refs: {
nodesRef, nodesRef,
edgesRef,
deletingNodeIds, deletingNodeIds,
convexNodeIdsSnapshotForEdgeCarryRef, convexNodeIdsSnapshotForEdgeCarryRef,
resolvedRealIdByClientRequestRef, resolvedRealIdByClientRequestRef,
@@ -141,9 +155,8 @@ describe("useCanvasFlowReconciliation", () => {
await act(async () => { await act(async () => {
root?.render( root?.render(
<HookHarness React.createElement(HookHarness, {
canvasId={asCanvasId("canvas-1")} initialNodes: [
initialNodes={[
{ {
id: "node-source", id: "node-source",
type: "image", type: "image",
@@ -156,15 +169,15 @@ describe("useCanvasFlowReconciliation", () => {
position: { x: 120, y: 80 }, position: { x: 120, y: 80 },
data: {}, data: {},
}, },
]} ],
initialEdges={[ initialEdges: [
{ {
id: "optimistic_edge_req-1", id: "optimistic_edge_req-1",
source: "node-source", source: "node-source",
target: "optimistic_req-1", target: "optimistic_req-1",
}, },
]} ],
convexNodes={[ convexNodes: [
{ {
_id: asNodeId("node-source"), _id: asNodeId("node-source"),
_creationTime: 0, _creationTime: 0,
@@ -187,17 +200,16 @@ describe("useCanvasFlowReconciliation", () => {
height: 220, height: 220,
data: {}, data: {},
} as Doc<"nodes">, } as Doc<"nodes">,
]} ],
convexEdges={[]} convexEdges: [],
storageUrlsById={{}} storageUrlsById: {},
themeMode="light" themeMode: "light",
edgeSyncNonce={0} isDragging: false,
isDragging={false} isResizing: false,
isResizing={false} resolvedRealIdByClientRequest: new Map<string, Id<"nodes">>(),
resolvedRealIdByClientRequest={new Map()} pendingConnectionCreateIds: new Set(["req-1"]),
pendingConnectionCreateIds={new Set(["req-1"])} previousConvexNodeIdsSnapshot: new Set(["node-source"]),
previousConvexNodeIdsSnapshot={new Set(["node-source"])} }),
/>,
); );
}); });
@@ -226,9 +238,8 @@ describe("useCanvasFlowReconciliation", () => {
await act(async () => { await act(async () => {
root?.render( root?.render(
<HookHarness React.createElement(HookHarness, {
canvasId={asCanvasId("canvas-1")} initialNodes: [
initialNodes={[
{ {
id: "optimistic_req-drag", id: "optimistic_req-drag",
type: "image", type: "image",
@@ -236,9 +247,9 @@ describe("useCanvasFlowReconciliation", () => {
data: { label: "local" }, data: { label: "local" },
dragging: true, dragging: true,
}, },
]} ],
initialEdges={[]} initialEdges: [],
convexNodes={[ convexNodes: [
{ {
_id: asNodeId("node-real"), _id: asNodeId("node-real"),
_creationTime: 1, _creationTime: 1,
@@ -250,19 +261,18 @@ describe("useCanvasFlowReconciliation", () => {
height: 200, height: 200,
data: { label: "server" }, data: { label: "server" },
} as Doc<"nodes">, } as Doc<"nodes">,
]} ],
convexEdges={[] as Doc<"edges">[]} convexEdges: [] as Doc<"edges">[],
storageUrlsById={{}} storageUrlsById: {},
themeMode="light" themeMode: "light",
edgeSyncNonce={0} isDragging: false,
isDragging={false} isResizing: false,
isResizing={false} resolvedRealIdByClientRequest: new Map([
resolvedRealIdByClientRequest={new Map([
["req-drag", asNodeId("node-real")], ["req-drag", asNodeId("node-real")],
])} ]),
pendingConnectionCreateIds={new Set()} pendingConnectionCreateIds: new Set<string>(),
previousConvexNodeIdsSnapshot={new Set(["node-real"])} previousConvexNodeIdsSnapshot: new Set(["node-real"]),
/>, }),
); );
}); });

View File

@@ -77,6 +77,8 @@ import {
getMiniMapNodeStrokeColor, getMiniMapNodeStrokeColor,
getNodeCenterClientPosition, getNodeCenterClientPosition,
getIntersectedEdgeId, getIntersectedEdgeId,
getPendingRemovedEdgeIdsFromLocalOps,
getPendingMovePinsFromLocalOps,
hasHandleKey, hasHandleKey,
isEditableKeyboardTarget, isEditableKeyboardTarget,
isOptimisticEdgeId, isOptimisticEdgeId,
@@ -239,6 +241,24 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
[assetBrowserTargetNodeId], [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) => { const handleNavToolChange = useCallback((tool: CanvasNavTool) => {
if (tool === "scissor") { if (tool === "scissor") {
setScissorsMode(true); setScissorsMode(true);
@@ -339,17 +359,17 @@ function CanvasInner({ canvasId }: CanvasInnerProps) {
}); });
useCanvasFlowReconciliation({ useCanvasFlowReconciliation({
canvasId,
convexNodes, convexNodes,
convexEdges, convexEdges,
storageUrlsById, storageUrlsById,
themeMode: resolvedTheme === "dark" ? "dark" : "light", themeMode: resolvedTheme === "dark" ? "dark" : "light",
edges, pendingRemovedEdgeIds,
edgeSyncNonce, pendingMovePins,
setNodes, setNodes,
setEdges, setEdges,
refs: { refs: {
nodesRef, nodesRef,
edgesRef,
deletingNodeIds, deletingNodeIds,
convexNodeIdsSnapshotForEdgeCarryRef, convexNodeIdsSnapshotForEdgeCarryRef,
resolvedRealIdByClientRequestRef, resolvedRealIdByClientRequestRef,

View File

@@ -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 { Edge as RFEdge, Node as RFNode } from "@xyflow/react";
import type { Doc, Id } from "@/convex/_generated/dataModel"; import type { Doc, Id } from "@/convex/_generated/dataModel";
import {
getPendingMovePinsFromLocalOps,
getPendingRemovedEdgeIdsFromLocalOps,
} from "./canvas-helpers";
import { import {
buildIncomingCanvasFlowNodes, buildIncomingCanvasFlowNodes,
reconcileCanvasFlowEdges, reconcileCanvasFlowEdges,
@@ -17,6 +12,7 @@ type PositionPin = { x: number; y: number };
type CanvasFlowReconciliationRefs = { type CanvasFlowReconciliationRefs = {
nodesRef: MutableRefObject<RFNode[]>; nodesRef: MutableRefObject<RFNode[]>;
edgesRef: MutableRefObject<RFEdge[]>;
deletingNodeIds: MutableRefObject<Set<string>>; deletingNodeIds: MutableRefObject<Set<string>>;
convexNodeIdsSnapshotForEdgeCarryRef: MutableRefObject<Set<string>>; convexNodeIdsSnapshotForEdgeCarryRef: MutableRefObject<Set<string>>;
resolvedRealIdByClientRequestRef: MutableRefObject<Map<string, Id<"nodes">>>; resolvedRealIdByClientRequestRef: MutableRefObject<Map<string, Id<"nodes">>>;
@@ -30,30 +26,29 @@ type CanvasFlowReconciliationRefs = {
}; };
export function useCanvasFlowReconciliation(args: { export function useCanvasFlowReconciliation(args: {
canvasId: Id<"canvases">;
convexNodes: Doc<"nodes">[] | undefined; convexNodes: Doc<"nodes">[] | undefined;
convexEdges: Doc<"edges">[] | undefined; convexEdges: Doc<"edges">[] | undefined;
storageUrlsById: Record<string, string | undefined> | undefined; storageUrlsById: Record<string, string | undefined> | undefined;
themeMode: "light" | "dark"; themeMode: "light" | "dark";
edges: RFEdge[]; pendingRemovedEdgeIds: ReadonlySet<string>;
edgeSyncNonce: number; pendingMovePins: ReadonlyMap<string, PositionPin>;
setNodes: Dispatch<SetStateAction<RFNode[]>>; setNodes: Dispatch<SetStateAction<RFNode[]>>;
setEdges: Dispatch<SetStateAction<RFEdge[]>>; setEdges: Dispatch<SetStateAction<RFEdge[]>>;
refs: CanvasFlowReconciliationRefs; refs: CanvasFlowReconciliationRefs;
}) { }) {
const { const {
canvasId,
convexEdges, convexEdges,
convexNodes, convexNodes,
storageUrlsById, storageUrlsById,
themeMode, themeMode,
edges, pendingRemovedEdgeIds,
edgeSyncNonce, pendingMovePins,
setNodes, setNodes,
setEdges, setEdges,
} = args; } = args;
const { const {
nodesRef, nodesRef,
edgesRef,
deletingNodeIds, deletingNodeIds,
convexNodeIdsSnapshotForEdgeCarryRef, convexNodeIdsSnapshotForEdgeCarryRef,
resolvedRealIdByClientRequestRef, resolvedRealIdByClientRequestRef,
@@ -73,7 +68,7 @@ export function useCanvasFlowReconciliation(args: {
convexEdges, convexEdges,
convexNodes, convexNodes,
previousConvexNodeIdsSnapshot: convexNodeIdsSnapshotForEdgeCarryRef.current, previousConvexNodeIdsSnapshot: convexNodeIdsSnapshotForEdgeCarryRef.current,
pendingRemovedEdgeIds: getPendingRemovedEdgeIdsFromLocalOps(canvasId as string), pendingRemovedEdgeIds,
pendingConnectionCreateIds: pendingConnectionCreatesRef.current, pendingConnectionCreateIds: pendingConnectionCreatesRef.current,
resolvedRealIdByClientRequest: resolvedRealIdByClientRequestRef.current, resolvedRealIdByClientRequest: resolvedRealIdByClientRequestRef.current,
localNodeIds: new Set(nodesRef.current.map((node) => node.id)), localNodeIds: new Set(nodesRef.current.map((node) => node.id)),
@@ -96,13 +91,13 @@ export function useCanvasFlowReconciliation(args: {
return reconciliation.edges; return reconciliation.edges;
}); });
}, [ }, [
canvasId,
convexEdges, convexEdges,
convexNodes, convexNodes,
edgeSyncNonce, pendingRemovedEdgeIds,
setEdges, setEdges,
themeMode, themeMode,
convexNodeIdsSnapshotForEdgeCarryRef, convexNodeIdsSnapshotForEdgeCarryRef,
edgesRef,
isDragging, isDragging,
nodesRef, nodesRef,
pendingConnectionCreatesRef, pendingConnectionCreatesRef,
@@ -124,7 +119,7 @@ export function useCanvasFlowReconciliation(args: {
convexNodes, convexNodes,
storageUrlsById, storageUrlsById,
previousNodes, previousNodes,
edges, edges: edgesRef.current,
}); });
const reconciliation = reconcileCanvasFlowNodes({ const reconciliation = reconcileCanvasFlowNodes({
@@ -136,7 +131,7 @@ export function useCanvasFlowReconciliation(args: {
pendingConnectionCreateIds: pendingConnectionCreatesRef.current, pendingConnectionCreateIds: pendingConnectionCreatesRef.current,
preferLocalPositionNodeIds: preferLocalPositionNodeIdsRef.current, preferLocalPositionNodeIds: preferLocalPositionNodeIdsRef.current,
pendingLocalPositionPins: pendingLocalPositionUntilConvexMatchesRef.current, pendingLocalPositionPins: pendingLocalPositionUntilConvexMatchesRef.current,
pendingMovePins: getPendingMovePinsFromLocalOps(canvasId as string), pendingMovePins,
}); });
resolvedRealIdByClientRequestRef.current = resolvedRealIdByClientRequestRef.current =
@@ -150,9 +145,9 @@ export function useCanvasFlowReconciliation(args: {
return reconciliation.nodes; return reconciliation.nodes;
}); });
}, [ }, [
canvasId,
convexNodes, convexNodes,
edges, edgesRef,
pendingMovePins,
setNodes, setNodes,
storageUrlsById, storageUrlsById,
deletingNodeIds, deletingNodeIds,

View File

@@ -12,7 +12,6 @@ export default defineConfig({
include: [ include: [
"tests/**/*.test.ts", "tests/**/*.test.ts",
"components/canvas/__tests__/canvas-flow-reconciliation-helpers.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.test.ts",
"components/canvas/__tests__/use-canvas-sync-engine-hook.test.tsx", "components/canvas/__tests__/use-canvas-sync-engine-hook.test.tsx",
], ],