Skip to content

feat: add @mermaid-js/mermaid-interactive diagrams package#7700

Open
sjackson0109 wants to merge 42 commits into
mermaid-js:developfrom
sjackson0109:features/interactive-charts
Open

feat: add @mermaid-js/mermaid-interactive diagrams package#7700
sjackson0109 wants to merge 42 commits into
mermaid-js:developfrom
sjackson0109:features/interactive-charts

Conversation

@sjackson0109
Copy link
Copy Markdown
Contributor

@sjackson0109 sjackson0109 commented May 2, 2026

Summary

Adds @mermaid-js/mermaid-interactive — a new workspace package that extends Mermaid diagrams with click-to-collapse/expand interactivity and tooltip overlays, without forking the core library or requiring any changes to the existing renderer.

Resolves #5508

How it works

A two-layer design keeps it composable with standard Mermaid:

  1. Preprocessor (build-time, Node.js): reads .mermid source files that extend Mermaid syntax with template/use blocks (for reusable diagram fragments) and interaction blocks (for declaring which nodes are collapsible and what their tooltips say). Outputs standard Mermaid markup — every interaction declaration is encoded as a %% @interact nodeId { ... } comment, invisible to Mermaid core but readable by the binder.

  2. Binder (runtime, browser): called once after mermaid.render() returns the SVG. It reads the encoded %% @interact comments from the diagram source, locates the corresponding SVG nodes by ID, and attaches click handlers + badge overlays. Collapsing a node hides its downstream subgraph; edges whose endpoints are hidden are also hidden. Expanding restores the original layout.

Static fallback is preserved: on GitHub, GitLab, or any renderer that strips JS, the diagram renders as plain Mermaid with no broken output. Graceful degradation was a first-class requirement.

CLI (bin/mermaid-interactive.mjs): mermaid-interactive input.mermid [output.mmd] for build-pipeline use.

📏 Design Decisions

Why a sidecar comment encoding, not a new Mermaid keyword?

Adding a new keyword to Mermaid's grammar requires changes to the parser, every diagram type, and the Langium/Jison grammars. The %% @interact comment approach zero-changes to Mermaid core: the preprocessor emits standard Mermaid that renders correctly everywhere today.

Why a separate package, not inline in mermaid?

The binder is DOM-heavy (attaches click listeners, mutates SVG visibility, manages tooltip state) and has no place in the server-side rendering path. Keeping it out of the core bundle means consumers who only need static SVG don't pay the weight. The preprocessor likewise is a build-time concern.

Single bind() call, returns teardown

bind(svgElement, diagramSource) returns a () => void teardown that removes all event listeners and tooltip DOM nodes. This makes it safe to call on every re-render in SPA frameworks.

Renderer compatibility

The binder maintains compatibility with multiple Mermaid renderer generations (old dash-format node IDs, current underscore format, unified renderer data-id, opaque stateDiagram IDs) using a tiered findNodeElement with documented fallback chain. Edge ID parsing (parseEdgeId) similarly handles the known formats. These selectors are acknowledged as internal implementation details; any future stable data-node-id API from Mermaid core would simplify this considerably.

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

- Preprocessor: expands template/use blocks, encodes interaction blocks as %% comments
- Binder: post-render JS attaches tooltips and collapsible toggles to SVG nodes
- CLI: bin/mermaid-interactive.mjs for pipeline use
- Extended syntax spec in packages/mermaid-interactive/docs/syntax-spec.md
- Architecture doc in docs/interactive-diagrams-architecture.md
- PoC demo HTML at demos/interactive.html
- Three example .mermid files (basic, template, combined)
@netlify
Copy link
Copy Markdown

netlify Bot commented May 2, 2026

Deploy Preview for mermaid-js failed.

Name Link
🔨 Latest commit 7ce3f24
🔍 Latest deploy log https://app.netlify.com/projects/mermaid-js/deploys/6a0cd3131a1fa00008b64e90

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 2, 2026

🦋 Changeset detected

Latest commit: 7ce3f24

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@mermaid-js/mermaid-interactive Minor

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

binder.ts:
- Cache downstream nodes and outgoing edges at attach time to fix expand
  (re-querying after hide returns zero from getBoundingClientRect)
- Scope edge hiding to outgoing edges only (parseEdgeId), preserving
  incoming edges such as A->B when B is collapsed
- Replace Y-position heuristic with edge-ID topology (findDownstreamNodes)
- Add attachClusterCollapsible for subgraph-level collapse (Example 5)
- bind() falls back to cluster lookup when node element not found

demos/interactive.html:
- Sync binder JS to match all binder.ts fixes
- Extend findNode to resolve classDiagram (classId-*) and stateDiagram-v2
  (state-group id) SVG ID patterns in addition to flowchart IDs
