From 3a03a1628115a2c0478ec9a4bb6ae4ba8f4aa13a Mon Sep 17 00:00:00 2001 From: handreyrc Date: Fri, 15 May 2026 18:47:45 -0400 Subject: [PATCH 1/6] Add ELKjs dependency Signed-off-by: handreyrc --- .../package.json | 1 + .../src/core/elkjs.ts | 31 +++++++++++++++++++ pnpm-lock.yaml | 11 +++++++ pnpm-workspace.yaml | 1 + 4 files changed, 44 insertions(+) create mode 100644 packages/serverless-workflow-diagram-editor/src/core/elkjs.ts diff --git a/packages/serverless-workflow-diagram-editor/package.json b/packages/serverless-workflow-diagram-editor/package.json index 50ccf70..184a523 100644 --- a/packages/serverless-workflow-diagram-editor/package.json +++ b/packages/serverless-workflow-diagram-editor/package.json @@ -46,6 +46,7 @@ "@xyflow/react": "catalog:", "class-variance-authority": "catalog:", "clsx": "catalog:", + "elkjs": "catalog:", "js-yaml": "catalog:", "radix-ui": "catalog:" }, diff --git a/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts b/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts new file mode 100644 index 0000000..e341369 --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts @@ -0,0 +1,31 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import ELK from "elkjs"; + +const elk = new ELK(); +const graph = { + id: "root", + children: [ + { id: "n1", width: 100, height: 50 }, + { id: "n2", width: 100, height: 50 }, + ], + edges: [{ id: "e1", sources: ["n1"], targets: ["n2"] }], +}; + +elk.layout(graph).then((layoutedGraph) => { + console.log(layoutedGraph); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2312f69..4d3bc1f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -69,6 +69,9 @@ catalogs: clsx: specifier: ^2.1.1 version: 2.1.1 + elkjs: + specifier: ^0.11.1 + version: 0.11.1 husky: specifier: ^9.1.7 version: 9.1.7 @@ -198,6 +201,9 @@ importers: clsx: specifier: 'catalog:' version: 2.1.1 + elkjs: + specifier: 'catalog:' + version: 0.11.1 js-yaml: specifier: 'catalog:' version: 4.1.1 @@ -2570,6 +2576,9 @@ packages: electron-to-chromium@1.5.359: resolution: {integrity: sha512-8lPELWuYZIWk7NDvCNthtmMw/7Q5Wu25NpM4djFMHBmk8DubPAtL4YTOp7ou0e7HyJtwkVlWv8XMLURnrtgJQw==} + elkjs@0.11.1: + resolution: {integrity: sha512-zxxR9k+rx5ktMwT/FwyLdPCrq7xN6e4VGGHH8hA01vVYKjTFik7nHOxBnAYtrgYUB1RpAiLvA1/U2YraWxyKKg==} + emoji-regex@10.6.0: resolution: {integrity: sha512-toUI84YS5YmxW219erniWD0CIVOo46xGKColeNQRgOzDorgBi1v4D71/OFzgD9GO2UGKIv1C3Sp8DAn0+j5w7A==} @@ -5558,6 +5567,8 @@ snapshots: electron-to-chromium@1.5.359: {} + elkjs@0.11.1: {} + emoji-regex@10.6.0: {} empathic@2.0.1: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 7abaf45..0629719 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -22,6 +22,7 @@ catalog: "@xyflow/react": ^12.10.2 class-variance-authority: ^0.7.1 clsx: ^2.1.1 + elkjs: "^0.11.1" husky: ^9.1.7 "js-yaml": ^4.1.1 jsdom: ^29.1.0 From 0544ac6c49dd121c353ba6a7b65eec6b0b19a8ce Mon Sep 17 00:00:00 2001 From: handreyrc Date: Wed, 20 May 2026 16:57:48 -0400 Subject: [PATCH 2/6] ELKjs auto-layout engine integration Signed-off-by: handreyrc # Conflicts: # packages/serverless-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx --- .../src/core/elkjs.ts | 28 +- .../src/core/index.ts | 1 + .../src/diagram-editor/DiagramEditor.tsx | 35 +- .../src/react-flow/diagram/Diagram.tsx | 13 +- .../src/react-flow/diagram/autoLayout.ts | 122 +++- .../src/react-flow/diagram/diagramBuilder.ts | 4 +- .../tests/core/elkjs.test.ts | 249 ++++++++ .../tests/react-flow/diagram/Diagram.test.tsx | 17 +- .../diagram/autoLayout.integration.test.ts | 534 +++++++++++++++++- 9 files changed, 936 insertions(+), 67 deletions(-) create mode 100644 packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts diff --git a/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts b/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts index e341369..a73d1b8 100644 --- a/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts +++ b/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts @@ -14,18 +14,22 @@ * limitations under the License. */ -import ELK from "elkjs"; +import ELK, { ElkNode } from "elkjs/lib/elk.bundled.js"; const elk = new ELK(); -const graph = { - id: "root", - children: [ - { id: "n1", width: 100, height: 50 }, - { id: "n2", width: 100, height: 50 }, - ], - edges: [{ id: "e1", sources: ["n1"], targets: ["n2"] }], -}; -elk.layout(graph).then((layoutedGraph) => { - console.log(layoutedGraph); -}); +export async function processElkLayout(graph: ElkNode): Promise { + try { + // Attempt to layout the graph + return await elk.layout(graph); + } catch (error: unknown) { + // Type-safe error handling + if (error instanceof Error) { + console.error("ELK Layout failed:", error.message); + } else { + console.error("An unexpected error occurred:", String(error)); + } + // Return a fallback, null, or rethrow the error as needed + return null; + } +} diff --git a/packages/serverless-workflow-diagram-editor/src/core/index.ts b/packages/serverless-workflow-diagram-editor/src/core/index.ts index e4b9e03..db02185 100644 --- a/packages/serverless-workflow-diagram-editor/src/core/index.ts +++ b/packages/serverless-workflow-diagram-editor/src/core/index.ts @@ -17,3 +17,4 @@ export * from "./workflowSdk"; export * from "./graph"; export * from "./taskSubType"; +export * from "./elkjs"; diff --git a/packages/serverless-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx b/packages/serverless-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx index cda8b85..b1f5e46 100644 --- a/packages/serverless-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx +++ b/packages/serverless-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx @@ -15,6 +15,7 @@ */ import * as React from "react"; +import { ReactFlowProvider } from "@xyflow/react"; import { Diagram, DiagramRef } from "../react-flow/diagram/Diagram"; import { DiagramEditorContextProvider } from "../store/DiagramEditorContextProvider"; import { I18nProvider, detectLocale, useI18n } from "@serverlessworkflow/i18n"; @@ -105,22 +106,24 @@ export const DiagramEditor = (props: DiagramEditorProps) => { }; return ( - - -
- -
- - - + + + +
+ +
+ + + + ); }} diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx index 7678152..8ee8d08 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx @@ -24,6 +24,7 @@ import { ReactFlowEdgeTypes } from "../edges/Edges"; import { useDiagramEditorContext } from "../../store/DiagramEditorContext"; import { buildDiagramElements } from "./diagramBuilder"; import { SidebarTrigger } from "@/components/ui/sidebar"; +import { applyAutoLayout } from "./autoLayout"; const FIT_VIEW_OPTIONS: RF.FitViewOptions = { maxZoom: 1, @@ -45,6 +46,7 @@ export type DiagramProps = { }; export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { + const reactFlowInstance: RF.ReactFlowInstance = RF.useReactFlow(); const { model, nodes, edges, setNodes, setEdges } = useDiagramEditorContext(); const [minimapVisible, setMinimapVisible] = React.useState(false); @@ -70,10 +72,13 @@ export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { // Rebuild nodes and edges as model changes React.useEffect(() => { - const { nodes, edges } = buildDiagramElements(model); - setNodes(nodes); - setEdges(edges); - }, [model, setNodes, setEdges]); + const graph = buildDiagramElements(model); + applyAutoLayout(graph).then(({ nodes, edges }) => { + setNodes(nodes); + setEdges(edges); + reactFlowInstance.fitView(); + }); + }, [model, reactFlowInstance, setNodes, setEdges]); return (
diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts index 7af17a3..79d04b2 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import type { ElkNode, LayoutOptions, ElkExtendedEdge } from "elkjs/lib/elk.bundled.js"; +import { processElkLayout } from "@/core"; import { ReactFlowGraph } from "./diagramBuilder"; // Defaults @@ -36,19 +38,119 @@ export type Size = { export type WayPoints = Point[]; -export function applyAutoLayout(graph: ReactFlowGraph): ReactFlowGraph { +export const ROOT_LAYOUT_OPTIONS: LayoutOptions = { + "elk.algorithm": "org.eclipse.elk.layered", + "elk.hierarchyHandling": "INCLUDE_CHILDREN", + "elk.direction": "DOWN", + "org.eclipse.elk.layered.layering.strategy": "INTERACTIVE", + "org.eclipse.elk.edgeRouting": "ORTHOGONAL", + "elk.layered.unnecessaryBendpoints": "true", + "org.eclipse.elk.layered.nodePlacement.bk.fixedAlignment": "BALANCED", + "org.eclipse.elk.layered.nodePlacement.bk.edgeStraightening": "IMPROVE_STRAIGHTNESS", + "org.eclipse.elk.layered.cycleBreaking.strategy": "DEPTH_FIRST", + "org.eclipse.elk.insideSelfLoops.activate": "true", + "elk.separateConnectedComponents": "false", + "org.eclipse.elk.layered.nodePlacement.favorStraightEdges": "true", + "org.eclipse.elk.layered.considerModelOrder.strategy": "EDGES", + "org.eclipse.elk.layered.considerModelOrder.crossingCounterNodeInfluence": "0.001", + "elk.layered.crossingMinimization.strategy": "INTERACTIVE", + spacing: "75", + "spacing.componentComponent": "70", + "spacing.nodeNodeBetweenLayers": "80", + "elk.layered.spacing.edgeNodeBetweenLayers": "40", + "org.eclipse.elk.spacing.edgeNode": "24", + "org.eclipse.elk.layered.mergeEdges": "true", +}; + +export function buildElkGraphFromReactFlowGraph(reactFlowGraph: ReactFlowGraph): ElkNode { + // Create a map for easy lookup + const nodeMap = new Map( + reactFlowGraph.nodes.map((node) => [ + node.id, + { + id: node.id, + width: node.measured?.width ?? DEFAULT_NODE_SIZE.width, + height: node.measured?.height ?? DEFAULT_NODE_SIZE.height, + children: [] as ElkNode[], + }, + ]), + ); + + const rootChildren: ElkNode[] = []; + // Nest children based on parentId + reactFlowGraph.nodes.forEach((node) => { + const elkNode = nodeMap.get(node.id)!; + if (node.parentId && nodeMap.has(node.parentId)) { + nodeMap.get(node.parentId)!.children.push(elkNode); + } else { + rootChildren.push(elkNode); + } + }); + + // edges + const elkEdges: ElkExtendedEdge[] = reactFlowGraph.edges.map((edge) => ({ + id: edge.id, + sources: [edge.source], + targets: [edge.target], + })); + + return { + id: "root", + layoutOptions: ROOT_LAYOUT_OPTIONS, + children: rootChildren, + edges: elkEdges, + }; +} + +// set +export function matchReactFlowGraphWithElkLayoutedGraph( + graph: ReactFlowGraph, + layoutedGraph: ElkNode, +): ReactFlowGraph { const graphClone = structuredClone(graph); - // TODO: This is just a temporary implementation until the actual auto-layout engine is integrated - let position: Position = { x: 0, y: 0 }; + // Map node positions + const layoutedNodes = graphClone.nodes.map((node) => { + const elkNode = layoutedGraph.children?.find((n) => n.id === node.id); + if (elkNode && elkNode.x !== undefined && elkNode.y !== undefined) { + return { + ...node, + position: { x: elkNode.x, y: elkNode.y }, + ...(elkNode.height && { height: elkNode.height }), + ...(elkNode.width && { width: elkNode.width }), + }; + } + return node; + }); - // TODO: Containment is not supported for now. - graphClone.nodes.forEach((node) => { - node.height = DEFAULT_NODE_SIZE.height; - node.width = DEFAULT_NODE_SIZE.width; - node.position = { ...position }; - position.y = position.y + 100; + // Map edge waypoints (bend points) + const layoutedEdges = graphClone.edges.map((edge) => { + const elkEdge = layoutedGraph.edges?.find((e) => e.id === edge.id); + if (elkEdge && elkEdge.sections) { + // ELK returns sections which contain bend points. We pass these bendpoints to a custom edge. + const bendPoints = elkEdge.sections.flatMap((section) => section.bendPoints || []); + return { + ...edge, + data: { + ...edge.data, + ...(bendPoints.length > 0 && { wayPoints: bendPoints }), + }, + }; + } + return edge; }); - return graphClone; + return { nodes: layoutedNodes, edges: layoutedEdges }; +} + +export async function applyAutoLayout(graph: ReactFlowGraph): Promise { + const elkGraph = buildElkGraphFromReactFlowGraph(graph); + const layoutedGraph = await processElkLayout(elkGraph); + + // it is not possible to calculate auto-layout + if (!layoutedGraph) { + return graph; + } + + return matchReactFlowGraphWithElkLayoutedGraph(graph, layoutedGraph); } diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts index d75e473..2847135 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts @@ -19,7 +19,7 @@ import { buildFlatGraph } from "../../core"; import { BaseNodeData, CATCH_CONTAINER_NODE_TYPE, ReactFlowNodeTypes } from "../nodes/Nodes"; import { BaseEdgeData, EdgeTypes } from "../edges/Edges"; import * as sdk from "@serverlessworkflow/sdk"; -import { applyAutoLayout, DEFAULT_NODE_SIZE } from "./autoLayout"; +import { DEFAULT_NODE_SIZE } from "./autoLayout"; export type ReactFlowGraph = { nodes: RF.Node[]; @@ -141,5 +141,5 @@ export function buildDiagramElements(model: sdk.Specification.Workflow | null): }); } - return applyAutoLayout({ nodes, edges }); + return { nodes, edges }; } diff --git a/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts new file mode 100644 index 0000000..526f0f4 --- /dev/null +++ b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts @@ -0,0 +1,249 @@ +/* + * Copyright 2021-Present The Serverless Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { ElkNode } from "elkjs/lib/elk.bundled.js"; +import { processElkLayout } from "../../src/core/elkjs"; + +// Mock the ELK module with a shared layout mock +vi.mock("elkjs/lib/elk.bundled.js", () => { + const mockLayoutFn = vi.fn(); + return { + default: vi.fn(function (this: unknown) { + return { + layout: mockLayoutFn, + }; + }), + // Export the mock so we can access it in tests + __mockLayout: mockLayoutFn, + }; +}); + +describe("elkjs", () => { + describe("processElkLayout", () => { + let consoleErrorSpy: ReturnType; + let elkMockLayout: ReturnType; + + beforeEach(async () => { + // Get the mock layout function from the mocked module + const ELK = await import("elkjs/lib/elk.bundled.js"); + // Access the mock instance's layout method + const elkInstance = new (ELK.default as unknown as new () => { + layout: ReturnType; + })(); + elkMockLayout = elkInstance.layout; + + // Spy on console.error to verify error logging + consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + vi.clearAllMocks(); + }); + + // Helper function to setup ELK mock with success or error response + function setupElkMock(returnValue: ElkNode | Error | string | null | undefined | object) { + if ( + returnValue instanceof Error || + typeof returnValue === "string" || + returnValue === null || + returnValue === undefined || + (typeof returnValue === "object" && !("id" in returnValue)) + ) { + elkMockLayout.mockRejectedValue(returnValue); + } else { + elkMockLayout.mockResolvedValue(returnValue); + } + + return elkMockLayout; + } + + // Test data factory for simple graphs + function createSimpleGraph(nodeCount: number = 2): ElkNode { + return { + id: "root", + children: Array.from({ length: nodeCount }, (_, i) => ({ + id: `node${i + 1}`, + width: 100, + height: 50, + })), + edges: nodeCount > 1 ? [{ id: "edge1", sources: ["node1"], targets: ["node2"] }] : [], + }; + } + + it("processes valid graph and returns positioned nodes", async () => { + const inputGraph = createSimpleGraph(2); + + const expectedOutput: ElkNode = { + id: "root", + children: [ + { id: "node1", width: 100, height: 50, x: 0, y: 0 }, + { id: "node2", width: 100, height: 50, x: 150, y: 0 }, + ], + edges: [{ id: "edge1", sources: ["node1"], targets: ["node2"] }], + x: 0, + y: 0, + width: 250, + height: 50, + }; + + await setupElkMock(expectedOutput); + + const result = await processElkLayout(inputGraph); + + expect(result).toEqual(expectedOutput); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it("handles empty graph with no children", async () => { + const inputGraph: ElkNode = { + id: "root", + children: [], + edges: [], + }; + + const expectedOutput: ElkNode = { + id: "root", + children: [], + edges: [], + x: 0, + y: 0, + width: 0, + height: 0, + }; + + await setupElkMock(expectedOutput); + + const result = await processElkLayout(inputGraph); + + expect(result).toEqual(expectedOutput); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it("handles graph with single node", async () => { + const inputGraph = createSimpleGraph(1); + + const expectedOutput: ElkNode = { + id: "root", + children: [{ id: "node1", width: 100, height: 50, x: 0, y: 0 }], + edges: [], + x: 0, + y: 0, + width: 100, + height: 50, + }; + + await setupElkMock(expectedOutput); + + const result = await processElkLayout(inputGraph); + + expect(result).toEqual(expectedOutput); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + it("handles complex graph with nested children", async () => { + const inputGraph: ElkNode = { + id: "root", + children: [ + { + id: "parent1", + width: 200, + height: 150, + children: [ + { id: "child1", width: 80, height: 40 }, + { id: "child2", width: 80, height: 40 }, + ], + }, + { id: "node2", width: 100, height: 50 }, + ], + edges: [{ id: "edge1", sources: ["parent1"], targets: ["node2"] }], + }; + + const expectedOutput: ElkNode = { + id: "root", + children: [ + { + id: "parent1", + width: 200, + height: 150, + x: 0, + y: 0, + children: [ + { id: "child1", width: 80, height: 40, x: 10, y: 10 }, + { id: "child2", width: 80, height: 40, x: 10, y: 60 }, + ], + }, + { id: "node2", width: 100, height: 50, x: 250, y: 50 }, + ], + edges: [{ id: "edge1", sources: ["parent1"], targets: ["node2"] }], + x: 0, + y: 0, + width: 350, + height: 150, + }; + + await setupElkMock(expectedOutput); + + const result = await processElkLayout(inputGraph); + + expect(result).toEqual(expectedOutput); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + + // Parameterized error handling tests + it.each([ + { + description: "Error instance", + error: new Error("Invalid graph structure"), + expectedLog: ["ELK Layout failed:", "Invalid graph structure"], + }, + { + description: "string value", + error: "String error message", + expectedLog: ["An unexpected error occurred:", "String error message"], + }, + { + description: "undefined", + error: undefined, + expectedLog: ["An unexpected error occurred:", "undefined"], + }, + { + description: "null", + error: null, + expectedLog: ["An unexpected error occurred:", "null"], + }, + { + description: "plain object", + error: { code: 500, message: "Internal error" }, + expectedLog: ["An unexpected error occurred:", "[object Object]"], + }, + ])( + "returns null and logs error when ELK layout throws $description", + async ({ error, expectedLog }) => { + const inputGraph = createSimpleGraph(1); + + await setupElkMock(error); + + const result = await processElkLayout(inputGraph); + + expect(result).toBeNull(); + expect(consoleErrorSpy).toHaveBeenCalledWith(...expectedLog); + expect(consoleErrorSpy).toHaveBeenCalledTimes(1); + }, + ); + }); +}); diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx index 69af289..82c57b4 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx @@ -21,6 +21,7 @@ import { DiagramEditorContextProvider } from "../../../src/store/DiagramEditorCo import { SidebarProvider } from "../../../src/components/ui/sidebar"; import { I18nProvider } from "@serverlessworkflow/i18n"; import { en } from "../../../src/i18n/locales/en"; +import { ReactFlowProvider } from "@xyflow/react"; describe("Diagram Component", () => { afterEach(() => { @@ -29,13 +30,15 @@ describe("Diagram Component", () => { it("render Diagram component and canvas", () => { render( - - - - - - - , + + + + + + + + + , ); const diagram = screen.getByTestId("diagram-container"); diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts index 3791461..b2d7519 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts @@ -14,27 +14,529 @@ * limitations under the License. */ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { ElkNode } from "elkjs/lib/elk.bundled.js"; +import type { Node, Edge } from "@xyflow/react"; +import { + buildElkGraphFromReactFlowGraph, + matchReactFlowGraphWithElkLayoutedGraph, + applyAutoLayout, + DEFAULT_NODE_SIZE, + ROOT_LAYOUT_OPTIONS, +} from "../../../src/react-flow/diagram/autoLayout"; +import type { ReactFlowGraph } from "../../../src/react-flow/diagram/diagramBuilder"; +import * as core from "../../../src/core"; -import { BASIC_VALID_WORKFLOW_JSON_TASKS } from "../../fixtures/workflows"; -import { applyAutoLayout } from "../../../src/react-flow/diagram/autoLayout"; -import { parseWorkflow } from "../../../src/core"; -import { buildDiagramElements } from "../../../src/react-flow/diagram/diagramBuilder"; +// Mock the processElkLayout function +vi.mock("../../../src/core", () => ({ + processElkLayout: vi.fn(), +})); -describe("applyAutoLayout", () => { - it("apply auto-layout calculated layout to graph elements", () => { - const result = parseWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); +describe("autoLayout", () => { + describe("buildElkGraphFromReactFlowGraph", () => { + it("converts simple ReactFlow graph to ELK graph", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {}, measured: { width: 150, height: 50 } }, + { id: "node2", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, + ] as Node[], + edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], + }; - const reacflowGraph = applyAutoLayout(buildDiagramElements(result.model)); + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); - expect(reacflowGraph.nodes).toHaveLength(7); - expect(reacflowGraph.edges).toHaveLength(6); + expect(elkGraph.id).toBe("root"); + expect(elkGraph.layoutOptions).toEqual(ROOT_LAYOUT_OPTIONS); + expect(elkGraph.children).toHaveLength(2); + expect(elkGraph.children?.[0]).toEqual({ + id: "node1", + width: 150, + height: 50, + children: [], + }); + expect(elkGraph.children?.[1]).toEqual({ + id: "node2", + width: 200, + height: 60, + children: [], + }); + expect(elkGraph.edges).toHaveLength(1); + expect(elkGraph.edges?.[0]).toEqual({ + id: "edge1", + sources: ["node1"], + targets: ["node2"], + }); + }); + + it("uses default node size when measured dimensions are not available", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 0, y: 0 }, data: {} }] as Node[], + edges: [], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + expect(elkGraph.children?.[0]).toEqual({ + id: "node1", + width: DEFAULT_NODE_SIZE.width, + height: DEFAULT_NODE_SIZE.height, + children: [], + }); + }); + + it("handles nested nodes with parentId", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { + id: "parent", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 300, height: 200 }, + }, + { + id: "child1", + position: { x: 10, y: 10 }, + data: {}, + parentId: "parent", + measured: { width: 100, height: 50 }, + }, + { + id: "child2", + position: { x: 10, y: 70 }, + data: {}, + parentId: "parent", + measured: { width: 100, height: 50 }, + }, + ] as Node[], + edges: [], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + expect(elkGraph.children).toHaveLength(1); + expect(elkGraph.children?.[0].id).toBe("parent"); + expect(elkGraph.children?.[0].children).toHaveLength(2); + expect(elkGraph.children?.[0].children?.[0].id).toBe("child1"); + expect(elkGraph.children?.[0].children?.[1].id).toBe("child2"); + }); + + it("handles empty graph", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [], + edges: [], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + expect(elkGraph.id).toBe("root"); + expect(elkGraph.children).toHaveLength(0); + expect(elkGraph.edges).toHaveLength(0); + }); + + it("handles multiple edges between nodes", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {} }, + { id: "node2", position: { x: 0, y: 0 }, data: {} }, + ] as Node[], + edges: [ + { id: "edge1", source: "node1", target: "node2", data: {} }, + { id: "edge2", source: "node2", target: "node1", data: {} }, + ] as Edge[], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + expect(elkGraph.edges).toHaveLength(2); + expect(elkGraph.edges?.[0]).toEqual({ + id: "edge1", + sources: ["node1"], + targets: ["node2"], + }); + expect(elkGraph.edges?.[1]).toEqual({ + id: "edge2", + sources: ["node2"], + targets: ["node1"], + }); + }); + + it("ignores nodes with non-existent parentId", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {}, parentId: "nonexistent" }, + ] as Node[], + edges: [], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + expect(elkGraph.children).toHaveLength(1); + expect(elkGraph.children?.[0].id).toBe("node1"); + }); + }); + + describe("matchReactFlowGraphWithElkLayoutedGraph", () => { + it("updates node positions from ELK layout", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {} }, + { id: "node2", position: { x: 0, y: 0 }, data: {} }, + ] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { id: "node1", x: 50, y: 100, width: 200, height: 60 }, + { id: "node2", x: 50, y: 200, width: 200, height: 60 }, + ], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.nodes[0].position).toEqual({ x: 50, y: 100 }); + expect(result.nodes[0].width).toBe(200); + expect(result.nodes[0].height).toBe(60); + expect(result.nodes[1].position).toEqual({ x: 50, y: 200 }); + }); + + it("preserves original node data when no ELK node found", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 10, y: 20 }, data: { label: "Test" } }] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.nodes[0].position).toEqual({ x: 10, y: 20 }); + expect(result.nodes[0].data).toEqual({ label: "Test" }); + }); + + it("adds waypoints to edges from ELK bend points", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {} }, + { id: "node2", position: { x: 0, y: 0 }, data: {} }, + ] as Node[], + edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { id: "node1", x: 0, y: 0 }, + { id: "node2", x: 200, y: 100 }, + ], + edges: [ + { + id: "edge1", + sources: ["node1"], + targets: ["node2"], + sections: [ + { + id: "section1", + startPoint: { x: 0, y: 0 }, + endPoint: { x: 200, y: 100 }, + bendPoints: [ + { x: 100, y: 0 }, + { x: 100, y: 100 }, + ], + }, + ], + }, + ], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.edges[0].data?.wayPoints).toEqual([ + { x: 100, y: 0 }, + { x: 100, y: 100 }, + ]); + }); + + it("handles edges without bend points", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {} }, + { id: "node2", position: { x: 0, y: 0 }, data: {} }, + ] as Node[], + edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { id: "node1", x: 0, y: 0 }, + { id: "node2", x: 200, y: 0 }, + ], + edges: [ + { + id: "edge1", + sources: ["node1"], + targets: ["node2"], + sections: [ + { + id: "section1", + startPoint: { x: 0, y: 0 }, + endPoint: { x: 200, y: 0 }, + }, + ], + }, + ], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.edges[0].data?.wayPoints).toBeUndefined(); + }); + + it("preserves edge data when no ELK edge found", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [], + edges: [ + { id: "edge1", source: "node1", target: "node2", data: { label: "Test Edge" } }, + ] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.edges[0].data).toEqual({ label: "Test Edge" }); + }); + + it("does not mutate original graph", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 0, y: 0 }, data: {} }] as Node[], + edges: [], + }; + + const originalPosition = { ...reactFlowGraph.nodes[0].position }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [{ id: "node1", x: 100, y: 200 }], + edges: [], + }; + + matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(reactFlowGraph.nodes[0].position).toEqual(originalPosition); + }); + + it("handles multiple sections with bend points", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [], + edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [], + edges: [ + { + id: "edge1", + sources: ["node1"], + targets: ["node2"], + sections: [ + { + id: "section1", + startPoint: { x: 0, y: 0 }, + endPoint: { x: 100, y: 50 }, + bendPoints: [{ x: 50, y: 0 }], + }, + { + id: "section2", + startPoint: { x: 100, y: 50 }, + endPoint: { x: 200, y: 100 }, + bendPoints: [{ x: 150, y: 100 }], + }, + ], + }, + ], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.edges[0].data?.wayPoints).toEqual([ + { x: 50, y: 0 }, + { x: 150, y: 100 }, + ]); + }); + }); + + describe("applyAutoLayout", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("applies auto layout successfully", async () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, + { id: "node2", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, + ] as Node[], + edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { id: "node1", x: 50, y: 100, width: 200, height: 60 }, + { id: "node2", x: 50, y: 200, width: 200, height: 60 }, + ], + edges: [ + { + id: "edge1", + sources: ["node1"], + targets: ["node2"], + }, + ], + }; + + vi.mocked(core.processElkLayout).mockResolvedValue(layoutedElkGraph); + + const result = await applyAutoLayout(reactFlowGraph); + + expect(core.processElkLayout).toHaveBeenCalledTimes(1); + expect(result.nodes[0].position).toEqual({ x: 50, y: 100 }); + expect(result.nodes[1].position).toEqual({ x: 50, y: 200 }); + }); + + it("returns original graph when ELK layout fails", async () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 10, y: 20 }, data: {} }] as Node[], + edges: [], + }; + + vi.mocked(core.processElkLayout).mockResolvedValue(null); + + const result = await applyAutoLayout(reactFlowGraph); + + expect(result).toEqual(reactFlowGraph); + expect(result.nodes[0].position).toEqual({ x: 10, y: 20 }); + }); + + it("handles empty graph", async () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [], + edges: [], + }; + + vi.mocked(core.processElkLayout).mockResolvedValue(layoutedElkGraph); + + const result = await applyAutoLayout(reactFlowGraph); + + expect(result.nodes).toHaveLength(0); + expect(result.edges).toHaveLength(0); + }); + + it("passes correct ELK graph structure to processElkLayout", async () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {}, measured: { width: 150, height: 50 } }, + ] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [{ id: "node1", x: 0, y: 0 }], + edges: [], + }; + + vi.mocked(core.processElkLayout).mockResolvedValue(layoutedElkGraph); + + await applyAutoLayout(reactFlowGraph); + + expect(core.processElkLayout).toHaveBeenCalledWith({ + id: "root", + layoutOptions: ROOT_LAYOUT_OPTIONS, + children: [ + { + id: "node1", + width: 150, + height: 50, + children: [], + }, + ], + edges: [], + }); + }); + + it("handles complex graph with nested nodes and edges", async () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { + id: "parent", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 300, height: 200 }, + }, + { + id: "child1", + position: { x: 10, y: 10 }, + data: {}, + parentId: "parent", + measured: { width: 100, height: 50 }, + }, + { id: "node2", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, + ] as Node[], + edges: [{ id: "edge1", source: "parent", target: "node2", data: {} }] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { + id: "parent", + x: 0, + y: 0, + width: 300, + height: 200, + children: [{ id: "child1", x: 10, y: 10, width: 100, height: 50 }], + }, + { id: "node2", x: 350, y: 50, width: 200, height: 60 }, + ], + edges: [ + { + id: "edge1", + sources: ["parent"], + targets: ["node2"], + }, + ], + }; + + vi.mocked(core.processElkLayout).mockResolvedValue(layoutedElkGraph); + + const result = await applyAutoLayout(reactFlowGraph); - let y = 0; - reacflowGraph.nodes.forEach((node) => { - // TODO coordinates are fixed (y = y + 100) for now - expect(node.position!.y).toBe(y); - y += 100; + expect(result.nodes).toHaveLength(3); + expect(result.nodes[0].position).toEqual({ x: 0, y: 0 }); + expect(result.nodes[2].position).toEqual({ x: 350, y: 50 }); }); }); }); From 85bba139a3a6d77e1920ec2d90ffd747de210a34 Mon Sep 17 00:00:00 2001 From: handreyrc Date: Wed, 20 May 2026 18:51:37 -0400 Subject: [PATCH 3/6] Fix copilot complaints Signed-off-by: handreyrc --- .../src/react-flow/diagram/Diagram.tsx | 16 +- .../src/react-flow/diagram/autoLayout.ts | 30 ++- .../tests/core/elkjs.test.ts | 2 +- .../diagram/autoLayout.integration.test.ts | 227 +++++++++++++++++- .../vitest.config.ts | 5 +- 5 files changed, 258 insertions(+), 22 deletions(-) diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx index 8ee8d08..01334d1 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx @@ -72,12 +72,22 @@ export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { // Rebuild nodes and edges as model changes React.useEffect(() => { + let isActive = true; + const graph = buildDiagramElements(model); applyAutoLayout(graph).then(({ nodes, edges }) => { - setNodes(nodes); - setEdges(edges); - reactFlowInstance.fitView(); + // Only update if this effect is still active (not cancelled by cleanup) + if (isActive) { + setNodes(nodes); + setEdges(edges); + reactFlowInstance.fitView(); + } }); + + // Cleanup function to cancel stale updates + return () => { + isActive = false; + }; }, [model, reactFlowInstance, setNodes, setEdges]); return ( diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts index 79d04b2..a3c0ab7 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts @@ -102,30 +102,46 @@ export function buildElkGraphFromReactFlowGraph(reactFlowGraph: ReactFlowGraph): }; } +// Helper function to recursively build a flat map of all ELK nodes +function buildElkNodeMap( + elkNode: ElkNode, + map: Map = new Map(), +): Map { + map.set(elkNode.id, elkNode); + if (elkNode.children) { + for (const child of elkNode.children) { + buildElkNodeMap(child, map); + } + } + return map; +} + // set export function matchReactFlowGraphWithElkLayoutedGraph( graph: ReactFlowGraph, layoutedGraph: ElkNode, ): ReactFlowGraph { - const graphClone = structuredClone(graph); + // Build flat maps for O(1) lookups + const elkNodeMap = buildElkNodeMap(layoutedGraph); + const elkEdgeMap = new Map(layoutedGraph.edges?.map((e) => [e.id, e]) || []); // Map node positions - const layoutedNodes = graphClone.nodes.map((node) => { - const elkNode = layoutedGraph.children?.find((n) => n.id === node.id); + const layoutedNodes = graph.nodes.map((node) => { + const elkNode = elkNodeMap.get(node.id); if (elkNode && elkNode.x !== undefined && elkNode.y !== undefined) { return { ...node, position: { x: elkNode.x, y: elkNode.y }, - ...(elkNode.height && { height: elkNode.height }), - ...(elkNode.width && { width: elkNode.width }), + ...(elkNode.height !== undefined && { height: elkNode.height }), + ...(elkNode.width !== undefined && { width: elkNode.width }), }; } return node; }); // Map edge waypoints (bend points) - const layoutedEdges = graphClone.edges.map((edge) => { - const elkEdge = layoutedGraph.edges?.find((e) => e.id === edge.id); + const layoutedEdges = graph.edges.map((edge) => { + const elkEdge = elkEdgeMap.get(edge.id); if (elkEdge && elkEdge.sections) { // ELK returns sections which contain bend points. We pass these bendpoints to a custom edge. const bendPoints = elkEdge.sections.flatMap((section) => section.bendPoints || []); diff --git a/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts index 526f0f4..a82c0e2 100644 --- a/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts @@ -15,7 +15,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { ElkNode } from "elkjs/lib/elk.bundled.js"; +import type { ElkNode } from "elkjs/lib/elk.bundled.js"; import { processElkLayout } from "../../src/core/elkjs"; // Mock the ELK module with a shared layout mock diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts index b2d7519..4cd3acd 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts @@ -25,10 +25,10 @@ import { ROOT_LAYOUT_OPTIONS, } from "../../../src/react-flow/diagram/autoLayout"; import type { ReactFlowGraph } from "../../../src/react-flow/diagram/diagramBuilder"; -import * as core from "../../../src/core"; +import * as core from "@/core"; // Mock the processElkLayout function -vi.mock("../../../src/core", () => ({ +vi.mock("@/core", () => ({ processElkLayout: vi.fn(), })); @@ -37,8 +37,18 @@ describe("autoLayout", () => { it("converts simple ReactFlow graph to ELK graph", () => { const reactFlowGraph: ReactFlowGraph = { nodes: [ - { id: "node1", position: { x: 0, y: 0 }, data: {}, measured: { width: 150, height: 50 } }, - { id: "node2", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, + { + id: "node1", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 150, height: 50 }, + }, + { + id: "node2", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 200, height: 60 }, + }, ] as Node[], edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], }; @@ -163,7 +173,12 @@ describe("autoLayout", () => { it("ignores nodes with non-existent parentId", () => { const reactFlowGraph: ReactFlowGraph = { nodes: [ - { id: "node1", position: { x: 0, y: 0 }, data: {}, parentId: "nonexistent" }, + { + id: "node1", + position: { x: 0, y: 0 }, + data: {}, + parentId: "nonexistent", + }, ] as Node[], edges: [], }; @@ -303,7 +318,12 @@ describe("autoLayout", () => { const reactFlowGraph: ReactFlowGraph = { nodes: [], edges: [ - { id: "edge1", source: "node1", target: "node2", data: { label: "Test Edge" } }, + { + id: "edge1", + source: "node1", + target: "node2", + data: { label: "Test Edge" }, + }, ] as Edge[], }; @@ -390,8 +410,18 @@ describe("autoLayout", () => { it("applies auto layout successfully", async () => { const reactFlowGraph: ReactFlowGraph = { nodes: [ - { id: "node1", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, - { id: "node2", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, + { + id: "node1", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 200, height: 60 }, + }, + { + id: "node2", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 200, height: 60 }, + }, ] as Node[], edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], }; @@ -457,7 +487,12 @@ describe("autoLayout", () => { it("passes correct ELK graph structure to processElkLayout", async () => { const reactFlowGraph: ReactFlowGraph = { nodes: [ - { id: "node1", position: { x: 0, y: 0 }, data: {}, measured: { width: 150, height: 50 } }, + { + id: "node1", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 150, height: 50 }, + }, ] as Node[], edges: [], }; @@ -503,7 +538,12 @@ describe("autoLayout", () => { parentId: "parent", measured: { width: 100, height: 50 }, }, - { id: "node2", position: { x: 0, y: 0 }, data: {}, measured: { width: 200, height: 60 } }, + { + id: "node2", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 200, height: 60 }, + }, ] as Node[], edges: [{ id: "edge1", source: "parent", target: "node2", data: {} }] as Edge[], }; @@ -538,5 +578,172 @@ describe("autoLayout", () => { expect(result.nodes[0].position).toEqual({ x: 0, y: 0 }); expect(result.nodes[2].position).toEqual({ x: 350, y: 50 }); }); + + describe("buildElkNodeMap helper", () => { + it("handles nested nodes correctly in matchReactFlowGraphWithElkLayoutedGraph", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "parent", position: { x: 0, y: 0 }, data: {} }, + { + id: "child1", + position: { x: 0, y: 0 }, + data: {}, + parentId: "parent", + }, + { + id: "child2", + position: { x: 0, y: 0 }, + data: {}, + parentId: "parent", + }, + ] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { + id: "parent", + x: 50, + y: 100, + width: 300, + height: 200, + children: [ + { id: "child1", x: 10, y: 10, width: 100, height: 50 }, + { id: "child2", x: 10, y: 70, width: 100, height: 50 }, + ], + }, + ], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + // Parent node should be positioned + expect(result.nodes[0].position).toEqual({ x: 50, y: 100 }); + expect(result.nodes[0].width).toBe(300); + expect(result.nodes[0].height).toBe(200); + + // Nested children should also be positioned (this was the bug) + expect(result.nodes[1].position).toEqual({ x: 10, y: 10 }); + expect(result.nodes[1].width).toBe(100); + expect(result.nodes[1].height).toBe(50); + + expect(result.nodes[2].position).toEqual({ x: 10, y: 70 }); + expect(result.nodes[2].width).toBe(100); + expect(result.nodes[2].height).toBe(50); + }); + + it("handles deeply nested nodes", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "level1", position: { x: 0, y: 0 }, data: {} }, + { + id: "level2", + position: { x: 0, y: 0 }, + data: {}, + parentId: "level1", + }, + { + id: "level3", + position: { x: 0, y: 0 }, + data: {}, + parentId: "level2", + }, + ] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { + id: "level1", + x: 0, + y: 0, + children: [ + { + id: "level2", + x: 10, + y: 10, + children: [{ id: "level3", x: 20, y: 20 }], + }, + ], + }, + ], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.nodes[0].position).toEqual({ x: 0, y: 0 }); + expect(result.nodes[1].position).toEqual({ x: 10, y: 10 }); + expect(result.nodes[2].position).toEqual({ x: 20, y: 20 }); + }); + }); + + describe("dimension handling with !== undefined", () => { + it("correctly handles width and height of 0", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 0, y: 0 }, data: {} }] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [{ id: "node1", x: 50, y: 100, width: 0, height: 0 }], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + // Should include width and height even though they are 0 + expect(result.nodes[0].width).toBe(0); + expect(result.nodes[0].height).toBe(0); + }); + + it("does not add width/height when undefined", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 0, y: 0 }, data: {} }] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [{ id: "node1", x: 50, y: 100 }], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + // Should not have width/height properties + expect(result.nodes[0].width).toBeUndefined(); + expect(result.nodes[0].height).toBeUndefined(); + }); + }); + + describe("DEFAULT_NODE_SIZE", () => { + it("has correct default dimensions", () => { + expect(DEFAULT_NODE_SIZE).toEqual({ + height: 60, + width: 200, + }); + }); + }); + + describe("ROOT_LAYOUT_OPTIONS", () => { + it("contains required ELK layout options", () => { + expect(ROOT_LAYOUT_OPTIONS["elk.algorithm"]).toBe("org.eclipse.elk.layered"); + expect(ROOT_LAYOUT_OPTIONS["elk.direction"]).toBe("DOWN"); + expect(ROOT_LAYOUT_OPTIONS["elk.hierarchyHandling"]).toBe("INCLUDE_CHILDREN"); + }); + + it("has proper spacing configuration", () => { + expect(ROOT_LAYOUT_OPTIONS["spacing"]).toBe("75"); + expect(ROOT_LAYOUT_OPTIONS["spacing.componentComponent"]).toBe("70"); + expect(ROOT_LAYOUT_OPTIONS["spacing.nodeNodeBetweenLayers"]).toBe("80"); + }); + }); }); }); diff --git a/packages/serverless-workflow-diagram-editor/vitest.config.ts b/packages/serverless-workflow-diagram-editor/vitest.config.ts index be65758..56659b3 100644 --- a/packages/serverless-workflow-diagram-editor/vitest.config.ts +++ b/packages/serverless-workflow-diagram-editor/vitest.config.ts @@ -15,10 +15,13 @@ */ import { defineConfig } from "vitest/config"; +import path from "path"; export default defineConfig({ resolve: { - tsconfigPaths: true, + alias: { + "@": path.resolve(__dirname, "./src"), + }, }, test: { globals: true, From b8f28ba14cd985bb49530cf891841717beb72383 Mon Sep 17 00:00:00 2001 From: handreyrc Date: Thu, 21 May 2026 09:55:53 -0400 Subject: [PATCH 4/6] Fix review comments Signed-off-by: handreyrc --- .../src/react-flow/diagram/Diagram.tsx | 4 +- .../tests/core/elkjs.test.ts | 73 ++++++++++--------- .../tests/react-flow/diagram/Diagram.test.tsx | 22 +++++- 3 files changed, 60 insertions(+), 39 deletions(-) diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx index 01334d1..aaa73d7 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx @@ -80,7 +80,9 @@ export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { if (isActive) { setNodes(nodes); setEdges(edges); - reactFlowInstance.fitView(); + + // Queue fitView to run after React updates the DOM + setTimeout(() => reactFlowInstance.fitView(), 0); } }); diff --git a/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts index a82c0e2..35bc83a 100644 --- a/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts @@ -32,6 +32,39 @@ vi.mock("elkjs/lib/elk.bundled.js", () => { }; }); +// Helper function to setup ELK mock with success or error response +function setupElkMock( + elkMockLayout: ReturnType, + returnValue: ElkNode | Error | string | null | undefined | object, +) { + if ( + returnValue instanceof Error || + typeof returnValue === "string" || + returnValue === null || + returnValue === undefined || + (typeof returnValue === "object" && !("id" in returnValue)) + ) { + elkMockLayout.mockRejectedValue(returnValue); + } else { + elkMockLayout.mockResolvedValue(returnValue); + } + + return elkMockLayout; +} + +// Test data factory for simple graphs +function createSimpleGraph(nodeCount: number = 2): ElkNode { + return { + id: "root", + children: Array.from({ length: nodeCount }, (_, i) => ({ + id: `node${i + 1}`, + width: 100, + height: 50, + })), + edges: nodeCount > 1 ? [{ id: "edge1", sources: ["node1"], targets: ["node2"] }] : [], + }; +} + describe("elkjs", () => { describe("processElkLayout", () => { let consoleErrorSpy: ReturnType; @@ -55,36 +88,6 @@ describe("elkjs", () => { vi.clearAllMocks(); }); - // Helper function to setup ELK mock with success or error response - function setupElkMock(returnValue: ElkNode | Error | string | null | undefined | object) { - if ( - returnValue instanceof Error || - typeof returnValue === "string" || - returnValue === null || - returnValue === undefined || - (typeof returnValue === "object" && !("id" in returnValue)) - ) { - elkMockLayout.mockRejectedValue(returnValue); - } else { - elkMockLayout.mockResolvedValue(returnValue); - } - - return elkMockLayout; - } - - // Test data factory for simple graphs - function createSimpleGraph(nodeCount: number = 2): ElkNode { - return { - id: "root", - children: Array.from({ length: nodeCount }, (_, i) => ({ - id: `node${i + 1}`, - width: 100, - height: 50, - })), - edges: nodeCount > 1 ? [{ id: "edge1", sources: ["node1"], targets: ["node2"] }] : [], - }; - } - it("processes valid graph and returns positioned nodes", async () => { const inputGraph = createSimpleGraph(2); @@ -101,7 +104,7 @@ describe("elkjs", () => { height: 50, }; - await setupElkMock(expectedOutput); + await setupElkMock(elkMockLayout, expectedOutput); const result = await processElkLayout(inputGraph); @@ -126,7 +129,7 @@ describe("elkjs", () => { height: 0, }; - await setupElkMock(expectedOutput); + await setupElkMock(elkMockLayout, expectedOutput); const result = await processElkLayout(inputGraph); @@ -147,7 +150,7 @@ describe("elkjs", () => { height: 50, }; - await setupElkMock(expectedOutput); + await setupElkMock(elkMockLayout, expectedOutput); const result = await processElkLayout(inputGraph); @@ -196,7 +199,7 @@ describe("elkjs", () => { height: 150, }; - await setupElkMock(expectedOutput); + await setupElkMock(elkMockLayout, expectedOutput); const result = await processElkLayout(inputGraph); @@ -236,7 +239,7 @@ describe("elkjs", () => { async ({ error, expectedLog }) => { const inputGraph = createSimpleGraph(1); - await setupElkMock(error); + await setupElkMock(elkMockLayout, error); const result = await processElkLayout(inputGraph); diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx index 82c57b4..bf1db18 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx @@ -14,21 +14,32 @@ * limitations under the License. */ -import { render, screen } from "@testing-library/react"; -import { vi, it, expect, afterEach, describe } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import { vi, it, expect, afterEach, describe, beforeEach } from "vitest"; import { Diagram } from "../../../src/react-flow/diagram/Diagram"; import { DiagramEditorContextProvider } from "../../../src/store/DiagramEditorContextProvider"; import { SidebarProvider } from "../../../src/components/ui/sidebar"; import { I18nProvider } from "@serverlessworkflow/i18n"; import { en } from "../../../src/i18n/locales/en"; import { ReactFlowProvider } from "@xyflow/react"; +import * as autoLayoutModule from "../../../src/react-flow/diagram/autoLayout"; describe("Diagram Component", () => { + let applyAutoLayoutSpy: ReturnType; + + beforeEach(() => { + // Mock applyAutoLayout to return a resolved promise with empty nodes and edges + applyAutoLayoutSpy = vi.spyOn(autoLayoutModule, "applyAutoLayout").mockResolvedValue({ + nodes: [], + edges: [], + }); + }); + afterEach(() => { vi.restoreAllMocks(); }); - it("render Diagram component and canvas", () => { + it("render Diagram component and canvas", async () => { render( @@ -46,5 +57,10 @@ describe("Diagram Component", () => { expect(diagram).toBeInTheDocument(); expect(canvas).toBeInTheDocument(); + + // Verify that applyAutoLayout was called + await waitFor(() => { + expect(applyAutoLayoutSpy).toHaveBeenCalled(); + }); }); }); From 96b2245ff7b0a7888e9f3e2f52bdd136ba619a73 Mon Sep 17 00:00:00 2001 From: handreyrc Date: Thu, 21 May 2026 10:35:54 -0400 Subject: [PATCH 5/6] Fix copilot complaints Signed-off-by: handreyrc --- .../src/react-flow/diagram/Diagram.tsx | 29 ++++++++++++------- .../src/react-flow/diagram/autoLayout.ts | 4 ++- .../tests/core/elkjs.test.ts | 6 +++- pnpm-workspace.yaml | 2 +- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx index aaa73d7..a2cd817 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx @@ -73,22 +73,31 @@ export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { // Rebuild nodes and edges as model changes React.useEffect(() => { let isActive = true; + let timeoutId: ReturnType | null = null; const graph = buildDiagramElements(model); - applyAutoLayout(graph).then(({ nodes, edges }) => { - // Only update if this effect is still active (not cancelled by cleanup) - if (isActive) { - setNodes(nodes); - setEdges(edges); + applyAutoLayout(graph) + .then(({ nodes, edges }) => { + // Only update if this effect is still active (not cancelled by cleanup) + if (isActive) { + setNodes(nodes); + setEdges(edges); - // Queue fitView to run after React updates the DOM - setTimeout(() => reactFlowInstance.fitView(), 0); - } - }); + // Queue fitView to run after React updates the DOM + timeoutId = setTimeout(() => reactFlowInstance.fitView(), 0); + } + }) + .catch((error) => { + // Handle auto-layout errors to prevent unhandled promise rejections + console.error("Failed to apply auto-layout:", error); + }); - // Cleanup function to cancel stale updates + // Cleanup function to cancel stale updates and clear timeout return () => { isActive = false; + if (timeoutId !== null) { + clearTimeout(timeoutId); + } }; }, [model, reactFlowInstance, setNodes, setEdges]); diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts index a3c0ab7..b3dd590 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts @@ -145,10 +145,12 @@ export function matchReactFlowGraphWithElkLayoutedGraph( if (elkEdge && elkEdge.sections) { // ELK returns sections which contain bend points. We pass these bendpoints to a custom edge. const bendPoints = elkEdge.sections.flatMap((section) => section.bendPoints || []); + // Reconstruct data without old wayPoints to avoid stale routing + const { wayPoints: _oldWayPoints, ...restData } = edge.data || {}; return { ...edge, data: { - ...edge.data, + ...restData, ...(bendPoints.length > 0 && { wayPoints: bendPoints }), }, }; diff --git a/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts index 35bc83a..11c32c0 100644 --- a/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/core/elkjs.test.ts @@ -16,7 +16,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import type { ElkNode } from "elkjs/lib/elk.bundled.js"; -import { processElkLayout } from "../../src/core/elkjs"; // Mock the ELK module with a shared layout mock vi.mock("elkjs/lib/elk.bundled.js", () => { @@ -69,8 +68,13 @@ describe("elkjs", () => { describe("processElkLayout", () => { let consoleErrorSpy: ReturnType; let elkMockLayout: ReturnType; + let processElkLayout: (graph: ElkNode) => Promise; beforeEach(async () => { + // Dynamically import processElkLayout after the mock is declared + const elkjsModule = await import("../../src/core/elkjs"); + processElkLayout = elkjsModule.processElkLayout; + // Get the mock layout function from the mocked module const ELK = await import("elkjs/lib/elk.bundled.js"); // Access the mock instance's layout method diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 0629719..58ca1e2 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -22,7 +22,7 @@ catalog: "@xyflow/react": ^12.10.2 class-variance-authority: ^0.7.1 clsx: ^2.1.1 - elkjs: "^0.11.1" + elkjs: ^0.11.1 husky: ^9.1.7 "js-yaml": ^4.1.1 jsdom: ^29.1.0 From b5fac892ea694b3c693c0cf1fd1e68110ef28084 Mon Sep 17 00:00:00 2001 From: handreyrc Date: Thu, 21 May 2026 18:18:44 -0400 Subject: [PATCH 6/6] Fix copilot complaints Signed-off-by: handreyrc --- .../src/core/elkjs.ts | 35 +++++- .../src/react-flow/diagram/Diagram.tsx | 70 ++++++++---- .../src/react-flow/diagram/autoLayout.ts | 14 ++- .../diagram/autoLayout.integration.test.ts | 104 +++++++++++++++--- .../vitest.config.ts | 5 +- 5 files changed, 175 insertions(+), 53 deletions(-) diff --git a/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts b/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts index a73d1b8..4a9982f 100644 --- a/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts +++ b/packages/serverless-workflow-diagram-editor/src/core/elkjs.ts @@ -18,12 +18,39 @@ import ELK, { ElkNode } from "elkjs/lib/elk.bundled.js"; const elk = new ELK(); -export async function processElkLayout(graph: ElkNode): Promise { +export async function processElkLayout( + graph: ElkNode, + signal?: AbortSignal, +): Promise { try { - // Attempt to layout the graph - return await elk.layout(graph); + // Check if already aborted before starting + if (signal?.aborted) { + throw new DOMException("Layout operation aborted", "AbortError"); + } + + // Create a promise that rejects when the signal is aborted + const abortPromise = new Promise((_, reject) => { + if (signal) { + signal.addEventListener("abort", () => { + reject(new DOMException("Layout operation aborted", "AbortError")); + }); + } + }); + + // Race between layout calculation and abort signal + const layoutPromise = elk.layout(graph); + + // If signal is provided, race the promises; otherwise just await layout + const result = signal ? await Promise.race([layoutPromise, abortPromise]) : await layoutPromise; + + return result; } catch (error: unknown) { - // Type-safe error handling + // Re-throw abort errors so they can be handled appropriately + if (error instanceof DOMException && error.name === "AbortError") { + throw error; + } + + // Type-safe error handling for other errors if (error instanceof Error) { console.error("ELK Layout failed:", error.message); } else { diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx index a2cd817..6cb41c3 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx @@ -70,33 +70,57 @@ export const Diagram = ({ divRef, ref, colorMode = "light" }: DiagramProps) => { [setEdges], ); - // Rebuild nodes and edges as model changes + // Rebuild nodes and edges as model changes with debouncing React.useEffect(() => { let isActive = true; - let timeoutId: ReturnType | null = null; - - const graph = buildDiagramElements(model); - applyAutoLayout(graph) - .then(({ nodes, edges }) => { - // Only update if this effect is still active (not cancelled by cleanup) - if (isActive) { - setNodes(nodes); - setEdges(edges); - - // Queue fitView to run after React updates the DOM - timeoutId = setTimeout(() => reactFlowInstance.fitView(), 0); - } - }) - .catch((error) => { - // Handle auto-layout errors to prevent unhandled promise rejections - console.error("Failed to apply auto-layout:", error); - }); - - // Cleanup function to cancel stale updates and clear timeout + let debounceTimeoutId: ReturnType | null = null; + let fitViewTimeoutId: ReturnType | null = null; + let abortController: AbortController | null = null; + + // Debounce layout calculation to avoid excessive CPU usage on rapid changes + debounceTimeoutId = setTimeout(() => { + // Create abort controller for this layout operation + abortController = new AbortController(); + + const graph = buildDiagramElements(model); + applyAutoLayout(graph, abortController.signal) + .then(({ nodes, edges }) => { + // Only update if this effect is still active (not cancelled by cleanup) + if (isActive && !abortController?.signal.aborted) { + setNodes(nodes); + setEdges(edges); + + // Queue fitView to run after React updates the DOM + fitViewTimeoutId = setTimeout(() => reactFlowInstance.fitView(), 0); + } + }) + .catch((error) => { + // Ignore abort errors as they are expected when cancelling + if (error.name === "AbortError") { + return; + } + // Handle other auto-layout errors to prevent unhandled promise rejections + console.error("Failed to apply auto-layout:", error); + }); + }, 100); // 150ms debounce delay + + // Cleanup function to cancel stale updates and clear timeouts return () => { isActive = false; - if (timeoutId !== null) { - clearTimeout(timeoutId); + + // Cancel debounce timer + if (debounceTimeoutId !== null) { + clearTimeout(debounceTimeoutId); + } + + // Cancel fitView timer + if (fitViewTimeoutId !== null) { + clearTimeout(fitViewTimeoutId); + } + + // Abort in-flight layout calculation + if (abortController) { + abortController.abort(); } }; }, [model, reactFlowInstance, setNodes, setEdges]); diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts index b3dd590..ec01f12 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts @@ -142,11 +142,10 @@ export function matchReactFlowGraphWithElkLayoutedGraph( // Map edge waypoints (bend points) const layoutedEdges = graph.edges.map((edge) => { const elkEdge = elkEdgeMap.get(edge.id); - if (elkEdge && elkEdge.sections) { - // ELK returns sections which contain bend points. We pass these bendpoints to a custom edge. - const bendPoints = elkEdge.sections.flatMap((section) => section.bendPoints || []); - // Reconstruct data without old wayPoints to avoid stale routing + if (elkEdge) { + // Reconstruct data without old wayPoints to avoid stale routing whenever ELK produced this edge. const { wayPoints: _oldWayPoints, ...restData } = edge.data || {}; + const bendPoints = elkEdge.sections?.flatMap((section) => section.bendPoints || []) || []; return { ...edge, data: { @@ -161,9 +160,12 @@ export function matchReactFlowGraphWithElkLayoutedGraph( return { nodes: layoutedNodes, edges: layoutedEdges }; } -export async function applyAutoLayout(graph: ReactFlowGraph): Promise { +export async function applyAutoLayout( + graph: ReactFlowGraph, + signal?: AbortSignal, +): Promise { const elkGraph = buildElkGraphFromReactFlowGraph(graph); - const layoutedGraph = await processElkLayout(elkGraph); + const layoutedGraph = await processElkLayout(elkGraph, signal); // it is not possible to calculate auto-layout if (!layoutedGraph) { diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts index 4cd3acd..a00b4db 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts @@ -25,10 +25,10 @@ import { ROOT_LAYOUT_OPTIONS, } from "../../../src/react-flow/diagram/autoLayout"; import type { ReactFlowGraph } from "../../../src/react-flow/diagram/diagramBuilder"; -import * as core from "@/core"; +import * as core from "../../../src/core"; // Mock the processElkLayout function -vi.mock("@/core", () => ({ +vi.mock("../../../src/core", () => ({ processElkLayout: vi.fn(), })); @@ -170,7 +170,7 @@ describe("autoLayout", () => { }); }); - it("ignores nodes with non-existent parentId", () => { + it("treat as a root-level if parentId is non-existent", () => { const reactFlowGraph: ReactFlowGraph = { nodes: [ { @@ -314,6 +314,75 @@ describe("autoLayout", () => { expect(result.edges[0].data?.wayPoints).toBeUndefined(); }); + it("clears stale wayPoints when ELK edge has no sections", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [], + edges: [ + { + id: "edge1", + source: "node1", + target: "node2", + data: { label: "Test Edge", wayPoints: [{ x: 10, y: 20 }] }, + }, + ] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [], + edges: [ + { + id: "edge1", + sources: ["node1"], + targets: ["node2"], + }, + ], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.edges[0].data).toEqual({ label: "Test Edge" }); + expect(result.edges[0].data?.wayPoints).toBeUndefined(); + }); + + it("clears stale wayPoints when ELK edge sections have no bend points", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [], + edges: [ + { + id: "edge1", + source: "node1", + target: "node2", + data: { label: "Test Edge", wayPoints: [{ x: 10, y: 20 }] }, + }, + ] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [], + edges: [ + { + id: "edge1", + sources: ["node1"], + targets: ["node2"], + sections: [ + { + id: "section1", + startPoint: { x: 0, y: 0 }, + endPoint: { x: 200, y: 0 }, + }, + ], + }, + ], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.edges[0].data).toEqual({ label: "Test Edge" }); + expect(result.edges[0].data?.wayPoints).toBeUndefined(); + }); + it("preserves edge data when no ELK edge found", () => { const reactFlowGraph: ReactFlowGraph = { nodes: [], @@ -507,19 +576,22 @@ describe("autoLayout", () => { await applyAutoLayout(reactFlowGraph); - expect(core.processElkLayout).toHaveBeenCalledWith({ - id: "root", - layoutOptions: ROOT_LAYOUT_OPTIONS, - children: [ - { - id: "node1", - width: 150, - height: 50, - children: [], - }, - ], - edges: [], - }); + expect(core.processElkLayout).toHaveBeenCalledWith( + { + id: "root", + layoutOptions: ROOT_LAYOUT_OPTIONS, + children: [ + { + id: "node1", + width: 150, + height: 50, + children: [], + }, + ], + edges: [], + }, + undefined, + ); }); it("handles complex graph with nested nodes and edges", async () => { diff --git a/packages/serverless-workflow-diagram-editor/vitest.config.ts b/packages/serverless-workflow-diagram-editor/vitest.config.ts index 56659b3..be65758 100644 --- a/packages/serverless-workflow-diagram-editor/vitest.config.ts +++ b/packages/serverless-workflow-diagram-editor/vitest.config.ts @@ -15,13 +15,10 @@ */ import { defineConfig } from "vitest/config"; -import path from "path"; export default defineConfig({ resolve: { - alias: { - "@": path.resolve(__dirname, "./src"), - }, + tsconfigPaths: true, }, test: { globals: true,