fix(canvas): refresh sync engine hook dependencies

This commit is contained in:
2026-04-03 21:26:24 +02:00
parent c060c57ad8
commit 5223d3d8d7
6 changed files with 570 additions and 78 deletions

View File

@@ -0,0 +1,173 @@
// @vitest-environment jsdom
import { act, useEffect, 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";
import type { Id } from "@/convex/_generated/dataModel";
const mocks = vi.hoisted(() => ({
enqueueCanvasSyncOp: vi.fn(async () => ({ replacedIds: [] as string[] })),
countCanvasSyncOps: vi.fn(async () => 0),
listCanvasSyncOps: vi.fn(async () => []),
mutationMocks: new Map<unknown, ReturnType<typeof vi.fn>>(),
}));
vi.mock("@/convex/_generated/api", () => ({
api: {
nodes: {
move: "nodes.move",
resize: "nodes.resize",
updateData: "nodes.updateData",
create: "nodes.create",
createWithEdgeFromSource: "nodes.createWithEdgeFromSource",
createWithEdgeToTarget: "nodes.createWithEdgeToTarget",
createWithEdgeSplit: "nodes.createWithEdgeSplit",
batchRemove: "nodes.batchRemove",
splitEdgeAtExistingNode: "nodes.splitEdgeAtExistingNode",
},
edges: {
create: "edges.create",
remove: "edges.remove",
},
},
}));
vi.mock("convex/react", () => ({
useConvexConnectionState: () => ({ isWebSocketConnected: true }),
useMutation: (key: unknown) => {
let mutation = mocks.mutationMocks.get(key);
if (!mutation) {
mutation = vi.fn(async () => undefined);
Object.assign(mutation, {
withOptimisticUpdate: () => mutation,
});
mocks.mutationMocks.set(key, mutation);
}
return mutation;
},
}));
vi.mock("@/lib/canvas-op-queue", () => ({
ackCanvasSyncOp: vi.fn(async () => undefined),
countCanvasSyncOps: mocks.countCanvasSyncOps,
dropCanvasSyncOpsByClientRequestIds: vi.fn(async () => []),
dropCanvasSyncOpsByEdgeIds: vi.fn(async () => []),
dropCanvasSyncOpsByNodeIds: vi.fn(async () => []),
dropExpiredCanvasSyncOps: vi.fn(async () => []),
enqueueCanvasSyncOp: mocks.enqueueCanvasSyncOp,
listCanvasSyncOps: mocks.listCanvasSyncOps,
markCanvasSyncOpFailed: vi.fn(async () => undefined),
remapCanvasSyncNodeId: vi.fn(async () => 0),
}));
vi.mock("@/lib/canvas-local-persistence", () => ({
dropCanvasOpsByClientRequestIds: vi.fn(() => []),
dropCanvasOpsByEdgeIds: vi.fn(() => []),
dropCanvasOpsByNodeIds: vi.fn(() => []),
enqueueCanvasOp: vi.fn(() => "op-1"),
remapCanvasOpNodeId: vi.fn(() => 0),
resolveCanvasOp: vi.fn(() => undefined),
resolveCanvasOps: vi.fn(() => undefined),
}));
vi.mock("@/lib/toast", () => ({
toast: {
info: vi.fn(),
warning: vi.fn(),
},
}));
import { useCanvasSyncEngine } from "@/components/canvas/use-canvas-sync-engine";
const asCanvasId = (id: string): Id<"canvases"> => id as Id<"canvases">;
const asNodeId = (id: string): Id<"nodes"> => id as Id<"nodes">;
const latestHookValueRef: {
current: ReturnType<typeof useCanvasSyncEngine> | null;
} = { current: null };
(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
function HookHarness({ canvasId }: { canvasId: Id<"canvases"> }) {
const [, setNodes] = useState<RFNode[]>([]);
const [edges, setEdges] = useState<RFEdge[]>([]);
const edgesRef = useRef<RFEdge[]>(edges);
const deletingNodeIds = useRef(new Set<string>());
const [, setAssetBrowserTargetNodeId] = useState<string | null>(null);
const [, setEdgeSyncNonce] = useState(0);
useEffect(() => {
edgesRef.current = edges;
}, [edges]);
const hookValue = useCanvasSyncEngine({
canvasId,
setNodes,
setEdges,
edgesRef,
setAssetBrowserTargetNodeId,
setEdgeSyncNonce,
deletingNodeIds,
});
useEffect(() => {
latestHookValueRef.current = hookValue;
}, [hookValue]);
return null;
}
describe("useCanvasSyncEngine hook wiring", () => {
let container: HTMLDivElement | null = null;
let root: Root | null = null;
afterEach(async () => {
latestHookValueRef.current = null;
mocks.mutationMocks.clear();
vi.clearAllMocks();
if (root) {
await act(async () => {
root?.unmount();
});
}
container?.remove();
root = null;
container = null;
});
it("uses the latest canvas id after rerendering the mounted hook", async () => {
container = document.createElement("div");
document.body.appendChild(container);
root = createRoot(container);
await act(async () => {
root?.render(<HookHarness canvasId={asCanvasId("canvas-1")} />);
});
await act(async () => {
root?.render(<HookHarness canvasId={asCanvasId("canvas-2")} />);
});
await act(async () => {
await latestHookValueRef.current?.actions.resizeNode({
nodeId: asNodeId("node-1"),
width: 480,
height: 320,
});
});
expect(mocks.enqueueCanvasSyncOp).toHaveBeenLastCalledWith(
expect.objectContaining({
canvasId: "canvas-2",
type: "resizeNode",
payload: {
nodeId: "node-1",
width: 480,
height: 320,
},
}),
);
});
});

View File

@@ -14,9 +14,9 @@ describe("useCanvasSyncEngine", () => {
const controller = createCanvasSyncEngineController({
canvasId: asCanvasId("canvas-1"),
isSyncOnline: true,
enqueueSyncMutation,
runBatchRemoveNodes,
runSplitEdgeAtExistingNode,
getEnqueueSyncMutation: () => enqueueSyncMutation,
getRunBatchRemoveNodes: () => runBatchRemoveNodes,
getRunSplitEdgeAtExistingNode: () => runSplitEdgeAtExistingNode,
});
controller.pendingMoveAfterCreateRef.current.set("req-1", {
@@ -48,9 +48,9 @@ describe("useCanvasSyncEngine", () => {
const controller = createCanvasSyncEngineController({
canvasId: asCanvasId("canvas-1"),
isSyncOnline: true,
enqueueSyncMutation,
runBatchRemoveNodes: vi.fn(async () => undefined),
runSplitEdgeAtExistingNode: vi.fn(async () => undefined),
getEnqueueSyncMutation: () => enqueueSyncMutation,
getRunBatchRemoveNodes: () => vi.fn(async () => undefined),
getRunSplitEdgeAtExistingNode: () => vi.fn(async () => undefined),
});
await controller.queueNodeResize({

View File

@@ -51,6 +51,12 @@ type QueueSyncMutation = <TType extends keyof CanvasSyncOpPayloadByType>(
payload: CanvasSyncOpPayloadByType[TType],
) => Promise<void>;
type DynamicValue<T> = T | (() => T);
function resolveDynamicValue<T>(value: DynamicValue<T>): T {
return typeof value === "function" ? (value as () => T)() : value;
}
type RunMoveNodeMutation = (args: {
nodeId: Id<"nodes">;
positionX: number;
@@ -75,16 +81,18 @@ type RunSplitEdgeAtExistingNodeMutation = (args: {
}) => Promise<void>;
type CanvasSyncEngineControllerParams = {
canvasId: Id<"canvases">;
isSyncOnline: boolean | (() => boolean);
enqueueSyncMutation: QueueSyncMutation;
runMoveNodeMutation?: RunMoveNodeMutation;
runBatchRemoveNodes?: RunBatchRemoveNodesMutation;
runSplitEdgeAtExistingNode?: RunSplitEdgeAtExistingNodeMutation;
setAssetBrowserTargetNodeId?: Dispatch<SetStateAction<string | null>>;
setNodes?: Dispatch<SetStateAction<RFNode[]>>;
setEdges?: Dispatch<SetStateAction<RFEdge[]>>;
deletingNodeIds?: MutableRefObject<Set<string>>;
canvasId: DynamicValue<Id<"canvases">>;
isSyncOnline: DynamicValue<boolean>;
getEnqueueSyncMutation: () => QueueSyncMutation;
getRunMoveNodeMutation?: () => RunMoveNodeMutation | undefined;
getRunBatchRemoveNodes?: () => RunBatchRemoveNodesMutation | undefined;
getRunSplitEdgeAtExistingNode?: () => RunSplitEdgeAtExistingNodeMutation | undefined;
getSetAssetBrowserTargetNodeId?: () =>
| Dispatch<SetStateAction<string | null>>
| undefined;
getSetNodes?: () => Dispatch<SetStateAction<RFNode[]>> | undefined;
getSetEdges?: () => Dispatch<SetStateAction<RFEdge[]>> | undefined;
getDeletingNodeIds?: () => MutableRefObject<Set<string>> | undefined;
};
type UseCanvasSyncEngineParams = {
@@ -165,17 +173,17 @@ function summarizeResizePayload(payload: unknown): Record<string, unknown> {
export function createCanvasSyncEngineController({
canvasId,
isSyncOnline,
enqueueSyncMutation,
runMoveNodeMutation,
runBatchRemoveNodes,
runSplitEdgeAtExistingNode,
setAssetBrowserTargetNodeId,
setNodes,
setEdges,
deletingNodeIds,
getEnqueueSyncMutation,
getRunMoveNodeMutation,
getRunBatchRemoveNodes,
getRunSplitEdgeAtExistingNode,
getSetAssetBrowserTargetNodeId,
getSetNodes,
getSetEdges,
getDeletingNodeIds,
}: CanvasSyncEngineControllerParams) {
const getIsSyncOnline = () =>
typeof isSyncOnline === "function" ? isSyncOnline() : isSyncOnline;
const getCanvasId = () => resolveDynamicValue(canvasId);
const getIsSyncOnline = () => resolveDynamicValue(isSyncOnline);
const pendingMoveAfterCreateRef = {
current: new Map<string, { positionX: number; positionY: number }>(),
@@ -206,7 +214,7 @@ export function createCanvasSyncEngineController({
const pendingResize = pendingResizeAfterCreateRef.current.get(clientRequestId);
if (!pendingResize) return;
pendingResizeAfterCreateRef.current.delete(clientRequestId);
await enqueueSyncMutation("resizeNode", {
await getEnqueueSyncMutation()("resizeNode", {
nodeId: realId,
width: pendingResize.width,
height: pendingResize.height,
@@ -220,7 +228,7 @@ export function createCanvasSyncEngineController({
if (!pendingDataAfterCreateRef.current.has(clientRequestId)) return;
const pendingData = pendingDataAfterCreateRef.current.get(clientRequestId);
pendingDataAfterCreateRef.current.delete(clientRequestId);
await enqueueSyncMutation("updateData", {
await getEnqueueSyncMutation()("updateData", {
nodeId: realId,
data: pendingData,
});
@@ -233,7 +241,7 @@ export function createCanvasSyncEngineController({
}): Promise<void> => {
const rawNodeId = args.nodeId as string;
if (!isOptimisticNodeId(rawNodeId) || !getIsSyncOnline()) {
await enqueueSyncMutation("resizeNode", args);
await getEnqueueSyncMutation()("resizeNode", args);
return;
}
@@ -243,7 +251,7 @@ export function createCanvasSyncEngineController({
: undefined;
if (resolvedRealId) {
await enqueueSyncMutation("resizeNode", {
await getEnqueueSyncMutation()("resizeNode", {
nodeId: resolvedRealId,
width: args.width,
height: args.height,
@@ -265,7 +273,7 @@ export function createCanvasSyncEngineController({
}): Promise<void> => {
const rawNodeId = args.nodeId as string;
if (!isOptimisticNodeId(rawNodeId) || !getIsSyncOnline()) {
await enqueueSyncMutation("updateData", args);
await getEnqueueSyncMutation()("updateData", args);
return;
}
@@ -275,7 +283,7 @@ export function createCanvasSyncEngineController({
: undefined;
if (resolvedRealId) {
await enqueueSyncMutation("updateData", {
await getEnqueueSyncMutation()("updateData", {
nodeId: resolvedRealId,
data: args.data,
});
@@ -308,6 +316,9 @@ export function createCanvasSyncEngineController({
resolvedRealIdByClientRequestRef.current.delete(clientRequestId);
const realNodeId = realId as string;
const deletingNodeIds = getDeletingNodeIds?.();
const setNodes = getSetNodes?.();
const setEdges = getSetEdges?.();
deletingNodeIds?.current.add(realNodeId);
setNodes?.((current) => current.filter((node) => node.id !== realNodeId));
setEdges?.((current) =>
@@ -315,13 +326,15 @@ export function createCanvasSyncEngineController({
(edge) => edge.source !== realNodeId && edge.target !== realNodeId,
),
);
if (runBatchRemoveNodes) {
await runBatchRemoveNodes({ nodeIds: [realId] });
const batchRemoveNodes = getRunBatchRemoveNodes?.();
if (batchRemoveNodes) {
await batchRemoveNodes({ nodeIds: [realId] });
}
return;
}
const optimisticNodeId = `${OPTIMISTIC_NODE_PREFIX}${clientRequestId}`;
const setAssetBrowserTargetNodeId = getSetAssetBrowserTargetNodeId?.();
setAssetBrowserTargetNodeId?.((current) =>
current === optimisticNodeId ? (realId as string) : current,
);
@@ -336,9 +349,10 @@ export function createCanvasSyncEngineController({
pendingMoveAfterCreateRef.current.delete(clientRequestId);
}
resolvedRealIdByClientRequestRef.current.delete(clientRequestId);
if (runSplitEdgeAtExistingNode) {
await runSplitEdgeAtExistingNode({
canvasId,
const splitEdgeAtExistingNode = getRunSplitEdgeAtExistingNode?.();
if (splitEdgeAtExistingNode) {
await splitEdgeAtExistingNode({
canvasId: getCanvasId(),
splitEdgeId: splitPayload.intersectedEdgeId,
middleNodeId: realId,
splitSourceHandle: splitPayload.intersectedSourceHandle,
@@ -361,14 +375,15 @@ export function createCanvasSyncEngineController({
x: pendingMove.positionX,
y: pendingMove.positionY,
});
if (runMoveNodeMutation) {
await runMoveNodeMutation({
const moveNodeMutation = getRunMoveNodeMutation?.();
if (moveNodeMutation) {
await moveNodeMutation({
nodeId: realId,
positionX: pendingMove.positionX,
positionY: pendingMove.positionY,
});
} else {
await enqueueSyncMutation("moveNode", {
await getEnqueueSyncMutation()("moveNode", {
nodeId: realId,
positionX: pendingMove.positionX,
positionY: pendingMove.positionY,
@@ -396,9 +411,10 @@ export function createCanvasSyncEngineController({
const splitPayload = pendingEdgeSplitByClientRequestRef.current.get(clientRequestId);
if (splitPayload) {
pendingEdgeSplitByClientRequestRef.current.delete(clientRequestId);
if (runSplitEdgeAtExistingNode) {
await runSplitEdgeAtExistingNode({
canvasId,
const splitEdgeAtExistingNode = getRunSplitEdgeAtExistingNode?.();
if (splitEdgeAtExistingNode) {
await splitEdgeAtExistingNode({
canvasId: getCanvasId(),
splitEdgeId: splitPayload.intersectedEdgeId,
middleNodeId: resolvedRealId,
splitSourceHandle: splitPayload.intersectedSourceHandle,
@@ -417,14 +433,15 @@ export function createCanvasSyncEngineController({
x: pendingMove.positionX,
y: pendingMove.positionY,
});
if (runMoveNodeMutation) {
await runMoveNodeMutation({
const moveNodeMutation = getRunMoveNodeMutation?.();
if (moveNodeMutation) {
await moveNodeMutation({
nodeId: resolvedRealId,
positionX: pendingMove.positionX,
positionY: pendingMove.positionY,
});
} else {
await enqueueSyncMutation("moveNode", {
await getEnqueueSyncMutation()("moveNode", {
nodeId: resolvedRealId,
positionX: pendingMove.positionX,
positionY: pendingMove.positionY,
@@ -477,9 +494,21 @@ export function useCanvasSyncEngine({
const isSyncOnline =
isBrowserOnline === true && connectionState.isWebSocketConnected === true;
const canvasIdRef = useRef(canvasId);
canvasIdRef.current = canvasId;
const isSyncOnlineRef = useRef(isSyncOnline);
isSyncOnlineRef.current = isSyncOnline;
const setNodesRef = useRef(setNodes);
setNodesRef.current = setNodes;
const setEdgesRef = useRef(setEdges);
setEdgesRef.current = setEdges;
const setAssetBrowserTargetNodeIdRef = useRef(setAssetBrowserTargetNodeId);
setAssetBrowserTargetNodeIdRef.current = setAssetBrowserTargetNodeId;
const deletingNodeIdsRef = useRef(deletingNodeIds);
deletingNodeIdsRef.current = deletingNodeIds;
const enqueueSyncMutationRef = useRef<QueueSyncMutation>(async () => undefined);
const runMoveNodeMutationRef = useRef<RunMoveNodeMutation>(async () => undefined);
const runBatchRemoveNodesMutationRef = useRef<RunBatchRemoveNodesMutation>(
async () => {},
);
@@ -514,6 +543,7 @@ export function useCanvasSyncEngine({
},
[canvasId, refreshPendingSyncCount],
);
enqueueSyncMutationRef.current = enqueueSyncMutation;
const runMoveNodeMutation = useCallback<RunMoveNodeMutation>(
async (args) => {
@@ -521,6 +551,7 @@ export function useCanvasSyncEngine({
},
[enqueueSyncMutation],
);
runMoveNodeMutationRef.current = runMoveNodeMutation;
const runBatchMoveNodesMutation = useCallback(
async (args: {
@@ -748,20 +779,22 @@ export function useCanvasSyncEngine({
const controllerRef = useRef<CanvasSyncEngineController | null>(null);
if (controllerRef.current === null) {
controllerRef.current = createCanvasSyncEngineController({
canvasId,
canvasId: () => canvasIdRef.current,
isSyncOnline: () => isSyncOnlineRef.current,
enqueueSyncMutation,
runMoveNodeMutation,
runBatchRemoveNodes: async (args) => {
getEnqueueSyncMutation: () => enqueueSyncMutationRef.current,
getRunMoveNodeMutation: () => runMoveNodeMutationRef.current,
getRunBatchRemoveNodes: () => async (args: { nodeIds: Id<"nodes">[] }) => {
await runBatchRemoveNodesMutationRef.current(args);
},
runSplitEdgeAtExistingNode: async (args) => {
getRunSplitEdgeAtExistingNode: () => async (
args: Parameters<RunSplitEdgeAtExistingNodeMutation>[0],
) => {
await runSplitEdgeAtExistingNodeMutationRef.current(args);
},
setAssetBrowserTargetNodeId,
setNodes,
setEdges,
deletingNodeIds,
getSetAssetBrowserTargetNodeId: () => setAssetBrowserTargetNodeIdRef.current,
getSetNodes: () => setNodesRef.current,
getSetEdges: () => setEdgesRef.current,
getDeletingNodeIds: () => deletingNodeIdsRef.current,
});
}
const controller = controllerRef.current;