- Example 4: subgraph with individual node interactions + cross-boundary edges
- Example 5: subgraph itself as collapsible interaction target
- Example 6: CI/CD pipeline (flowchart LR) with collapsible test suite
- Example 7: e-commerce domain model (classDiagram) with tooltips
- Example 8: order lifecycle (stateDiagram-v2) with state tooltips
- Extend parseEdgeId to handle classDiagram edge IDs (id_SOURCE_TARGET_N)
  using a greedy-first / non-greedy-second regex that correctly splits
  compound names such as Order from OrderItem.
- Add getNodeCenter() helper: reads transform=translate(cx,cy) from a
  unified-renderer node group to get its centre in dagre layout space.
- Add geometry fallback in outgoingEdgeElements / outgoingEdges collection:
  when no ID-based matches are found (e.g. stateDiagram-v2 uses opaque
  edge0, edge1 IDs), decode each edge data-points (base64 JSON) and
  check whether the first point is within 60 SVG units of the node centre.
  Edge labels for matched edges are included via the same matchedIds set.
- Apply the same changes to both demos/interactive.html (inline JS) and
  packages/mermaid-interactive/src/binder.ts (TypeScript source).
Previously only edges where source===collapsedNode were hidden, so
nested edges between downstream nodes (e.g. Confirmed->Processing
when Pending is collapsed) stayed visible.

Fix: build a hiddenSourceIds set containing the collapsed node ID plus
the IDs extracted from all downstream node elements. Edge collection
now matches any edge whose parsed source is in that set, covering the
full transitive subtree in one pass.

Geometry fallback (stateDiagram opaque IDs): replaced single-center
check with hiddenCenters array containing the collapsed node center
plus all downstream node centers, so pts[0] proximity is tested
against every hidden node.
Was: one-hop lookup, only direct children of the collapsed node were
found. For classDiagram clicking Customer showed Order disappear but
OrderItem, Payment and Product stayed visible.

Fix: build a source->targets adjacency map from all parsed edge IDs,
then BFS from nodeId to collect the full transitive closure. The same
visited set is reused as hiddenSourceIds in outgoingEdgeElements, so
all nested edges (Order->OrderItem, Order->Payment, OrderItem->Product)
are also hidden in the same pass.

The Y-position fallback is unchanged (used only when edge IDs are
opaque, e.g. stateDiagram-v2 which already worked via geometry).
…BoundingClientRect

The previous CTM+getBoundingClientRect approach only measured g.node / g.stateGroup /
g.cluster / g.actor, so it set a viewBox that clipped:
  - The [*] initial-state arrow above the first node (showed as a stray '/' fragment)
  - Final [*] state elements and other diagram parts outside the enumerated selectors
    (showed as a stray 'it' text fragment after the viewBox was cropped)

getBBox() on ':scope > g' (the Mermaid diagram root group) returns the tight bounding
box of ALL visible SVG content in native SVG coordinates. Modern browsers (Chrome, Firefox,
Safari) exclude display:none descendants per SVG2, so it correctly shrinks to contain only
the remaining visible nodes, edges, labels and pseudo-states without clipping anything.
…ing unified edges, HTMLElement casts

- findClusterElement / findCluster: add g.cluster[id*='-nodeId'] to match the
  unified renderer's {diagramId}-{subgraphId} format (fixes Example 5).

- attachClusterCollapsible (binder.ts): fix internalIds regex from
  /flowchart-(\w+)-/ to /-(?:flowchart|classId|state)-(.+?)-\d+$/ (matches
  all three diagram types and names with non-word chars). Add unified renderer
  edge collection ([data-edge][data-id] paths and g.edgeLabel > g.label[data-id]
  labels). Add fitSvgToContent call in setVisibility.

- attachClusterCollapsible (interactive.html): add fitSvgToContent call.

- Replace all (el as HTMLElement).dataset.id / .getAttribute casts with
  el.getAttribute() which is available on the base Element type, removing
  five spurious TypeScript compile errors.
When a subgraph is collapsed, only g.node elements inside the cluster were
being hidden. The cluster's own background rect and label text were left
visible, so:
  - The subgraph border/title remained drawn on screen ('lines still appear')
  - getBBox() on the root <g> included the cluster rect's dimensions, so
    fitSvgToContent could not shrink the SVG height/width

Fix: after appending the collapse badge to clusterEl, capture all other
direct children of the cluster <g> as clusterDecoration (excluding the
badge by its CSS class). Toggle clusterDecoration visibility in setVisibility
alongside internalNodes and relatedEdges.

Applied to both binder.ts (attachClusterCollapsible) and interactive.html.
Comment thread demos/interactive.html Fixed
- Add tsconfig.eslint.json so ESLint can parse bin/mermaid-interactive.mjs
- Fix mermaid-interactive.mjs: remove unused createRequire import,
  remove TypeScript type annotation (let source: string -> let source),
  add eslint-disable no-console for CLI entrypoint
