Repository navigation
feat(flowchart): collapsible subgraphs via @{ view: collapsed } - #7785
Conversation
A subgraph annotated with `@{ view: collapsed }` now renders as a single
compact node. Its internal nodes are hidden and edges crossing the subgraph
boundary are redirected to the collapsed node; nested collapses resolve to
the outermost collapsed ancestor.
Subgraph metadata is routed through the existing `id@{ ... }` statement
syntax in addVertex, so no grammar change is required. A new `collapsedGroup`
shape renders the compact node with a title and an ellipsis indicator.
Resolves #7784
🦋 Changeset detectedLatest commit: 5c877e1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7785 +/- ##
===========================================
- Coverage 77.43% 77.42% -0.01%
===========================================
Files 563 564 +1
Lines 74717 74900 +183
Branches 14570 12659 -1911
===========================================
+ Hits 57854 57995 +141
- Misses 15870 15909 +39
- Partials 993 996 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Covers an edge that targets the deepest node (two levels in) while the outermost subgraph is collapsed — the edge should redirect to the outermost collapsed node.
ashishjain0512
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Thanks for this — collapsible subgraphs is a frequently requested feature (#7784) and this is a really clean take on it. I read the full diff, ran an XSS/injection pass over the new shape and the metadata-routing changes, and verified the collapse/redirect logic against the unit and parser tests. Summary below; one important gap (docs) and a few small polish items, nothing blocking.
What's working well
- 🎉 Reusing
id@{ ... }instead of a grammar change. Routing subgraph metadata through the existingaddVertexpath (flowDb.ts:154 area) is exactly the right call — it avoids touching the JISON grammar, keeps the syntax consistent with node metadata, and means zero parser-regression risk. Nicely reasoned in the commit message too. - 🎉 Excellent test coverage for the scope. Unit tests in
flowDb.spec.tscover the normal-group baseline, collapse-to-single-node, boundary-edge redirect, internal-edge drop (self-loop avoidance), and nested outermost-ancestor resolution. The parser test and five e2e snapshots (including a handDrawn case and a deeply-nested redirect) round it out. This is the kind of coverage that makes a shared-rendering change safe to merge. - 🎉 Hand-drawn mode implemented from the start in
collapsedGroup.tsvia the rough.js path — not deferred, which is great. - 🎉 The edge-redirect logic is genuinely correct. I traced the
outermostCollapsedcontainment walk (with theseencycle guard) and the self-loop-vs-internal-edge distinction — thestart === end && (map.has(start) || map.has(end))guard correctly drops internal edges while preserving real self-loops on uncollapsed nodes. The nested case resolving to the outermost ancestor is handled well.
Security
XSS/injection review: clean. The @{ view: ... } value is parsed via yaml.load, only ever compared === 'collapsed' to select a literal shape string, and never reaches a DOM sink. The subgraph title flows through the same shared labelHelper (which sanitizes) as the existing expanded rect branch, and the new shape emits only plain rect/line/circle nodes that pass through the normal DOMPurify boundary. No new tags/attributes, no foreignObject/<use>/event handlers. No widening of the attack surface.
Things to address
- 🟡 [important] Missing user-facing documentation for the new syntax. This PR introduces a brand-new public syntax (
subgraphId@{ view: collapsed }) but adds no docs inpackages/mermaid/src/docs/syntax/flowchart.md(the only doc changes are the auto-generated interface line-number shifts). The subgraphs section is around flowchart.md:894 — it'd be great to add a short subsection there with an example, so the feature is discoverable. Remember to use theMERMAID_RELEASE_VERSIONplaceholder for the version.
Nits & suggestions
- 🟢 [nit] Import ordering in
shapes.ts:6—collapsedGroupis inserted betweenbowTieRectandcard, but alphabetically it sorts aftercard(ca<co). Minor, just for consistency with the rest of the block. - 🟢 [nit]
collapsedGroup.ts:585—const labelEl = shapeSvg.select('.label'); if (labelEl)is always truthy (d3selectreturns a non-null selection even when empty). If the intent is to guard against a missing label, useif (!labelEl.empty()). Harmless as-is since.attr()on an empty selection is a no-op. - 💡 [suggestion] When several edges from the same external node target different internal members of one collapsed subgraph (e.g.
X-->AandX-->BwithA,Bcollapsed), they redirect to two parallelX-->collapsededges with distinct counter-based ids. Functionally fine, but it renders as duplicate arrows. Deduplicating redirected edges on(start, end, label)could tidy that up — worth considering, not required for this PR. - 💡 [suggestion] The early
returnin the new subgraph-metadata branch ofaddVertexmeans anysubgraphId@{ ... }now routes all keys tosubGraph.metadata(onlyviewis consumed) and skips vertex creation. That's the right behavior forview, but worth a one-line comment noting other metadata keys on a subgraph id are intentionally ignored, so a future reader doesn't expect e.g.shapeto apply there.
Really solid work overall — once the docs are in, this is good to go. Let me know if you'd like a hand with the docs example. 🚀
🤖 Automated review. Severity: 0 blocking / 1 important / 2 nit / 2 suggestion / 4 praise.
The live editor cloned and built by scripts/editor.bash now requires Node >=24.16.0, but Netlify fell back to its default (22.14.0), failing the live editor's pnpm install with ERR_PNPM_UNSUPPORTED_ENGINE. Pin the Netlify build to a compatible Node version; the rest of CI is unaffected.
The Netlify deploy preview (scripts/editor.bash) clones and builds the Mermaid Live Editor, which now requires Node >=24.16.0. Netlify selects the build's Node from the repo's .node-version (the file takes precedence over the NODE_VERSION env var), so the live editor's pnpm install failed with ERR_PNPM_UNSUPPORTED_ENGINE on the old 22.14.0. Bump .node-version to 24.16.0. Verified locally on Node 24.16.0: build:esbuild, build:types and the full unit suite (5061 tests) pass; Cypress 14.5.4 supports Node 24 and includes the Node 24.16.0 binary fix. Reverts the ineffective netlify.toml NODE_VERSION pin.
knsv-bot
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Re-review after the latest pushes. Since the prior round (at 5f7d987), three things landed: a merge from develop, the docs commit that closes the one important gap from last time, and a pair of CI commits. I re-read the post-review delta, re-ran the unit/parser suites (42 passing locally), and ran a fresh XSS pass. The feature itself is in great shape — but one of the CI commits quietly bumped the repo-wide Node version, which I'd like to see split out before this merges. Verdict: COMMENT.
Resolved since last review
- ✅ Docs are now in (
src/docs/syntax/flowchart.md, commitd1f6e32). This fully addresses the prior 🟡. And they're genuinely good — a workingmermaid-example, thev<MERMAID_RELEASE_VERSION>+placeholder, and a clear bullet list of the behaviors that are easy to get surprised by (boundary-edge redirect, internal-edge drop, nested → outermost-ancestor resolution). Nicely done. 🎉
What's still working well
- 🎉 Core logic unchanged and still correct. I re-traced
outermostCollapsed(with itsseencycle guard) and the self-loop guardstart === end && (collapsedAncestorMap.has(start) || …)— the internal-edge drop vs. genuine-self-loop distinction holds, and nested collapses resolve to the outermost ancestor as documented (flowDb.ts:1131,:1236-1249). - 🎉 The new shape is tidy (
collapsedGroup.ts) — title + dashed separator + ellipsis indicator, hand-drawn path included, all geometry via the sharedlabelHelper/updateNodeBounds/intersecthelpers, no bespoke DOM. - 🎉 Tests stayed green through the develop merge —
flowDb.spec.ts(18) andsubgraph.spec.js(24) pass locally, and the full flowchart dir is 1041 passing. ESLint clean (0 errors), Prettier clean.
Things to address
🟡 [important] The .node-version bump is unrelated scope creep — please split it out
Commit 7c0379e ("ci: build on Node 24.16.0 to match the live editor") changes .node-version from 22.14.0 → 24.16.0 (confirmed: develop is still on 22.14.0, so merging this PR bumps it). A repo-wide Node major bump affects every CI job and every contributor's local toolchain (fnm/nvm/nodenv/asdf all read this file), and Node 22→24 can surface build/test differences that have nothing to do with collapsible subgraphs. That's exactly the kind of cross-cutting infra change that shouldn't ride along inside a feature PR — a maintainer reviewing "collapsible subgraphs" wouldn't expect it, and if it regresses an unrelated CI job the bisect points at the wrong PR. The sibling commit cc229e3 added a netlify.toml pin and 7c0379e then reverted it in favor of the global bump, which suggests this started as a deploy-preview fix for this PR's live editor — a narrower fix (or a standalone "bump Node to 24.16" PR with its own CI run and maintainer sign-off) would be cleaner. Could we drop it from here and land it separately? If it's already an intentional, coordinated bump happening elsewhere, just say so and I'll drop this.
Nits & suggestions (carried over from last round — all minor, none addressed since the core code didn't change)
- 🟢 [nit]
shapes.ts:6—collapsedGroupimport is betweenbowTieRectandblock_arrow; alphabetically it sorts aftercard/choice. Cosmetic only. - 🟢 [nit]
collapsedGroup.ts:59—if (labelEl)is always truthy (d3selectreturns a non-null selection even with no match). If the guard is meant to handle a missing label, useif (!labelEl.empty()). Harmless as-is since.attr()on an empty selection is a no-op. - 💡 [suggestion] Several edges from one external node into different members of the same collapsed subgraph still redirect to parallel
X-->collapsededges with distinct ids (renders as duplicate arrows). De-duping redirected edges on(start, end, label)would tidy it — not required here. - 💡 [suggestion] The early
returnin the subgraph-metadata branch ofaddVertex(flowDb.ts:156-159) routes all keys ontosubGraph.metadatathough onlyviewis read. A one-line comment that other keys on a subgraph id are intentionally ignored would save a future reader.
Security
Fresh XSS/injection pass over the current diff: clean. The @{ view: … } value is parsed by the existing JSON-schema-restricted yaml.load, only ever compared === 'collapsed' to pick a literal shape string, and never reaches a DOM sink. The subgraph title flows through the same sanitizing labelHelper/createText (DOMPurify) boundary as the existing expanded rect branch; the new shape emits only rect/line/circle; styles.ts interpolates only the clusterBorder theme variable. No new tags/attributes, no widening of the attack surface.
Severity tally: 🔴 0 · 🟡 1 · 🟢 2 · 💡 2 · 🎉 4. The feature is ready — the only thing standing between this and a clean approval is getting that Node bump out of the diff (or a note that it's intentional). Thanks again for adding the docs so promptly. 🚀
Re-reviewed the post-5f7d987 delta at head 7c0379e, against a freshly-fetched origin/develop, with the flowchart unit/parser suites run locally and a dedicated XSS sub-review.
Upstream release: https://github.com/mermaid-js/mermaid/releases/tag/mermaid%4011.17.0 Release notes: ### Minor Changes - [#7842](mermaid-js/mermaid#7842) [`3670b4e`](mermaid-js/mermaid@3670b4e) Thanks [@filipsajdak](https://github.com/filipsajdak)! - feat(c4): render C4 elements through the unified shape system, using the new person shape - [#7812](mermaid-js/mermaid#7812) [`cdfc0ea`](mermaid-js/mermaid@cdfc0ea) Thanks [@knsv-bot](https://github.com/knsv-bot)! - feat(class): route `classDiagram` to the unified (v2) renderer by default Set `class: { defaultRenderer: 'dagre-d3' }` in the config to restore the legacy renderer. - [#7785](mermaid-js/mermaid#7785) [`c45cde9`](mermaid-js/mermaid@c45cde9) Thanks [@knsv-bot](https://github.com/knsv-bot)! - feat(flowchart): add collapsible flowchart subgraphs via `subgraphId@{ view: collapsed }` - [#7828](mermaid-js/mermaid#7828) [`8eb3afc`](mermaid-js/mermaid@8eb3afc) Thanks [@knsv-bot](https://github.com/knsv-bot)! - feat(elk): add `elk.keepEntryNodeOnTop` config option to keep a recursive flow's entry node on top - [#7803](mermaid-js/mermaid#7803) [`74e44eb`](mermaid-js/mermaid@74e44eb) Thanks [@knsv-bot](https://github.com/knsv-bot)! - feat(elk): add `elk.nodePlacementAlignment` config option - [#7792](mermaid-js/mermaid#7792) [`ea55b31`](mermaid-js/mermaid@ea55b31) Thanks [@RodrigojndSantos](https://github.com/RodrigojndSantos)! - feat(er): add subgraph support to ER diagrams. - [#7970](mermaid-js/mermaid#7970) [`a2c0fb6`](mermaid-js/mermaid@a2c0fb6) Thanks [@filipsajdak](https://github.com/filipsajdak)! - feat(flowchart): add `folder`, `bucket`, `console` (terminal window) and `browser` shapes - [#7842](mermaid-js/mermaid#7842) [`ae3e115`](mermaid-js/mermaid@ae3e115) Thanks [@filipsajdak](https://github.com/filipsajdak)! - feat(flowchart): add `person` shape (circular head above a rounded body), usable in flowcharts via `A@{ shape: person }` - [#7724](mermaid-js/mermaid#7724) [`0fd7a9f`](mermaid-js/mermaid@0fd7a9f) Thanks [@xdumaine](https://github.com/xdumaine)! - feat(xyChart): add legends for named line and bar series ### Patch Changes - [#7847](mermaid-js/mermaid#7847) [`215fe89`](mermaid-js/mermaid@215fe89) Thanks [@filipsajdak](https://github.com/filipsajdak)! - fix(c4): named attributes such as `$tags`, `$link` and `$sprite` are no longer clobbered to undefined when they arrive in an earlier positional slot of Person/System/Container/Component/Boundary/Rel statements. - [#7871](mermaid-js/mermaid#7871) [`8d874c4`](mermaid-js/mermaid@8d874c4) Thanks [@knsv-bot](https://github.com/knsv-bot)! - fix(flowchart): stop dagre layout from spamming `warn`-level logs on every node/edge/cluster - [#8071](mermaid-js/mermaid#8071) [`b3d1f63`](mermaid-js/mermaid@b3d1f63) Thanks [@pbrolin47](https://github.com/pbrolin47)! - fix(block): sibling blocks overlapping in block diagrams when one has a label wider than 200px - [#7870](mermaid-js/mermaid#7870) [`71b8843`](mermaid-js/mermaid@71b8843) Thanks [@knsv-bot](https://github.com/knsv-bot)! - fix: a `RangeError: Invalid array length` crash when rendering certain edges. - [#7924](mermaid-js/mermaid#7924) [`9cbef5d`](mermaid-js/mermaid@9cbef5d) Thanks [@nightt5879](https://github.com/nightt5879)! - fix(treeView): icons disappearing after strict security sanitization. - [#7850](mermaid-js/mermaid#7850) [`a34cbf0`](mermaid-js/mermaid@a34cbf0) Thanks [@aloisklink](https://github.com/aloisklink)! - fix(block): allow classdefs to update text color - [#7937](mermaid-js/mermaid#7937) [`f9cbe1e`](mermaid-js/mermaid@f9cbe1e) Thanks [@filipsajdak](https://github.com/filipsajdak)! - fix(dagre): let a diagram's own nodeSpacing/rankSpacing take effect in the unified dagre layout - [#8005](mermaid-js/mermaid#8005) [`90eeece`](mermaid-js/mermaid@90eeece) Thanks [@pbrolin47](https://github.com/pbrolin47)! - fix(flowchart): reverts the behavior change from #7672 (fix/4648-directions), since arrows between subgraphs are broken - [#7951](mermaid-js/mermaid#7951) [`afa2f80`](mermaid-js/mermaid@afa2f80) Thanks [@aloisklink](https://github.com/aloisklink)! - perf: use `fastdom` to batch DOM measurements (up to 25% speedup) - Updated dependencies \[[`e848423`](mermaid-js/mermaid@e848423)]: - @mermaid-js/parser@1.2.1 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: give me a 98K <240642031+inhuman-0@users.noreply.github.com>
Summary
Resolves #7784
Adds opt-in collapsible subgraphs to the flowchart. A subgraph annotated with
@{ view: collapsed }is rendered as a single compact node instead of an expanded container:flowchart TD Start --> one subgraph one [My Group] A --> B B --> C end one --> End one@{ view: collapsed }How it works
id@{ ... }statement syntax —addVertexdetects when the id refers to a declared subgraph and routes the metadata onto it. No grammar change is required.flowDb.getData().collapsedGroupshape renders the compact node (title + dashed separator + ellipsis indicator), reusing the cluster theme variables.Classification
rendering-util(new shape)view: collapsedsyntax; existing diagrams produce identical output. The new shape is purely additive.rendering-util/rendering-elements/shapes/collapsedGroup.ts+ one registration entry inshapes.ts(additive only; follows the existing pattern forkanbanItem/classBox/erBox).Verification
vitest run packages/mermaid/src/diagrams/flowchart/)cypress/integration/rendering/flowchart/flowchart-v2.spec.js(collapsed, side-by-side, hand-drawn, nested)--no-sandbox; suite is heavy) — covered by the new snapshot tests in CIminor)