Skip to content

Fix duplicate IDs in general#1

Closed
alexander-turner wants to merge 25 commits intodevelopfrom
claude/fix-mermaid-duplicate-ids-eQ6BY
Closed

Fix duplicate IDs in general#1
alexander-turner wants to merge 25 commits intodevelopfrom
claude/fix-mermaid-duplicate-ids-eQ6BY

Conversation

@alexander-turner
Copy link
Copy Markdown
Owner

@alexander-turner alexander-turner commented Feb 19, 2026

📑 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:

  • #5741 — Duplicated IDs for markers (still open; stale, and without test coverage; this PR should obsolete its changes)
  • #6621 — Open PR addressing duplicate ID generation in fast multi-block renders (complementary; targets container IDs, not element IDs)
  • #4825 — Merged PR that gave markers unique IDs per graph (our approach extends this pattern to all element types)

Related issues:

  • #4346bindFunctions interactions only work on one diagram instance when multiple are on the same page
  • #1318 — Duplicate IDs across diagrams have potential to break some diagrams
  • #3267 — Generated SVG has static IDs which cause issues with multiple SVGs
  • #3433revealjs + mermaidjs arrows 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:

  • Added diagramId field + setDiagramId() to FlowDB and ClassDB; added to the DiagramDB interface in types.ts so any diagram DB can opt in
  • Each v3-unified renderer calls setDiagramId(id) with the SVG container ID before layout
  • lookUpDomId() in FlowDB/ClassDB returns ${diagramId}-${originalId}
  • The generic rendering path (render.ts, createGraph.ts, edges.js, clusters.js) prefixes IDs for diagrams routed through the shared renderer
  • Diagram-specific renderers (sequence, journey, timeline, gantt, C4, kanban) prefix IDs in their own svgDraw / renderer files
  • diagramId resets on clear() to prevent state leakage
  • Cypress E2E test (multi_diagram_unique_ids.spec.js) + unit tests (unique-dom-ids.spec.ts, multi-diagram-id-uniqueness.spec.ts) covering all diagram types and collision scenarios

Breaking changes

  • 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 #arrowhead should use attribute-ending selectors like [id$="-arrowhead"] instead.
  • The first 13 changed Argos snapshots should be accepted. Explanation: CSS selectors like #statediagram-barbEnd and #compositionStart were changed to [id$="-barbEnd"] and [id$="-compositionStart"]. The old selectors were dead code — since PR Give markers unique id's per graph mermaid-js/mermaid#4825, marker IDs have been in the format ${diagramId}_${type}-markerName, so the bare ID selectors like #statediagram-barbEnd never matched. The new attribute selectors correctly apply marker styles that were always intended but never worked.

📋 Tasks

Make sure you

  • 📖 have read the contribution guidelines
  • 💻 have added necessary unit/e2e tests.
  • 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
  • 🦋 If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Note: shape rendering should have unit tests

The packages/mermaid/src/rendering-util/rendering-elements/shapes/ directory has essentially no unit tests. Shape functions are only exercised via E2E visual tests, which always go through the full render pipeline. This means subtle bugs in how shapes handle domId vs id can go undetected until visual regression catches them (as happened with erBox.ts in this PR).

claude and others added 25 commits February 19, 2026 17:27
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
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
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
…-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
@alexander-turner alexander-turner changed the title Claude/fix mermaid duplicate ids e q6 by Fix duplicate IDs in general Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants