Bug/5741 fix duplicate svg element ids#7410
Open
alexander-turner wants to merge 27 commits intomermaid-js:developfrom
Open
Bug/5741 fix duplicate svg element ids#7410alexander-turner wants to merge 27 commits intomermaid-js:developfrom
alexander-turner wants to merge 27 commits intomermaid-js:developfrom
Conversation
When multiple mermaid diagrams appear on the same page, internal SVG element IDs for nodes and edges collide (e.g., flowchart-A-0 appears twice). This causes invalid HTML (WCAG 4.1.1), broken url(#...) refs, and CSS selectors matching the wrong element. Prefix all internal element IDs with the diagram's SVG element ID (e.g., mermaid-0-flowchart-A-0), following the same pattern used for marker IDs in PR mermaid-js#4825. Changes: - render.ts: prefix all node domIds before layout - edges.js: prefix edge path IDs with diagram ID - createGraph.ts: prefix edge label node IDs - flowDb.ts/classDb.ts: add setDiagramId(), defer click handler domId lookup to bind time so prefixed IDs are used - flowRenderer/classRenderer: call setDiagramId() before getData() - flowRenderer: fix link selector to use domId instead of id Affects all diagram types that go through the unified render path: flowchart, class, state, ER, requirement, mindmap. https://claude.ai/code/session_01FPVnyf54nFQNQnhZ7dSWCE
Extract addFlowVertex helper to deduplicate verbose addVertex calls. Remove three describe blocks that only tested local mock functions (string concatenation), keeping all tests that exercise real FlowDB/ClassDB code and the full collision simulation. https://claude.ai/code/session_01FPVnyf54nFQNQnhZ7dSWCE
- Rename setDiagramId param from `id` to `svgElementId` for clarity - Add setDiagramId to DiagramDB interface to centralize the contract - Remove stale "defer lookUpDomId" comments in classDb.ts - Remove PR mermaid-js#4825 reference from render.ts comment https://claude.ai/code/session_01FPVnyf54nFQNQnhZ7dSWCE
…base class Extract shared `diagramId` field and `setDiagramId` setter from FlowDB and ClassDB into a new abstract base class `ScopedDiagramDB`. Both DB classes now extend it instead of duplicating the field and method. https://claude.ai/code/session_01FPVnyf54nFQNQnhZ7dSWCE
…agramDB base class" This reverts commit 7245574.
Fix unscoped element IDs in clusters.js (5 locations), defaultMindmapNode.ts, and kanbanRenderer.ts that caused duplicate DOM IDs when multiple diagrams with identical node names appeared on the same page. Add self-enforcing integration test that renders two identical diagrams for every registered diagram type and asserts no duplicate element IDs. The test auto-detects new diagram types via the registry and fails if they're not covered. Legacy renderers with known issues use it.fails to document them. Add runtime duplicate-ID warning in mermaid.run() (debug log level only) with a pre-filled GitHub issue link for easy reporting. Add Cypress tests for browser-level multi-diagram ID uniqueness verification. https://claude.ai/code/session_01QCm1SAitm8ZpLjjsk7eFv3
These type declaration files are generated by the prepare script during pnpm install. Adding them to avoid untracked file warnings. https://claude.ai/code/session_01QCm1SAitm8ZpLjjsk7eFv3
This reverts commit 10002cf.
New diagram types are added ~once per 5 months. The 4-category test taxonomy (unified/simple/legacy/jsdom-incompatible), self-enforcement registry check, and runtime duplicate-ID warning were not worth the maintenance cost. Simplified to a flat list of diagram tests that covers all types where IDs should be unique. https://claude.ai/code/session_01QCm1SAitm8ZpLjjsk7eFv3
Remove `|| node.id` fallback from clusters.js — every code path that calls insertCluster (dagre, cose-bilkent, kanban) already sets domId. The fallback silently hid missing domId bugs instead of surfacing them. Extract assertNoDuplicateIds into cypress/helpers/util.ts and use it in both marker_unique_id and multi_diagram_unique_ids specs. https://claude.ai/code/session_01QCm1SAitm8ZpLjjsk7eFv3
Prefix all hardcoded SVG marker IDs (arrowhead, filled-head, crosshead, sequencenumber, etc.) and element IDs with the diagram's unique ID in sequence, journey, timeline, gantt, and C4 renderers. This prevents cross-diagram ID collisions when multiple diagrams render on the same page. Covers: sequence (12 markers), journey (arrowhead), timeline (arrowhead), C4 (arrowhead, arrowend, filled-head, crosshead, sequencenumber, plus database/computer/clock symbols), and gantt (task + exclude-day IDs). https://claude.ai/code/session_01QCm1SAitm8ZpLjjsk7eFv3
…uniqueness tests Extends the parametrized renderTwoAndCheckIds test suite to cover all 5 diagram types that previously had hardcoded marker/element IDs. Also fixes a pre-existing bug in timeline's svgDraw.js where node IDs were `node-undefined` (now uses a monotonic counter prefixed with diagram ID). All 19 diagram types now pass the duplicate-ID stress test. https://claude.ai/code/session_01QCm1SAitm8ZpLjjsk7eFv3
…types Adds a meta-test that cross-references the detector registry against the test map, so any new diagram type added to mermaid will fail CI unless it has a corresponding ID uniqueness test (or is explicitly excluded with a justification). Also documents block and architecture as known-failing (pre-existing ID collision bugs tracked via it.fails), and excludes mindmap (cytoscape JSDOM limitation, uses unified pipeline so IDs are correct). https://claude.ai/code/session_01QCm1SAitm8ZpLjjsk7eFv3
Exercises the ID scoping mechanism under adversarial conditions: - 5x identical diagrams for 10 different diagram types (flowchart, class, ER, state, sequence, gantt, pie, C4, journey, timeline) - Complex graphs with 20 nodes and fan-out topologies - Nested subgraphs (3 levels deep) - Mixed diagram types on the same page (up to 10 types simultaneously) - FlowDB/ClassDB unit-level stress with 10-100 instances - Sequential render stability and clear/reset cycles - Adversarial node names that mimic diagramId prefixes - SVG marker definition uniqueness (sequence, C4) - Edge label ID uniqueness across renders - Kanban, git graph, requirement, XY chart, quadrant, sankey - Diagram ID prefix propagation verification https://claude.ai/code/session_01MtKnkbaUyKZY6R5vTrMNam
The journey diagram's svgDraw.js used bare `task0`, `task1`, etc. as element IDs without any diagram-scoped prefix. This worked by accident because the module-level taskCount never reset, but was fragile and inconsistent with the ID scoping pattern used by all other diagram types. Fix: - Store the diagramId passed to initGraphics() - Reset taskCount on each render via initGraphics() - Prefix task line IDs with diagramId (e.g. `mermaid-0-task0`) https://claude.ai/code/session_01MtKnkbaUyKZY6R5vTrMNam
Add a focused regression test that verifies journey diagram task line
IDs are scoped with the diagramId prefix. This test fails against the
pre-fix code (bare "task0"/"task1" IDs) and passes after the fix
("journey-a-task0"/"journey-b-task0").
Remove the large stress test file — the targeted test plus the existing
multi-diagram-id-uniqueness suite provide sufficient coverage.
https://claude.ai/code/session_01MtKnkbaUyKZY6R5vTrMNam
Claude/stress test pr ms jh o
…-graphs-gMtbB Claude/unique ids mermaid graphs g mtb b
Add 43 stress tests covering scenarios beyond the basic two-diagram tests: - Scale: 10 and 20 identical diagrams of 11 different types (flowchart, class, sequence, journey, timeline, gantt, C4, state, ER, pie, git) - Cross-type: mixed diagram types rendered into a single container - Subgraphs/clusters: nested and multi-level subgraph ID isolation - Large diagrams: 50-node flowcharts, 20-message sequences, 20-class diagrams, 15-task journeys - Minimal diagrams: single-node/single-message edge cases - DiagramId boundaries: hyphenated, underscored, numeric-prefixed IDs - Sequential re-rendering: clear-and-rerender and append-without-clear - Module-level counter resets: journey taskCount and timeline nodeCount - SVG marker/defs scoping: sequence, flowchart, and C4 markers - DB-layer scoping: FlowDB and ClassDB lookUpDomId under 5x stress - Kanban pre-flight domId injection - Gantt special characters in task IDs All 43 tests pass, confirming the ID uniqueness fix holds under stress. https://claude.ai/code/session_012gG2dXNE8BJAZfHJjs96aX
1. flowDb.ts lookUpDomId fallback: when called with an ID not in the
vertex map (e.g. subgraph IDs), the fallback now applies the
diagramId prefix instead of returning the bare ID.
2. sequence/svgDraw.js drawActorTypeControl: remove the `|| ''`
fallback on conf.diagramId that produced colliding marker IDs
(e.g. "-filled-head-control") when multiple sequence diagrams
share a page. The renderer always sets conf.diagramId before
rendering, so the fallback was masking a potential collision.
3. Eliminate module-level diagramId variables in sequence, journey,
and timeline renderers to prevent race conditions in concurrent
or SSR rendering scenarios:
- sequenceRenderer.ts: use conf.diagramId instead of redundant
module-level diagramId variable
- journey/svgDraw.js: pass diagramId as parameter to drawTask
instead of reading from module scope
- timeline/svgDraw.js: pass diagramId as parameter to drawTask,
drawNode, and defaultBkg; also fixes timeline drawTask which
was missing the diagramId prefix entirely ("task0" vs
"diagramId-task0")
- timeline/timelineRenderer.ts: pass diagramId through drawTasks
and drawEvents instead of reading from module-level
currentDiagramId
Adds 5 regression tests that would have failed before these fixes.
https://claude.ai/code/session_012gG2dXNE8BJAZfHJjs96aX
lookUpDomId in flowDb and classDb, and createGraph's edge-label domId, all had `diagramId ? prefixed : bare` ternaries. Since the render pipeline always calls setDiagramId before any lookups, the bare-ID fallback just silently masked missing diagramId bugs. Now these always prefix, making a missing setDiagramId call surface immediately. https://claude.ai/code/session_012gG2dXNE8BJAZfHJjs96aX
Stress test PR changes
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
github-actions
bot
added
the
Type: Bug / Error
@mermaid-js/examples
mermaid
@mermaid-js/layout-elk
@mermaid-js/layout-tidy-tree
@mermaid-js/mermaid-zenuml
@mermaid-js/parser
@mermaid-js/tiny
commit: |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
The `|| node.id` / `|| vertex.id` / `: edge.id` fallbacks are dead code — render.ts always sets domId before shape/edge functions run. Worse, if reached, they'd silently produce unscoped IDs, reintroducing the duplicate-ID bug this PR fixes. Changes: - defaultMindmapNode.ts: throw on missing domId, use node.domId directly - flowRenderer-v3-unified.ts: throw on missing domId, use vertex.domId - edges.js: rename `id` param to `diagramId`, throw if missing, drop ternary fallback to bare edge.id
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📑 Summary
When multiple Mermaid diagrams share a page, identical SVG element IDs (nodes, edges, edge labels, markers, task lines, sequence lifelines, etc.) collide across
<svg>containers. This causes browsers to bind event handlers, arrow markers, and CSS selectors to the wrong elements. Such incorrect binding breaks click callbacks, makes arrowheads disappear, and corrupts styling.This PR extends the namespacing approach introduced in #4825 (which scoped marker IDs per graph) to all internally generated SVG element IDs across every diagram type. Each ID is prefixed with its diagram's unique SVG container ID, guaranteeing no collisions regardless of how many diagrams appear on a page.
Related PRs:
Related issues:
bindFunctionsinteractions only work on one diagram instance when multiple are on the same pagerevealjs+mermaidjsarrows missing on 4th+ diagram (same root cause)📏 Design Decisions
Following the precedent set by #4825: that merged PR scoped marker IDs by prefixing them with the graph container ID. We apply the same namespacing strategy comprehensively — to node IDs, edge IDs, edge-label IDs, cluster IDs, task-line IDs, sequence lifeline/actor IDs, timeline section IDs, C4 element IDs, and journey task IDs.
Implementation approach:
diagramIdfield +setDiagramId()toFlowDBandClassDB; added to theDiagramDBinterface intypes.tsso any diagram DB can opt insetDiagramId(id)with the SVG container ID before layoutlookUpDomId()in FlowDB/ClassDB returns${diagramId}-${originalId}render.ts,createGraph.ts,edges.js,clusters.js) prefixes IDs for diagrams routed through the shared renderersvgDraw/ renderer filesdiagramIdresets onclear()to prevent state leakagemulti_diagram_unique_ids.spec.js) + unit tests (unique-dom-ids.spec.ts,multi-diagram-id-uniqueness.spec.ts) covering all diagram types and collision scenariosBreaking change
Marker IDs (e.g.
arrowhead,crosshead) are now prefixed with the diagram's SVG element ID. Custom CSS or JS using exact ID selectors like#arrowheadshould use attribute-ending selectors like[id$="-arrowhead"]instead.📋 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:.