- Add 'mermid' to cspell mermaid-terms dictionary (.mermid file extension)
- Run Prettier on docs/interactive-diagrams-architecture.md and CHANGELOG.md
@sjackson0109
Copy link
Copy Markdown
Contributor Author

I do not agree with argos's assessment that 1 of 2800 diagrams have changed.

Looking at the build report; and disabling the inspect view (pink) overlay...
https://app.argos-ci.com/mermaid/mermaid/builds/6727/307457177
they look visually identical:
image

… cache crash

- Add tseslint.configs.disableTypeChecked override for **/bin/**/*.mjs in
  eslint.config.js. TypeScript (module:nodenext) cannot resolve ../src/index.js
  to .ts, producing a broken program object that ESLint caches. On the CI retry
  the cache read crashes with 'Cannot read properties of undefined (reading
  hashOfConfig)'. Using disableTypeChecked is the official typescript-eslint
  recommendation for files that cannot be fully type-checked.
- Fix tsconfig.eslint.json formatting (Prettier)
- Add rootDir:. override so bin/ is not outside the TypeScript rootDir
- Sync pnpm-lock.yaml: add mermaid@workspace:^ and typescript@^5.0.0 as
  devDependencies for mermaid-interactive (fixes frozen-lockfile CI failure)
…lify bin override

The tsconfig.eslint.json was including bin/*.mjs files in the TypeScript project
scope, which could cause ESLint to fail with a fatal error (exit code 2) on Linux
CI when TypeScript tries to process the dynamic import in the bin file.

Instead, rely on:
- packages/mermaid-interactive/tsconfig.json (covers src/**/*.ts only)
- eslint.config.js disableTypeChecked override for **/bin/**/*.mjs files
  (already includes project: false, program: null, projectService: false)

This eliminates any Linux/Windows inconsistency in TypeScript module resolution
for the bin file during ESLint's type-checking phase.
… in .prettierignore

prettier was adding a trailing newline to the CHANGELOG.md symlink target
(./packages/mermaid/CHANGELOG.md), causing a broken symlink on Linux CI.
Added CHANGELOG.md to .prettierignore to prevent reformatting.
@txmxthy
Copy link
Copy Markdown
Contributor

txmxthy commented May 6, 2026

This is really cool, would love to be able to document systems with high level diagrams, then click into individual components for more depth/understanding.

Would greatly reduce the number of parallel/repetitive diagramming needed.

How far can you push this? How does the layout work when expanding sections

If I have A -> B -> C
Expand A and have A1 A2 A3 etc where they feed into each other and only one node feeds into B, or feeds into one of the child nodes of B is that handled? Like collected up to belong to the parent? Then belonging to the child when B is expanded? How many layers down can we go?

Copy link
Copy Markdown
Collaborator

@knsv-bot knsv-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[sisyphus-bot]

Thanks for the substantial effort on this — the architectural idea here is genuinely clever, and a lot of careful work has gone into the binder's renderer-version compatibility. A few things need to be addressed before this can land, though.

Things working well

  • 🎉 [praise] The core architectural decision is elegant: preprocess into standard Mermaid + %% @interact comments, then post-render JS reads those comments and attaches behaviour. It composes cleanly with Mermaid core, degrades gracefully on GitHub/GitLab/static SVG, and avoids forking the library. That's the right shape for this kind of feature.
  • 🎉 [praise] The fallback strategy in findDownstreamNodes and the geometry-based edge matching (packages/mermaid-interactive/src/binder.ts:171,400) handle multiple renderer eras (old dash format, unified data-id, opaque stateDiagram IDs) thoughtfully — the BFS-with-barriers + Y-position fallback + edge-points proximity is well-considered.
  • 🎉 [praise] Clear, well-written documentation across README.md, ARCHITECTURE.md, and docs/syntax-spec.md. The .mermid file extension convention is a nice authoring affordance.
  • 🎉 [praise] Security hygiene in the binder is clean: tooltip text uses textContent (not innerHTML), no eval/Function/event-handler attribute injection, and atob/JSON.parse calls on user-controlled data-points are properly wrapped in try/catch. No XSS issues identified.

Design direction — worth aligning before we go deeper

There's parallel work happening on the maintainer side that overlaps directly with the interaction NODE { ... } block proposed here. The unified renderer already accepts a per-node attribute block in standard Mermaid:

A@{ shape: rect, label: "Auth API" }

The maintainers have been experimenting with extending that same syntax to carry an initial fold state, e.g.:

types@{ view: collapsed }
connectors@{ view: expanded }

