diff --git a/.changeset/fix-4648-subgraph-direction.md b/.changeset/fix-4648-subgraph-direction.md new file mode 100644 index 00000000000..11fd89572fd --- /dev/null +++ b/.changeset/fix-4648-subgraph-direction.md @@ -0,0 +1,30 @@ +--- +'mermaid': patch +--- + +fix(flowchart): respect per-subgraph direction keyword in Dagre layout + +Subgraphs with an explicit `direction` keyword (e.g. `direction LR`) now +trigger a separate cluster sub-layout, restoring the intended visual direction +for that group. Previously the cluster-extraction predicate used +`!externalConnections`, which caused subgraphs with external edges to silently +inherit the top-level `rankdir` even when the user had specified a different +direction. + +**Behavior change:** + +- **Old:** A separate cluster graph was created for any subgraph that had + children and no external connections (`!externalConnections && hasChildren`). + Subgraphs with external edges always inherited the parent direction. +- **New:** A separate cluster graph is created only when the user has + explicitly set a `direction` keyword on the subgraph + (`clusterData?.explicitDir && hasChildren`). Subgraphs without an explicit + `direction` now inherit the parent `rankdir` (previously they defaulted to + the opposite of the parent direction). + +Also normalises `direction TD` to `direction TB` inside `addSubGraph` to match +the existing top-level normalisation in `setDirection`, fixing the +"`direction TD` doesn't work inside subgraphs" complaint in #4648. + +Fixes #4648 +See also #6785 diff --git a/cypress/integration/rendering/flowchart/flowchart-v2.spec.js b/cypress/integration/rendering/flowchart/flowchart-v2.spec.js index 9adb1f4be39..b0971976196 100644 --- a/cypress/integration/rendering/flowchart/flowchart-v2.spec.js +++ b/cypress/integration/rendering/flowchart/flowchart-v2.spec.js @@ -351,6 +351,55 @@ end { htmlLabels: true, flowchart: { htmlLabels: true }, securityLevel: 'loose' } ); }); + it('issue-4648: sibling subgraphs with different directions and external connections', () => { + imgSnapshotTest( + `flowchart TD + +subgraph Group1 + direction TB + A1 --> A2 + A2 --> A3 +end + +subgraph Group2 + direction LR + B1 --> B2 + B2 --> B3 +end + +subgraph Group3 + direction LR + C1 --> C2 + C2 --> C3 +end + +%% External connections between subgraphs +A3 --- B1 +B3 --- C1 + `, + { htmlLabels: true, flowchart: { htmlLabels: true }, securityLevel: 'loose' } + ); + }); + + it('issue-4648: nested subgraph with external connection', () => { + imgSnapshotTest( + `flowchart TD + +subgraph Wrapper + direction LR + subgraph Inner + D1 --> D2 + D2 --> D3 + end +end + +%% External connection to nested subgraph +D3 --- E1 + `, + { htmlLabels: true, flowchart: { htmlLabels: true }, securityLevel: 'loose' } + ); + }); + it('57.x: handle nested subgraphs with outgoing links 5', () => { imgSnapshotTest( `%% this does not produce the desired result diff --git a/cypress/platform/regression/issue-4648.html b/cypress/platform/regression/issue-4648.html new file mode 100644 index 00000000000..b4d2ccfe54d --- /dev/null +++ b/cypress/platform/regression/issue-4648.html @@ -0,0 +1,69 @@ + + + + + Mermaid Subgraph Direction Test (Issue #4648) + + + + +

