Repository navigation
feat(erDiagram): add subgraph support - #7792
Conversation
Adds subgraph support to ER diagrams, allowing grouping of entities into logical domains. Includes: - Subgraphs with id/title syntax - Nested subgraphs - Relationships within and across subgraphs - Per-subgraph direction handling Also adds parser and DB test coverage, and documentation for subgraph usage. Closes mermaid-js#7417 Co-authored-by: Beatriz Braga <beatrizagbraga@tecnico.ulisboa.pt>
🦋 Changeset detectedLatest commit: c81fcd4 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 #7792 +/- ##
==========================================
- Coverage 3.27% 2.80% -0.47%
==========================================
Files 598 669 +71
Lines 60420 73038 +12618
Branches 913 992 +79
==========================================
+ Hits 1980 2051 +71
- Misses 58440 70987 +12547
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 ↗︎
|
|
Hi @RodrigojndSantos . [sisyphos-bot] What's working well 🎉 Sound architectural pattern. The implementation mirrors flowchart's established subgraph model — isGroup: true nodes with parentId, delegating cluster rendering to the 🎉 Strong parser/DB test coverage. subgraph.spec.js (332 lines) and erDb.spec.js thoroughly cover nested subgraphs, direction inheritance, makeUniq deduplication, and 🎉 Correct title sanitization. Subgraph title goes through sanitizeText → common.sanitizeText → DOMPurify before storage. Confirmed by XSS sub-agent (no blocking security 🎉 Lexer cleanup is a good catch. Removing the dead SPACE token rule ([\s]+ was unreachable since \s+ came first) and switching to [ \t\r]+ is a correct fix that was Security XSS sub-agent reviewed all changed files and the full data flow. Summary of findings (all pre-existing patterns, not new regressions):
No new XSS sinks or sanitization bypasses introduced. Things to address 🔴 [blocking] — Missing E2E visual regression tests styles.ts adds new .cluster rect, .cluster text, .cluster-label text rules. getData() now produces cluster nodes (isGroup: true). Nested subgraph layout, relationships Per CLAUDE.md: "Any change that affects rendered SVG output [...] must add or update e2e tests." Unit tests verify that the DB stores the right data structures. They don't At minimum, add a test in cypress/integration/rendering/erDiagram.spec.js using imgSnapshotTest covering:
🟡 [important] — setClass auto-creates entities: potential ghost nodes Before this PR, setClass was a no-op if the entity didn't exist. The PR adds: // erDb.ts:setClass (new code) If a user writes class UnknownId someStyle where UnknownId is neither a declared entity nor a subgraph, a ghost entity is now created and added to this.entities. Since The existing behavior was intentional — setClass was a post-declaration step, not an entity creator. The comment says "Ensure entity exists so classes applied even if class 🟡 [important] — document rule push → concat change applies to all ER diagrams -| document line {$1.push( This change is necessary for subgraphs (statements now return arrays of entity names, so concat correctly flattens them). But it affects every ER diagram parse, not just Specifically: classDefStatement, classStatement, styleStatement, and some entityName SQS entityName SQE alternatives don't have explicit $$ = [...] assignments. JISON This is likely fine in practice (the tests pass), but it's worth explicitly confirming with the existing ER diagram E2E suite rather than relying only on unit tests. 🟡 [important] — subgraph "Some Title" makes id === title, including the space 🟡 [important] — Please add a unit test for DB:getData() with cluster output 🟡 [important] — subgraph "Some Title" makes id === title, including the space The docs correctly note: "If the subgraph identifier contains more than one word, it must be written in quotes. In this case, the quoted value is used both as the id and the title." So subgraph This means relationships involving space-containing subgraph ids must also use quoted syntax: "Customer Domain" }|--|| other. This is not documented. The test verifies the parse works but doesn't Nits 🟢 Dead code: _id === _title reference check (erDb.ts:addSubGraph) 🟢 makeUniq visibility — makeUniq is public but is only called internally. Should be private. 🟢 subgraphDepth visibility — subgraphDepth is public because JISON's yy object accesses it. A comment explaining this constraint would help future readers who'd otherwise wonder why a private 🟢 private config = getConfig() capture — this.config is captured at ErDB construction time. If mermaid.initialize() is called after construction (as can happen in some integration patterns), the |
Addresses feedback from the review: - add Cypress snapshot tests for subgraphs. - add unit tests for getData() cluster output. - clarify documentation for quoted identifiers in relationships. - address minor nits and code cleanup. Closes mermaid-js#7417 Co-authored-by: Beatriz Braga <beatrizagbraga@tecnico.ulisboa.pt>
|
Hi @pbrolin47 ,
Regarding the change from push to concat in the document rule, we validated the impact by running the full existing test suite (unit and E2E), which continues to pass without issues. This indicates that the previous behavior remains unchanged while correctly supporting the array-based returns introduced for subgraphs. Ready for re-review. |
knsv-bot
left a comment
There was a problem hiding this comment.
[sisyphus-bot]
Review: feat(erDiagram): add subgraph support
Thanks for this, @RodrigojndSantos — this is a genuinely well-executed feature. Subgraphs are one of the most-requested ER additions (#7417), and the way you've approached it — modelling subgraphs as isGroup cluster nodes with parentId, so they flow through the existing unified layout/cluster renderer rather than needing a custom ER renderer — is exactly the right call. The test suite is thorough and I was able to verify the important properties locally. A couple of small things to tidy, but nothing blocking. 🎉
What's working well
- 🎉 Architecture reuses the proven flowchart subgraph model. Mapping subgraphs to
{ isGroup: true, parentId, dir, look }nodes ingetData()(erDb.ts) means clusters, nesting, and handdrawn all route through the sharedrendering-utilcluster code — no new renderer, minimal surface area, and you inherit the already-hardened rendering path. - 🎉 Strong, multi-layered test coverage.
parser/subgraph.spec.js(16 cases: nesting, per-subgraphdirection, cross-subgraph relationships, quoted /id [title]/ multi-word titles, empty subgraphs, whitespace-around-end), pluserDb.spec.jsunit tests forgetData()cluster output (isGroup/parentIdparent-child wiring),makeUniqdedup, and subgraph-as-relationship-endpoint, plus 9 Cypress visual snapshots. This is the kind of coverage that makes a feature like this safe to land. - 🎉 No regressions from the lexer rewrite. Reworking the whitespace handling (dropping the
SPACEtoken in favour of significantNEWLINEs) is the riskiest part of the diff since it changes tokenization for every ER diagram. I ran the full existing ER suite against this branch and all 660 tests still pass, which is strong evidence the refactor is safe. - 🎉 Nice bonus correctness fix:
getData()now returnsdirection: this.directioninstead of the previously hardcoded'TB', so ER diagrams finally honour a top-leveldirectiondeclaration.
Security
I ran a dedicated XSS/injection pass over the new code. No issues found. Subgraph titles are sanitized at DB-write time via common.sanitizeText (erDb.ts), the .cluster CSS in styles.ts:113-125 interpolates only theme variables (never diagram text), user style/cssStyles flow through the same grammar-constrained, DOMPurify-backstopped path flowchart uses, and the subgraph id is only ever set as a literal .attr('id', …) value — never into a selector or raw markup. The sanitization boundary matches the established flowchart baseline exactly.
Things to address
-
🟡 Handdrawn mode isn't demonstrated. The cluster nodes set
look: config.look, so handdrawn should work for free via the shared cluster renderer — but there's no snapshot proving it. Per project convention, handdrawn support should either be demonstrated or explicitly noted as unavailable. Could you add one Cypress snapshot with{ look: 'handDrawn' }(the existinger/erDiagram.spec.jscases make a good template)? That locks it in and confirms nothing breaks. -
🟢 Edge case: accessibility/
titlestatements inside a subgraph body leak as phantom nodes. I verified thaterDiagram\nsubgraph G\nA\ntitle My Title\nendproducessubgraph G.nodes === ["A", "title", "My", "Title"]— thetitle …line is split into bare entities instead of being treated as a title (or rejected). It's an unusual construct, so not blocking, but a small guard inaddSubGraph'suniq()(or filtering non-node statements out of the document list) would make the membership logic more robust. Worth a quick test either way. -
🟢
addSubGraphleans onanymore than it needs to.const erConfig = (getConfig().er ?? {}) as anyand theprims: any/objs: any[]scratch objects inuniq()—er.inheritDiris already a typed config option (it exists inconfig.schema.yaml/BaseDiagramConfigondevelop), sogetConfig().er?.inheritDirshould type-check without the cast. Theuniq()helper is a port of flowchart's, so I won't push hard, but tightening the types here would be a nice cleanup. -
🟢
makeUniqsilently drops a node that's already claimed by an earlier subgraph. This matches flowchart semantics (a node belongs to exactly one subgraph), and yourerDb.spec.jstest documents it well — but ER users coming from the entity-centric mental model may be surprised. A one-linelog.warnwhen a node is dropped would make the behaviour discoverable. Non-blocking.
Process ✅
Changeset present (minor, feat:), docs updated in src/docs/ with the v<MERMAID_RELEASE_VERSION>+ placeholder (and the generated docs/ copy regenerated to match), and the PR links #7417. All good.
This is close — the handdrawn snapshot is the main thing I'd like to see, and the two 🟢 robustness items are quick. Really nice work on a long-requested feature. Let's get it landed. 🚀
Addresses review feedback:
-add Cypress snapshot coverage for ER subgraphs, with
{ look: 'handDrawn' }.
-simplify uniq() and direction handling;
direction now relies on result.dir with global/default fallback.
-add log.warn in makeUniq when nodes are dropped.
Closes mermaid-js#7417
Co-authored-by: Beatriz Braga <beatrizagbraga@tecnico.ulisboa.pt>
|
Hi @knsv-bot,
Please review and let us know your feedback. Thanks. |
|
Hi @RodrigojndSantos [sisyphos-bot]What's working well 🎉 All items from the two prior reviews have been addressed — this is a well-managed iteration, and the quality delta between the first and current version is significant. 🎉 The handdrawn mode concern is fully resolved. The tests are in erDiagram-unified.spec.js inside the testOptions.forEach() loop that includes { description: 'HD: ', options: { 🎉 The addSubGraph cleanup is clean. The erConfig as any complexity and the unreachable _id === _title dead-code check (copied from flowchart's version) are both absent. The 🎉 makeUniq is correctly private, subgraphDepth has its visibility comment, and the log.warn for duplicate node membership is in place. 🎉 The getData() unit tests in erDb.spec.js are thorough — nested cluster parentId wiring, isGroup flags, and edges connected to subgraph endpoints are all covered. 🎉 The E2E coverage is solid: 8 cases in erDiagram.spec.js (simple, empty, nested, cross-subgraph relationships, quoted ids, explicit id+title, direction override) + 2 in the Security XSS sub-agent review pending — incorporating findings below once complete. Based on manual review of the diff:
Remaining open items None blocking. The title My Title inside subgraph producing phantom entity nodes is an acknowledged pre-existing pattern (consistent behavior with entities outside subgraphs). |
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
This PR introduces support for subgraphs in Entity Relationship (ER) diagrams, enabling entities to be grouped into logical domains. This improves diagram clarity and allows better structuring of complex diagrams, including support for nested subgraphs and relationships across different subgraph scopes.
Resolves #7417
📏 Design Decisions
The parser was extended to support ER subgraph syntax, including nested subgraphs. Subgraphs can be defined using different formats :
The database was updated to store subgraphs as core structures in the model, similar to entities, adding support for subgraph storage, lookup, and id management. Relationship logic was adapted to allow subgraphs as valid endpoints, enabling relationships between entities and subgraphs. The data preparation step was updated to treat subgraphs as grouped nodes while preserving existing entity behavior. Existing class, style, and configuration handling were reused to ensure consistent rendering.
The example below illustrates the main behavior of the feature:
📋 Tasks
Make sure you
MERMAID_RELEASE_VERSIONis used for all new features.pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.