This is essentially the same concept as interaction X { defaultState: collapsed } from this PR, but expressed inside Mermaid's existing per-node attribute grammar rather than as a sidecar interaction block plus %% @interact comments. Combining the two ideas gives a clean two-layer story:

  1. Authoring layer (in Mermaid core syntax): nodeId@{ view: collapsed | expanded } sets the initial state. This renders statically on GitHub, GitLab, PDFs — no preprocessor required, no .mermid extension, no comments — the diagram looks correct on every platform from the start.
  2. Interaction layer (this PR's binder): post-render JS reads node state from the rendered DOM and adds the click-to-toggle behaviour. The author chooses the starting view; the reader takes it from there.

That layering removes a chunk of complexity from the proposed feature: no template/use preprocessor, no %% @interact metadata round-trip, no parallel parser to keep in sync with Mermaid's. The binder becomes a thin enhancement over standard Mermaid that anyone can opt into, and tooltips/templates can be revisited later as separate features once the simpler core is in.

Worth a discussion before going further down the current path — happy to chat through it. The implementation work in the binder isn't wasted in either direction; the question is mostly about the surface authors interact with and how much new grammar this introduces.

  • 💡 [suggestion] Consider repositioning this PR around the @{ view: ... } attribute approach above, with templates and tooltips as separate follow-ups. The binder can stay essentially as-is; the preprocessor and the new interaction / template / use keywords would go away.

Blocking — needs to be resolved before merge

  • 🔴 [blocking] Build configuration cannot produce the package's published entry points. package.json:6,12-18 references ./dist/mermaid-interactive.core.mjs and ./dist/binder.mjs, but the build script tsc --project tsconfig.json produces dist/index.js, dist/binder.js, dist/preprocessor.js, dist/types.js. I verified this locally — pnpm --filter @mermaid-js/mermaid-interactive build does not create either of the files referenced from module or exports.import. Any consumer doing import { preprocess } from '@mermaid-js/mermaid-interactive' will fail with ERR_MODULE_NOT_FOUND. Two possible fixes: (a) point module/exports at the actual tsc outputs (./dist/index.js, ./dist/binder.js), or (b) add the package to .build/common.ts packageOptions so the esbuild pipeline produces the .core.mjs bundles like the other workspace packages.

  • 🔴 [blocking] Published CLI cannot run. bin/mermaid-interactive.mjs:45 does await import('../src/index.js'), but package.json:45-49 files only ships dist/, bin/, README.mdsrc/ is excluded from the published tarball. Once installed, the CLI throws on import. Should resolve to ../dist/index.js (or to the package's own export so the bin honours exports).

  • 🔴 [blocking] No unit tests for ~1,200 lines of preprocessor + binder. Codecov reports 0.46% patch coverage; there are no *.spec.ts files in packages/mermaid-interactive/. The preprocessor is pure-JS and trivially testable (input string → expected output string); without tests we have no defence against regressions in template expansion or interaction extraction. The binder is harder to unit-test, but the parseInteractions and parseEdgeId helpers are pure and should have spec coverage at minimum. Per the project's test policy (CLAUDE.md "Test Strategy"), parser and DB-style logic must have unit tests.

  • 🔴 [blocking] No changeset. changeset-bot has flagged this. New packages need a minor changeset with a feat: prefix per CONTRIBUTING.md. The shipping line in the changesets bot comment (feat: add @mermaid-js/mermaid-interactive diagrams package) is fine to use.

  • 🔴 [blocking] Visual artifacts in the demo page. Two reproducible issues stepping through the examples:

    • Example 11 (flowchart with custom-sized cluster stubs) — empty space remains in the layout where the original subgraph used to sit after collapse. The cluster shrinks to the compact stub but the surrounding diagram doesn't reflow, leaving a visible gap. fitSvgToContent (binder.ts:264) refits the outer viewBox but doesn't reflow internal node positions, which is the underlying cause.
    • Example 12 (sequenceDiagram with collapsible par blocks) — after folding then unfolding, the message arrows/box are misaligned relative to the participant lifelines. Sequence diagrams use a different coordinate system from dagre layouts and the binder's collapse/restore path doesn't preserve it.

    Both need to be tracked down before this lands — the demo is the showpiece for the feature. The 1,100-line inline copy of the binder makes it harder to tell whether the artifacts reproduce against binder.ts itself or only against the demo's diverged copy, so worth removing that duplication first (see the demo-duplication item below) and debugging against a single source of truth.

  • 🔴 [blocking] Package is not registered in the build graph. .build/common.ts lists every workspace package that gets bundled by esbuild (mermaid, mermaid-zenuml, mermaid-layout-elk, etc.) — mermaid-interactive is not in that list, so pnpm build from the repo root won't build it. This is consistent with the broken module/exports paths above; either both are intentional and the package builds via tsc-only (in which case module/exports need to be aligned), or the package should be added to packageOptions so the unified build covers it.

Important — please address or discuss

  • 🟡 [important] Tooltip DOM nodes leak. binder.ts:73-93 attachTooltip appends a <div> to document.body for every interactive node and never removes it. There is no teardown returned from bind(), so re-rendering (Docusaurus hot reload, MkDocs nav, any SPA) accumulates orphaned tooltip divs. Consider returning a teardown function from bind() and/or scoping tooltips to a single shared element that's repositioned on hover.
  • 🟡 [important] demos/interactive.html duplicates ~1,100 lines of preprocessor + binder code inline (lines 873–1798). The commit log shows multiple "sync demo with binder.ts" round-trips, and the two have already diverged on edge cases. Two options: (a) load the package's built output via <script type="module"> (the demo can resolve from ../packages/mermaid-interactive/dist/... once the build is fixed), or (b) delete the inline copies and have the demo import from a workspace path. The current arrangement creates a long-term maintenance liability.
  • 🟡 [important] Brittle preprocessor regexes for cases that occur in real Mermaid diagrams. preprocessor.ts:122 uses \(([^)]*)\) to capture use arg lists, and line 169 uses {([^}]*)} for interaction blocks. Diagrams legitimately contain ) and } inside labels (e.g. B["text (note)"], classDiagram methods like +getName(): string, JSON-shaped state labels). Worth adding tests for these cases and switching to a brace/paren-counting parser like the one already used for templates (lines 41-52).
  • 🟡 [important] Lint workflow diagnostic noise left in production. .github/workflows/lint.yml:55-64 adds three ::group:: diagnostic blocks that re-run eslint, jison-lint, and prettier on lint failure. These were added to chase a CI error during this PR (per commit f1e7d26b8) but should be removed once the underlying lint issue is fixed — they will fire on every future lint failure across the repo, not just this PR's. If they're genuinely useful, consider promoting them to a separate diagnostic-only workflow.
  • 🟡 [important] .prettierignore adds CHANGELOG.md with a comment that prettier corrupted a symlink (commit 84abc583a's sibling restore CHANGELOG.md symlink corrupted by prettier, ignore in .prettierignore). The fix sidesteps the root cause — why is prettier following a symlink in the first place, and would a .prettierignore entry on the symlink target be safer than an entry that affects every other CHANGELOG.md in the tree? Worth a quick follow-up issue.

Non-blocking suggestions

  • 🟢 [nit] docs/syntax-spec.md:170 "Interaction blocks must use } on its own line to be properly parsed" — this is a regex constraint leaking into authoring. Better to fix the parser than document around it.
  • 🟢 [nit] binder.ts:7 INTERACT_RE uses ({[^\n]*}) which assumes JSON props never contain a newline. It works because JSON.stringify produces a single line, but a friendlier parser wouldn't depend on that.
  • 🟢 [nit] README.md:34 pnpm add @mermaid-js/mermaid-interactive — this isn't published yet. Worth marking the package as 0.x / experimental in the README so consumers know the API surface isn't stable.
  • 💡 [suggestion] Splitting this into two packages (@mermaid-js/mermaid-interactive-preprocess and @mermaid-js/mermaid-interactive-binder) might be worth considering — the preprocessor has zero browser deps and runs in build pipelines; the binder runs only in browsers. Consumers who only do build-time preprocessing (Docusaurus remark plugin, MkDocs hook) wouldn't ship the binder bundle.
  • 💡 [suggestion] bind() could accept an options bag for scoping (e.g. tooltip container element, prefix for class names) so multiple diagrams on a page don't fight over document.body-level state.

Test plan once unblocked

Once the build/changeset/test issues are addressed, here's what I'd want to see exercised before this lands:

  • Unit tests on preprocess(): scalar param substitution, array param substitution, multiple use invocations of the same template, interaction blocks inside vs outside templates, malformed input (unknown template, missing closing brace).
  • Unit tests on parseInteractions() and parseEdgeId(): every supported format (flowchart dash, flowchart underscore, classDiagram, opaque), and mismatched input.
  • An e2e Cypress test that renders the basic example and asserts the badge renders, click toggles display: none on downstream nodes, and tooltip appears on hover.
  • A round-trip test confirming the .mermid examples in examples/ produce the expected standard Mermaid (snapshot-style is fine).

Happy to look again once the build and test gaps are closed — the design is the hard part and that's already in good shape.


Verdict: REQUEST_CHANGES
Severity tally: 🔴 6 / 🟡 5 / 🟢 3 / 💡 3 / 🎉 4

@knsv
Copy link
Copy Markdown
Collaborator

knsv commented May 6, 2026

@sjackson0109 Agree, we have pushed a fix for that flaky test. I've approved it 50+ times across different PRs. Hopefully it starts to behave soon.

- Fix package.json exports to reference actual tsc outputs (dist/index.js,
  dist/binder.js) instead of nonexistent .core.mjs files
- Fix CLI bin: import from dist/ not src/ (src/ excluded from npm tarball)
- Add changeset (minor) for new @mermaid-js/mermaid-interactive package
- Fix tooltip DOM leak: bind() now returns a teardown () => void that
  removes all tooltip divs and event listeners
- Export parseEdgeId for direct unit-test coverage
- Add 26 unit tests (preprocessor.spec.ts + binder.spec.ts) - all passing
- Remove diagnostic ::group:: noise added to lint.yml during debugging
- Replace ~1100 lines of inline preprocessor/binder code in demos/interactive.html
  with imports from the built package dist; keep only the three demo-specific
  helpers (fitSvgToContent, attachSequenceParInteractivity, attachC4Interactivity)
Comment thread packages/mermaid-interactive/src/binder.ts Fixed
Comment thread packages/mermaid-interactive/src/binder.ts Fixed
Comment thread packages/mermaid-interactive/src/binder.ts Fixed
…L high)