Mermaid Subgraph Direction Test (Issue #4648)

+

This page demonstrates various subgraph direction scenarios for regression testing.

+ +

Sibling Subgraphs with Different Directions

+
+flowchart TD
+
+subgraph Group1
+  direction TB
+  A1 --> A2
+  A2 --> A3
+end
+
+subgraph Group2
+  direction LR
+  B1 --> B2
+  B2 --> B3
+end
+
+subgraph Group3
+  direction LR
+  C1 --> C2
+  C2 --> C3
+end
+
+%% External connections between subgraphs
+A3 --- B1
+B3 --- C1
+  
+ +

Nested Subgraph with External Connections

+
+flowchart TD
+
+subgraph Wrapper
+  direction LR
+  subgraph Inner
+    D1 --> D2
+    D2 --> D3
+  end
+end
+
+%% External connection to nested subgraph
+D3 --- E1
+  
+ + diff --git a/docs/config/setup/mermaid/interfaces/LayoutData.md b/docs/config/setup/mermaid/interfaces/LayoutData.md index a85eb53043f..cd84690ea9b 100644 --- a/docs/config/setup/mermaid/interfaces/LayoutData.md +++ b/docs/config/setup/mermaid/interfaces/LayoutData.md @@ -10,7 +10,7 @@ # Interface: LayoutData -Defined in: [packages/mermaid/src/rendering-util/types.ts:169](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L169) +Defined in: [packages/mermaid/src/rendering-util/types.ts:170](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L170) ## Indexable @@ -22,7 +22,7 @@ Defined in: [packages/mermaid/src/rendering-util/types.ts:169](https://github.co > **config**: [`MermaidConfig`](MermaidConfig.md) -Defined in: [packages/mermaid/src/rendering-util/types.ts:172](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L172) +Defined in: [packages/mermaid/src/rendering-util/types.ts:173](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L173) --- @@ -30,7 +30,7 @@ Defined in: [packages/mermaid/src/rendering-util/types.ts:172](https://github.co > `optional` **diagramId**: `string` -Defined in: [packages/mermaid/src/rendering-util/types.ts:173](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L173) +Defined in: [packages/mermaid/src/rendering-util/types.ts:174](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L174) --- @@ -38,7 +38,7 @@ Defined in: [packages/mermaid/src/rendering-util/types.ts:173](https://github.co > **edges**: `Edge`\[] -Defined in: [packages/mermaid/src/rendering-util/types.ts:171](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L171) +Defined in: [packages/mermaid/src/rendering-util/types.ts:172](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L172) --- @@ -46,4 +46,4 @@ Defined in: [packages/mermaid/src/rendering-util/types.ts:171](https://github.co > **nodes**: `Node`\[] -Defined in: [packages/mermaid/src/rendering-util/types.ts:170](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L170) +Defined in: [packages/mermaid/src/rendering-util/types.ts:171](https://github.com/mermaid-js/mermaid/blob/master/packages/mermaid/src/rendering-util/types.ts#L171) diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.ts index fa3db3af681..87de8209bff 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.ts @@ -699,10 +699,18 @@ You have to call mermaid.initialize.` const result = uniq(list.flat()); const nodeList = result.nodeList; - let dir = result.dir; + // Preserve the raw user-authored direction value (e.g. 'TD') on the subGraph + // object so that tests and callers see what the user actually wrote. + // Normalization to dagre's canonical 'TB' happens in getData() when the dir + // is consumed by the layout engine. + const rawDir = result.dir; + // Capture whether the user explicitly wrote a direction keyword BEFORE any + // inheritDir override, so that explicitDir is true only for user-authored + // direction statements. + const hasExplicitDir = rawDir !== undefined; const flowchartConfig = getConfig().flowchart ?? {}; - dir = - dir ?? + const dir = + rawDir ?? (flowchartConfig.inheritDir ? (this.getDirection() ?? (getConfig() as any).direction ?? undefined) : undefined); @@ -724,6 +732,7 @@ You have to call mermaid.initialize.` title: title.trim(), classes: [], dir, + hasExplicitDir, labelType: this.sanitizeNodeLabelType(_title?.type), }; @@ -1117,7 +1126,8 @@ You have to call mermaid.initialize.` cssCompiledStyles: this.getCompiledStyles(subGraph.classes), cssClasses: subGraph.classes.join(' '), shape: 'rect', - dir: subGraph.dir, + dir: subGraph.dir === 'TD' ? 'TB' : subGraph.dir, // normalize TD→TB for dagre + explicitDir: subGraph.hasExplicitDir, // true only when the user wrote an explicit 'direction X' keyword isGroup: true, look: config.look, }); diff --git a/packages/mermaid/src/diagrams/flowchart/types.ts b/packages/mermaid/src/diagrams/flowchart/types.ts index ac2d0401518..6df7e110478 100644 --- a/packages/mermaid/src/diagrams/flowchart/types.ts +++ b/packages/mermaid/src/diagrams/flowchart/types.ts @@ -78,6 +78,7 @@ export interface FlowClass { export interface FlowSubGraph { classes: string[]; dir?: string; + hasExplicitDir: boolean; id: string; labelType: string; nodes: string[]; diff --git a/packages/mermaid/src/diagrams/state/dataFetcher.ts b/packages/mermaid/src/diagrams/state/dataFetcher.ts index 76212656207..76c388e0fe5 100644 --- a/packages/mermaid/src/diagrams/state/dataFetcher.ts +++ b/packages/mermaid/src/diagrams/state/dataFetcher.ts @@ -271,6 +271,11 @@ export const dataFetcher = ( newNode.type = 'group'; newNode.isGroup = true; newNode.dir = getDir(parsedItem); + // Set explicitDir only when the user actually wrote a 'direction X' keyword + // inside this state body. mermaid-graphlib's Branch 1 checks explicitDir (not dir) + // so that state compound states without an explicit direction follow the original + // !externalConnections extraction path, while keeping dir for Branch 2's direction arithmetic. + newNode.explicitDir = parsedItem.doc.some((s) => s.stmt === 'dir'); newNode.shape = parsedItem.type === DIVIDER_TYPE ? SHAPE_DIVIDER : SHAPE_GROUP; newNode.cssClasses = `${newNode.cssClasses} ${CSS_DIAGRAM_CLUSTER} ${altFlag ? CSS_DIAGRAM_CLUSTER_ALT : ''}`; } diff --git a/packages/mermaid/src/diagrams/state/stateDb.ts b/packages/mermaid/src/diagrams/state/stateDb.ts index adb15d661d3..16ab64f5f80 100644 --- a/packages/mermaid/src/diagrams/state/stateDb.ts +++ b/packages/mermaid/src/diagrams/state/stateDb.ts @@ -150,6 +150,7 @@ export interface NodeData { cssStyles: string[]; id: string; dir?: string; + explicitDir?: boolean; // true only when the user wrote an explicit 'direction X' keyword domId?: string; type?: string; isGroup?: boolean; diff --git a/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.js b/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.js index bab1c6c479f..ffe9062154f 100644 --- a/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.js +++ b/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.js @@ -92,9 +92,30 @@ const copy = (clusterId, graph, newGraph, rootId) => { log.info('Edge data', data, rootId); try { if (edgeInCluster(edge, rootId)) { - log.info('Copying as ', edge.v, edge.w, data, edge.name); - newGraph.setEdge(edge.v, edge.w, data, edge.name); - log.info('newGraph edges ', newGraph.edges(), newGraph.edge(newGraph.edges()[0])); + // Determine whether BOTH endpoints are strictly inside the cluster. + // edgeInCluster uses OR logic (either endpoint inside), so a + // cross-boundary edge (one endpoint outside rootId) also passes. + // Copying such an edge into newGraph would auto-create the external + // node as an orphan with no layout data, crashing the renderer. + // Instead, rebind cross-boundary edges in the outer graph as + // rootId → externalNode + // so the connection is preserved after the leaf is removed. + const rootDescendants = descendants.get(rootId) || []; + const vIn = + rootDescendants.includes(edge.v) || isDescendant(edge.v, rootId) || edge.v === rootId; + const wIn = + rootDescendants.includes(edge.w) || isDescendant(edge.w, rootId) || edge.w === rootId; + if (vIn && wIn) { + log.info('Copying as ', edge.v, edge.w, data, edge.name); + newGraph.setEdge(edge.v, edge.w, data, edge.name); + log.info('newGraph edges ', newGraph.edges(), newGraph.edge(newGraph.edges()[0])); + } else { + // Cross-boundary: rebind to the cluster root in the outer graph. + const newV = vIn ? rootId : edge.v; + const newW = wIn ? rootId : edge.w; + log.info('Rebinding cross-boundary edge as ', newV, newW, data, edge.name); + graph.setEdge(newV, newW, data, edge.name); + } } else { log.info( 'Skipping copy of edge ', @@ -340,11 +361,54 @@ export const extractor = (graph, depth) => { ); if (!clusterDb.has(node)) { log.debug('Not a cluster', node, depth); + } else if ( + clusterDb.get(node)?.clusterData?.explicitDir && + graph.children(node) && + graph.children(node).length > 0 + ) { + // Cluster with an explicit direction keyword — always create a subgraph, + // even when it has external connections (fixes issue #4648). + log.warn('Cluster with explicit dir, creating subgraph for children', node, depth); + + const dir = clusterDb.get(node).clusterData.dir; + const clusterGraph = new graphlib.Graph({ + multigraph: true, + compound: true, + }) + .setGraph({ + rankdir: dir, + nodesep: 50, + ranksep: 50, + marginx: 8, + marginy: 8, + }) + .setDefaultEdgeLabel(function () { + return {}; + }); + + // Copy the cluster (and any nested sub-clusters) into the subgraph + copy(node, graph, clusterGraph, node); + // Attach the subgraph to the cluster node for internal layout + const clusterNodeData = graph.node(node) || {}; + graph.setNode(node, { + ...clusterNodeData, + clusterNode: true, + id: node, + clusterData: clusterDb.get(node).clusterData, + label: clusterDb.get(node).label, + graph: clusterGraph, + }); + log.warn( + 'Subgraph for cluster with explicit dir created:', + node, + graphlibJson.write(clusterGraph) + ); } else if ( !clusterDb.get(node).externalConnections && graph.children(node) && graph.children(node).length > 0 ) { + // Original behaviour: cluster without external connections gets its own sub-graph. log.warn( 'Cluster without external connections, without a parent and with children', node, @@ -373,16 +437,16 @@ export const extractor = (graph, depth) => { return {}; }); - log.warn('Old graph before copy', graphlibJson.write(graph)); copy(node, graph, clusterGraph, node); + const clusterNodeData = graph.node(node) || {}; graph.setNode(node, { + ...clusterNodeData, clusterNode: true, id: node, clusterData: clusterDb.get(node).clusterData, label: clusterDb.get(node).label, graph: clusterGraph, }); - log.warn('New graph after copy node: (', node, ')', graphlibJson.write(clusterGraph)); log.debug('Old graph after copy', graphlibJson.write(graph)); } else { log.warn( diff --git a/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.spec.js b/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.spec.js index 11acd44eb18..2e21b8ec4d9 100644 --- a/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.spec.js +++ b/packages/mermaid/src/rendering-util/layout-algorithms/dagre/mermaid-graphlib.spec.js @@ -399,6 +399,109 @@ flowchart TB expect(aGraph.parent('c')).toBe('B'); expect(aGraph.parent('B')).toBe(undefined); }); + describe('explicit direction (clusterData.dir) – Branch 1 coverage', function () { + it('GLB-DIR1: cluster with explicit dir and external connections should become a clusterNode', function () { + /* + subgraph C1 [direction LR] + a + end + C1 --> b (external connection) + */ + g.setNode('a', { data: 1 }); + g.setNode('b', { data: 2 }); + g.setNode('C1', { data: 3, dir: 'LR', explicitDir: true }); + g.setParent('a', 'C1'); + g.setEdge('C1', 'b', { data: 'link1' }, '1'); + + adjustClustersAndEdges(g); + + // C1 should have been extracted into a clusterNode via Branch 1 (explicitDir) + expect(g.node('C1').clusterNode).toBe(true); + // The internal child 'a' should be in the clusterGraph, not in the outer graph + const C1Graph = g.node('C1').graph; + expect(C1Graph.nodes()).toEqual(['a']); + // Outer graph should only contain C1 and b + expect(g.nodes()).toEqual(['b', 'C1']); + }); + + it('GLB-DIR2: cluster with explicit dir and no external connections should still become a clusterNode', function () { + /* + subgraph C1 [direction LR] + a --> b + end + (no edges leaving C1) + */ + g.setNode('a', { data: 1 }); + g.setNode('b', { data: 2 }); + g.setNode('C1', { data: 3, dir: 'LR', explicitDir: true }); + g.setParent('a', 'C1'); + g.setParent('b', 'C1'); + g.setEdge('a', 'b', { data: 'internal' }, '1'); + + adjustClustersAndEdges(g); + + expect(g.node('C1').clusterNode).toBe(true); + const C1Graph = g.node('C1').graph; + expect(C1Graph.nodes().sort()).toEqual(['a', 'b']); + expect(C1Graph.edges().length).toBe(1); + expect(g.nodes()).toEqual(['C1']); + }); + + it('GLB-DIR3: sibling clusters with different explicit directions and inter-cluster edge', function () { + /* + subgraph B1 [direction RL] + i1 + end + subgraph B2 [direction BT] + i2 + end + B1 --> B2 + */ + g.setNode('i1', { data: 1 }); + g.setNode('i2', { data: 2 }); + g.setNode('B1', { data: 3, dir: 'RL', explicitDir: true }); + g.setNode('B2', { data: 4, dir: 'BT', explicitDir: true }); + g.setParent('i1', 'B1'); + g.setParent('i2', 'B2'); + g.setEdge('B1', 'B2', { data: 'sibling-link' }, '1'); + + adjustClustersAndEdges(g); + + expect(g.node('B1').clusterNode).toBe(true); + expect(g.node('B2').clusterNode).toBe(true); + expect(g.node('B1').graph.nodes()).toEqual(['i1']); + expect(g.node('B2').graph.nodes()).toEqual(['i2']); + }); + + it('GLB-DIR4: cross-boundary edge should be rebound in outer graph, not copied into clusterGraph', function () { + /* + subgraph C2 [direction LR] ← uses a fresh cluster id to avoid stale module state + D1 + end + D1 --> D2 (cross-boundary: D1 is inside C2, D2 is outside) + */ + g.setNode('D1', { data: 1 }); + g.setNode('D2', { data: 2 }); + g.setNode('C2', { data: 3, dir: 'LR', explicitDir: true }); + g.setParent('D1', 'C2'); + g.setEdge('D1', 'D2', { data: 'cross' }, 'cross1'); + + adjustClustersAndEdges(g); + + // C2 should be a clusterNode (Branch 1 fired) + expect(g.node('C2').clusterNode).toBe(true); + + // The clusterGraph should contain D1 but have NO edges + // (the cross-boundary edge must NOT be copied inside) + const C2Graph = g.node('C2').graph; + expect(C2Graph.nodes()).toContain('D1'); + expect(C2Graph.edges().length).toBe(0); + + // The outer graph should have a rebound edge from the cluster root (C2) to D2 + const rebound = g.edges().some((e) => e.v === 'C2' && e.w === 'D2'); + expect(rebound).toBe(true); + }); + }); }); describe('extractDescendants', function () { let g; diff --git a/packages/mermaid/src/rendering-util/types.ts b/packages/mermaid/src/rendering-util/types.ts index 4c4099f6e8d..9f924e3bcd9 100644 --- a/packages/mermaid/src/rendering-util/types.ts +++ b/packages/mermaid/src/rendering-util/types.ts @@ -34,6 +34,7 @@ interface BaseNode { domId?: string; // When you create the node in the getData function you do not have the domId yet // Rendering specific properties for both Flowchart and State Diagram nodes dir?: string; // Only relevant for isGroup true, i.e. a sub-graph or composite state. + explicitDir?: boolean; // true only when the user wrote an explicit 'direction X' keyword haveCallback?: boolean; link?: string; linkTarget?: string;