Build minimal React Flow diagram from model#142
Conversation
There was a problem hiding this comment.
Pull request overview
This PR completes the model → SDK graph → React Flow nodes/edges pipeline by switching to the SDK FlatGraph, adding graph normalization utilities, and introducing a diagram builder that produces React Flow elements (with a temporary auto-layout) for rendering workflows end-to-end.
Changes:
- Upgraded
@serverlessworkflow/sdkto1.0.3-alpha1and migrated graph usage fromGraphtoFlatGraph. - Added
diagramBuilderto map the workflow model/graph into React FlowNode[]andEdge[], and updatedDiagramto rebuild elements from the parsed model. - Updated editor store context to carry
nodes/edgesplus setters, and adjusted unit/e2e tests + fixtures for new DSL version and rendering behavior.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-workspace.yaml | Bumps SDK dependency to 1.0.3-alpha1. |
| pnpm-lock.yaml | Locks updated SDK version and engine constraints. |
| packages/serverless-workflow-diagram-editor/tests/test-utils/render-helpers.tsx | Updates mock context to include nodes/edges and new setter names. |
| packages/serverless-workflow-diagram-editor/tests/react-flow/nodes/Nodes.test.tsx | Renames node types export and expands coverage to Start/End nodes. |
| packages/serverless-workflow-diagram-editor/tests/react-flow/edges/Edges.test.tsx | Updates edge type exports/tests to new EdgeTypes + ReactFlowEdgeTypes. |
| packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx | Wraps Diagram with context provider (now required). |
| packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts | Updates integration test inputs to go through diagram builder. |
| packages/serverless-workflow-diagram-editor/tests/fixtures/workflows.ts | Updates DSL version strings to 1.0.3. |
| packages/serverless-workflow-diagram-editor/tests/core/workflowSdk.integration.test.ts | Migrates integration tests to buildFlatGraph and updated DSL. |
| packages/serverless-workflow-diagram-editor/tests/core/graph.test.ts | Replaces extended-graph edge typing tests with flat-graph utilities tests. |
| packages/serverless-workflow-diagram-editor/tests/core/snapshots/workflowSdk.integration.test.ts.snap | Updates snapshots for FlatGraph output and SDK model shape. |
| packages/serverless-workflow-diagram-editor/tests-e2e/diagram-editor.spec.ts | Aligns e2e expectations with model-derived node IDs/counts. |
| packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx | Adds nodes/edges to store and exposes state setters directly. |
| packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContext.tsx | Updates context type + adds useDiagramEditorContext hook contract. |
| packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx | Renames nodeTypes export, adds Start/End node components, and threads task typing into node data. |
| packages/serverless-workflow-diagram-editor/src/react-flow/edges/Edges.tsx | Introduces local EdgeTypes enum and ReactFlowEdgeTypes mapping. |
| packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts | New builder that converts FlatGraph into React Flow nodes/edges (plus temporary layout). |
| packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx | Removes hardcoded elements; rebuilds from model via buildDiagramElements and store state. |
| packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts | Moves/rewires auto-layout to operate on React Flow elements. |
| packages/serverless-workflow-diagram-editor/src/core/workflowSdk.ts | Switches to buildFlatGraph and applies connection-fixing normalization. |
| packages/serverless-workflow-diagram-editor/src/core/index.ts | Removes re-export of old core auto-layout. |
| packages/serverless-workflow-diagram-editor/src/core/graph.ts | Replaces extended-graph edge typing with flat-graph node helpers + connection fixing. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)
packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts:45
- The TODO comment has an accidental trailing ".DEFAULT_NODE_SIZE" which looks like leftover text and makes the comment misleading. Please clean up the comment so it reads correctly.
packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts:31 - This test applies
applyAutoLayoutto the result ofbuildDiagramElements, butbuildDiagramElementsalready callsapplyAutoLayoutinternally. That double-application makes the test less meaningful and could mask regressions (e.g., ifbuildDiagramElementsstops applying layout). Consider either testingapplyAutoLayoutwith a raw{nodes, edges}input, or removing the extraapplyAutoLayoutcall and asserting thatbuildDiagramElementsreturns laid-out elements.
packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts:31 - Minor typo: variable name
reacflowGraphis missing the 't' (likely meantreactflowGraph). Renaming improves readability and consistency with the React Flow terminology used elsewhere.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
960bc00 to
bcb3035
Compare
|
@handreyrc Thanks for the PR, pulled locally and looks really nice in drag-drop section. |
bcb3035 to
1e1e068
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts:46
- The TODO comment has an obvious typo/stray text ("for now.DEFAULT_NODE_SIZE"), which makes the intent unclear. Please correct the comment so it reads cleanly (e.g., end the sentence after "for now").
packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts:31 - This test applies auto-layout twice (
applyAutoLayout(buildDiagramElements(...))), sincebuildDiagramElementsalready callsapplyAutoLayoutinternally. This makes the test less meaningful and a bit confusing; consider either (a) asserting onbuildDiagramElementsoutput directly, or (b) building a graph without layout and then applyingapplyAutoLayoutonce. Also, the variable namereacflowGraphlooks like a typo (missing 't').
1e1e068 to
b180eca
Compare
b180eca to
a4e2861
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts:46
- The TODO comment has an accidental suffix
DEFAULT_NODE_SIZE("for now.DEFAULT_NODE_SIZE"), which looks like a copy/paste artifact and makes the comment harder to read. Please remove/fix the stray text.
packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts:31 - Minor naming typo:
reacflowGraphis likely intended to bereactflowGraph. Renaming improves readability and avoids propagating the misspelling to other tests.
a4e2861 to
e79ee11
Compare
e79ee11 to
40a10d7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts:46
- Typo in the TODO comment:
for now.DEFAULT_NODE_SIZEreads like two sentences were concatenated. Please fix the comment text to avoid confusion when scanning the file.
packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts:31 - Variable name typo:
reacflowGraphshould bereactflowGraph(or similar). Keeping naming consistent makes the test easier to read and search.
40a10d7 to
470ed90
Compare
|
If you have no futher considerations this PR is good enough to be merged! Thanks |
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
470ed90 to
8746925
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts:17
autoLayout.tsimportsReactFlowGraphas a runtime value, but it’s only used for typing. In the Vite/esbuild pipeline this import will be emitted, creating a real circular dependency (autoLayout -> diagramBuilder -> autoLayout) that can lead to partially-initialized modules. Switch this to a type-only import (import type { ReactFlowGraph } ...) or move the shared type into a separate file to break the runtime cycle.
packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts:29buildDiagramElementsalready appliesapplyAutoLayoutinternally, but this test appliesapplyAutoLayout(...)again. This makes the test less representative (and can hide issues if the builder’s layout behavior changes). Also,reacflowGraphlooks like a typo and should likely bereactflowGraphfor readability.
014b92b
into
serverlessworkflow:main
Closes #53
Summary
This PR binds Store / model and SDK / graph to build the actual react flow nodes and edges and then render a workflow.
This way we have a fully implemented e2e (YAML / JSON -> model -> graph -> react flow nodes / edges -> workflow /view).
Changes