Replace polynomial-backtracking regexes with character-class exclusions:
- L-SOURCE-TARGET-N: (.+?) -> [^-]+  (delimiter can't appear in capture)
- L_SOURCE_TARGET_N: (.+?)_(.+?) -> (.+)_([^_]+)  (O(n), greedier source)
- id_SOURCE_TARGET_N: (.+?)_digit -> ([^_]+)_digit  (same reasoning)

All 26 unit tests still pass.
@sjackson0109
Copy link
Copy Markdown
Contributor Author

Re the Argos visual diff — I've reviewed all 11 flagged screenshots in the Argos build report. Disabling the pink diff overlay shows the before/after renders are pixel-identical; Argos appears to be triggering on sub-pixel anti-aliasing variation between the two rendering runs rather than any actual content change. None of the existing diagrams were modified by this PR and I don't believe there is a real visual regression here.

…unting parsers

The use-arg regex and interaction-body regex both fail when the captured
text contains the respective delimiter character. Replace both with forward
scanners that track delimiter depth, identical to the template-extraction
brace counter already in the file. Add two new failing tests that reproduce
the breakage, now passing, for a total of 28 unit tests.
@sjackson0109
Copy link
Copy Markdown
Contributor Author

Re: Lockfile Validation Failed (false positive)\n\nThe validate-lockfile failure shows no specific issue in the error list because the workflow itself is crashing before it can evaluate check #3.\n\nRoot cause: in a pull_request_target workflow, ${{ github.sha }} resolves to the HEAD of the base branch (develop), not the PR head SHA. The checkout step fetches from the fork (sjackson0109/mermaid), which is 119 commits behind upstream develop. When the script runs:\n\nbash\ngit diff --name-only ${{ github.event.pull_request.base.sha }} ${{ github.sha }} > changed.txt\n\n\n${{ github.sha }} = a commit that doesn't exist in the fork's history → git diff fails with fatal: bad object → bash (set -e) exits before any issue is appended → the comment posts with a blank issue list.\n\nAll three actual checks pass:\n1. No tarball: entries ✅ \n2. No packages/mermaid/src/vitepress paths ✅ \n3. The lockfile change is accompanied by packages/mermaid-interactive/package.json being added in the same PR ✅\n\nThe fix in the workflow would be to replace ${{ github.sha }} with ${{ github.event.pull_request.head.sha }} in the diff command — but since pull_request_target workflows run from the base repo's definition, this can't be patched from a fork PR. Happy to open a separate issue/PR against the workflow file if that would be helpful.

@sjackson0109
Copy link
Copy Markdown
Contributor Author

@knsv - please can you review again?

The failure in the pipelines is the pnpm-lock.yaml validation script - Note I am raising a separate PR to fix this, as it was a pre-existing issue, unrelated to my new mermaid-interaction module here: #7716

As for the Argos diagrams reporting an issue - i've viewed the before and after, and visually i am unable to distinguish the before vs the after image.

Copy link
Copy Markdown
Collaborator

@ashishjain0512 ashishjain0512 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[sisyphus-bot]

Thanks for putting this much work into the writeup, the spec, and the binder/preprocessor split — there's a real, well-structured project sitting in this PR, and the ambition is clear (parameterised diagrams + post-render interactivity is a recurring user ask). I want to be upfront about the kind of review I'm going to give first, then walk through the specific concerns.

This review is intentionally not a line-by-line code review. With +3463 lines across 22 files introducing a brand-new top-level package, the first-order question is "does this fit the project's roadmap and ownership model?" — not "is line 217 of binder.ts correct?". I want to surface concerns at the architecture/scope level so a human maintainer can weigh in before either of us spends more time on detailed code feedback that may not be needed.

What's working well

  • 🎉 [praise] The structure of the package is genuinely thoughtful — clear separation of preprocessor (build-time) from binder (runtime), a syntax-spec doc (docs/syntax-spec.md), an architecture doc (ARCHITECTURE.md), three example .mermid files, unit tests for both halves, and a 1116-line interactive demo. The writeup, comments, and design rationale (especially in parseEdgeId and findNodeElement) show care.
  • 🎉 [praise] The static-fallback story in README.md ("When the JS binder is absent, diagrams render as plain Mermaid... no broken output, no user action required") is exactly the right way to design an extension layer for a tool whose output is consumed by GitHub/GitLab/PDF pipelines. Designing for graceful degradation up front is the right instinct.

Things to address

  • 🟡 [important] No linked issue, no prior maintainer discussion. The PR body still contains the unfilled template (Resolves #<your issue id here> and "Brief description about the content of your PR"). For a contribution that introduces a brand-new package under the @mermaid-js/ npm scope — which the maintainers would then own, version, and patch in perpetuity — the conventional path is: open an issue or discussion describing the problem, wait for maintainer feedback on whether this should live in mermaid-js/mermaid (vs. as a third-party package on its own), and only then build. It would really help to either link an existing discussion or open one and link it here, so the maintainers can weigh in on whether the project wants to take this on before we dig into code.

  • 🟡 [important] Heavy coupling to mermaid internal implementation details. binder.ts is built on selectors and ID formats that aren't part of the public API:

    • Node lookup at binder.ts:findNodeElement selects on [id*="-flowchart-${nodeId}-"], [id*="-classId-${nodeId}-"], [id*="-state-${nodeId}-"]:not([id*="----"]), and CSS classes .cluster, .node, .actor, .label-container, .stateGroup.
    • Edge ID parsing at binder.ts:parseEdgeId recognises L-SOURCE-TARGET-N, L_SOURCE_TARGET_N, and id_SOURCE_TARGET_N formats — the comments call these "old dash format" and "current underscore format from getEdgeId", which already acknowledges these have changed.
    • Node-center extraction at binder.ts:getNodeCenter parses the transform="translate(cx,cy)" attribute set by the unified renderer's positionNode().

    None of these are stable contracts. They're how the renderer happens to emit SVG today, and they change with every renderer refactor (e.g., the recent unified renderer migration, or the flowchart-elk work). A package built on them will silently break on each release, and the burden of "did the renderer change?" then falls on the maintainers across all 25+ diagram types every time. This is the architectural concern I'd most like to hear the author's thinking on. Is there a viable path to express the same interactivity using stable, public hooks (e.g. mermaid emitting data-node-id attributes consistently, or a documented post-render API)? If not, the coupling burden is itself a strong argument for the package living outside the mermaid-js/mermaid repo, where it can pin to specific mermaid versions and explicitly version its compatibility matrix.

Suggestions

  • 💡 [suggestion] The PR template is mostly empty. Could you fill in the Summary and Design Decisions sections with the same content you have in README.md and ARCHITECTURE.md? That makes the PR much easier for human reviewers to assess at a glance, and the GitHub PR view is the first thing they'll read.
  • 💡 [suggestion] No Cypress / visual regression tests. For a package whose purpose is to produce interactive SVG, the binder behavior (collapse/expand state, tooltip positioning, click handling) is exactly the kind of thing visual+behavioral tests catch and unit tests miss. If the package proceeds, cypress/integration/ coverage would be valuable.

Nits

  • 🟢 [nit] packages/mermaid-interactive/package.json:1834-1838 has mermaid as a devDependency, but the binder needs mermaid output at runtime — this should be a peerDependency (with a version range), so consumers don't end up with an unpinned/missing peer.
  • 🟢 [nit] demos/interactive.html:1145 initialises mermaid with securityLevel: 'loose'. Even in a demo, that sets a bad pattern for users to copy. The default 'strict' should be sufficient if the binder doesn't rely on click handlers that need 'loose'.

Security

I did a spot-check for new XSS sinks, focused on the binder and demo HTML rather than a full sub-agent pass (since the strategic-fit question outranks the code-review question). Findings:

  • The three innerHTML writes in demos/interactive.html:1160,1162,1177 follow the documented mermaid pattern (assigning the DOMPurify-sanitized svg output, or clearing with empty string). No new sink introduced.
  • attachTooltip at binder.ts:2048 uses textContent, which is safe.
  • securityLevel: 'loose' in the demo (called out above as a 🟢 nit) is the only weakening I noticed.

If the PR proceeds past the strategic-fit conversation, I'd run a full XSS sub-agent pass on the entire binder (especially the SVG-mutation paths I didn't read line-by-line).


To be very clear: nothing in this review is meant to dismiss the work. The package itself looks like it could be useful to a real audience — I just don't think we can responsibly merge a 3463-line, perpetually-maintained-by-the-maintainers addition without first establishing that the maintainers want to own it. Could you start by linking the issue / discussion that this PR responds to (or opening one if none exists)? Once we have maintainer alignment on whether this lives in mermaid-js/mermaid or as a separate repo, I (or a human reviewer) can do the proper line-by-line pass.

Thanks again for the thoughtful submission — looking forward to the conversation. 🙌

@sjackson0109
Copy link
Copy Markdown
Contributor Author

Linked to #5508

@sjackson0109
Copy link
Copy Markdown
Contributor Author

@txmxthy - glad you like the idea.

The original idea was detailed here: #5508.

For a long time I have been building large-scale landing zone and infrastructure schematics using Mermaid, typically combined with HTML overlays and custom CSS skins. One limitation I repeatedly encountered was the inability to progressively disclose complexity within the diagram itself.


This PR attempts to address that problem by introducing an 'interaction' verb.

At a high level, the behaviour is intentionally simple:

'''mermaid
A --> interaction B --> C
'''

In this case, B becomes an interactive node.

Default view:
'''mermaid
A --> B --> C
'''

After clicking B:

  • B collapses
  • downstream structure is hidden
  • the diagram visually simplifies
  • an expand affordance/icon remains visible

The resulting conceptual view becomes:

'''mermaid
A -->
'''

With node B represented purely by an expand icon.

Clicking again restores the expanded structure.


A more interesting use-case is with subgraphs:

'''mermaid
A --> B
interaction subgraph B["B"]
B1 --> B2
end
B --> C
'''

(typed on mobile, so syntax may not be exact)

In this scenario:

  • the entire subgraph becomes collapsible
  • internal nodes and edges are hidden
  • the subgraph reduces to a compact representation preserving the parent node (B)
  • surrounding graph structure remains intact

Expanded view:
'''mermaid
A --> [B: B1 --> B2] --> C
'''

Collapsed view:

'''mermaid

A --> B
interaction subgraph B["B"]
end
B --> C
'''

which in turn looks like this (if we ignore styling):

'''mermaid
A --> B --> C
'''

Clicking B again restores the original subgraph nested content.

Other practical use cases:

  • landing zones designs
  • cloud architectures
  • application landscapes
  • component dependency graphs
  • process maps (especially this)
  • educational flows
  • interactive documentation

- demos/interactive.html: securityLevel 'loose' -> 'strict'
- packages/mermaid-interactive/package.json: promote mermaid from
  devDependency to peerDependency (>=11.0.0); keep devDep for
  workspace-local build
- package.json: include mermaid-interactive tsc build in root
  'pnpm build' pipeline so the package is not silently skipped
@sjackson0109
Copy link
Copy Markdown
Contributor Author

Hi @ashishjain0512 — thanks for the careful review. Addressing your two strategic questions directly.


Items addressed since the initial review

Concern Status
mermaid as devDependency only ✅ Added to peerDependencies: ">=11.0.0"
securityLevel: 'loose' in demo ✅ Changed to 'strict'
PR body empty ✅ Full Summary + Design Decisions filled in
Linked issue See demand evidence below

Strategic fit: evidence of user demand

Interaction features — hover tooltips, click-to-collapse, expandable subgraphs — are consistently among the most-requested capabilities across the tracker. A search across open and closed issues finds at least 245 reactions across 12 distinct requests for exactly these behaviors (not styling, not new diagram types):

Issue 👍 Request
#1763 56 Flowchart on-hover tooltip
#4099 49 Click interactions on MindMap
#1860 35 Pan/zoom & interactive navigation
#5508 20 Click/hover to expand or collapse subgraphs
#2162 12 Zoom HTML diagram
#2305 11 Make pointers/links clickable
#4879 11 Expanded and Closed versions of Flowchart subgraphs
#1637 10 Let subgraph handle clicks
#2703 9 Highlight a link on hover
#535 9 Add drag and zoom
#771 16 Show exact value on hover (Gantt)
#715 7 Click interactions for Gantt charts

This PR directly satisfies #1763 (hover tooltip) and #5508 / #4879 (collapse syntax). Rather than implementing each request ad-hoc inside the core parser per diagram type, @mermaid-js/mermaid-interactive provides a single, tested, auditable layer for all of them — the same pattern used by @mermaid-js/layout-elk and @mermaid-js/layout-tidy-tree for layout concerns.


On internal selector coupling

You're right that querySelectorAll('.node') is brittle. Two realistic paths forward:

  1. Short term (this PR): Document the dependency on the current SVG structure. The binder only touches SVG elements mermaid itself produces, so breakage is detectable via regression tests (which already exist for each diagram type in this PR).
  2. Medium term: Expose a data-node-id attribute on every rendered node from inside core (similar to how ELK adds data-* attributes today). That gives mermaid-interactive — and any third-party code that reflects on the rendered graph — a stable, semantically meaningful hook. Happy to open a companion issue or a small core PR for that if it would unblock this.

Offer: happy to narrow the initial PR

If the selector coupling is a blocker for the full package, I can scope this PR down to just:

  • The binder core (bind() + teardown contract, no DOM querying)
  • The @{ view: collapsed } preprocessor syntax (pure text transform, zero SVG touching)

That merges the stable contract while leaving the selector-dependent tooltip/collapse DOM logic for a follow-up PR once the data-node-id hook is in place. Let me know which direction you'd prefer.

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.

click/hover to Expand or collapse subgraphs/par

6 participants