perf: use the fastdom library to avoid unnecessary reflows#7951
perf: use the fastdom library to avoid unnecessary reflows#7951aloisklink wants to merge 3 commits into
fastdom library to avoid unnecessary reflows#7951Conversation
Add the `fastdom` library to batch reflows when `getBoundingBox()` is called in `addHtmlSpan`, leading to a performance increase. Only the `addHtmlSpan` function was edited at first, since it's the common bottleneck, we can add more later. I've pinned the used version of `fastdom` to v1.0.12 to lower the risk of supply-chain attacks, since this library is quite simple, has no dependencies (`strictdom` is a `dependency`, but not for any of the code we use), and so is unlikely to need urgent bugfixes.
Extend the `fastdom` batching from `addHtmlSpan` to other hot call sites: `insertEdgeLabel`, `labelHelper`, and `insertLabel`. This increases performance by up to 15% in some cases.
Fixes a conflict with 6510e01 (feat(profiling): DevTools phase markers + per-op measure buckets, 2026-06-16) that wrapped some `getBBox()` and `getBoundingClientRect()` calls. Conflicts: - packages/mermaid/package.json - packages/mermaid/src/rendering-util/rendering-elements/shapes/util.ts
|
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
❌ Lockfile Validation Failed The following issue(s) were detected: Please address these and push an update. Posted automatically by GitHub Actions |
📝 WalkthroughWalkthroughIntroduces a fastdom wrapper module using microtask-based scheduling and fastdom-promised, adds it as a package dependency, restricts direct fastdom imports via ESLint, and replaces synchronous DOM bounding-box reads with fastdom.measure calls in createText.ts, edges.js, and shapes/util.ts. ChangesFastdom Measurement Integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Renderer as Rendering Code
participant Fastdom as fastdom wrapper
participant Scheduler as queueMicrotask/setTimeout
participant DOM as Browser DOM
Renderer->>Fastdom: fastdom.measure(() => element.getBoundingClientRect())
Fastdom->>Scheduler: schedule read via raf override
Scheduler->>DOM: execute getBoundingClientRect()/getBBox()
DOM-->>Scheduler: bounding box result
Scheduler-->>Fastdom: resolve promise
Fastdom-->>Renderer: bbox/transformBbox
Renderer->>Renderer: apply computeLabelTransform / style updates
Related PRs: None identified. Suggested labels: performance, dependencies Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
@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 #7951 +/- ##
===========================================
+ Coverage 77.42% 77.57% +0.15%
===========================================
Files 564 565 +1
Lines 74900 74930 +30
Branches 12659 14626 +1967
===========================================
+ Hits 57988 58125 +137
+ Misses 15915 15806 -109
- Partials 997 999 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
eslint.config.js (1)
74-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestriction only covers the bare
fastdomspecifier, not subpath imports.
paths: [{ name: 'fastdom' }]blocksimport ... from 'fastdom'but notimport ... from 'fastdom/extensions/fastdom-promised.js', whichfastdom.tsitself uses. A future contributor could import that subpath directly elsewhere, bypassing the wrapper without triggering this rule.Suggested fix to also block subpath imports
'`@typescript-eslint/no-restricted-imports`': [ 'error', { paths: [ { name: 'fastdom', message: 'Use the `fastdom` wrapper from `rendering-util/fastdom.ts` instead of importing `fastdom` directly.', }, ], + patterns: [ + { + group: ['fastdom/*'], + message: + 'Use the `fastdom` wrapper from `rendering-util/fastdom.ts` instead of importing `fastdom` directly.', + }, + ], }, ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eslint.config.js` around lines 74 - 85, The `@typescript-eslint/no-restricted-imports` rule in `eslint.config.js` only blocks the bare `fastdom` import and can still be bypassed with subpath imports. Update the restriction entry for `fastdom` so it also matches and rejects subpath specifiers like `fastdom/extensions/fastdom-promised.js`, keeping the `rendering-util/fastdom.ts` wrapper as the only allowed entry point.packages/mermaid/src/rendering-util/rendering-elements/shapes/util.ts (2)
85-86: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSame write-batching gap as
dv.attr(...)inedges.js.The width/height attribute writes here also execute synchronously right after the awaited
fastdom.measure, outside afastdom.mutatecall, leaving the write phase unbatched. Same recommendation as raised inedges.js: wrap these infastdom.mutateif the wrapper exposes it, to complete the read/write separation.Also applies to: 150-151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mermaid/src/rendering-util/rendering-elements/shapes/util.ts` around lines 85 - 86, The width and height attribute updates in the rendering path are being applied immediately after the `fastdom.measure` read, so they bypass write batching. Update the `dv.attr(...)` calls in the relevant shape rendering helper to run inside `fastdom.mutate` when available, matching the read/write separation used elsewhere such as in `edges.js` and the same helper’s other `dv.attr` writes.
80-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate fastdom.measure logic between
labelHelperandinsertLabel.Both functions now carry nearly identical measure blocks (HTML
getBoundingClientRectvs SVGgetBBox, each with the sameinjected.profiling && profiler.tickSyncbranching). Since both were touched in this change, extracting a sharedmeasureLabelBBox(useHtml, div, text)helper would reduce the risk of the two copies diverging (e.g., if profiling instrumentation or fastdom usage is later updated in only one place).Also applies to: 145-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mermaid/src/rendering-util/rendering-elements/shapes/util.ts` around lines 80 - 93, The `fastdom.measure` sizing logic is duplicated in `labelHelper` and `insertLabel`, with the same `injected.profiling && profiler.tickSync` branching for HTML and SVG measurement. Extract that shared behavior into a helper like `measureLabelBBox(useHtml, div, text)` in the rendering utilities, then have both functions call it so the `getBoundingClientRect`/`getBBox` and profiling paths stay consistent in one place.packages/mermaid/src/rendering-util/rendering-elements/edges.js (1)
114-117: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider batching the follow-up writes too.
dv.attr('width', ...)/dv.attr('height', ...)execute synchronously right after the awaitedfastdom.measureresolves, outside anyfastdom.mutatecall. This keeps the read phase batched but leaves writes unbatched, so a write can still interleave with another pending read scheduled in the same microtask cycle, partially undermining the reflow-avoidance goal. If the wrapper exposesfastdom.mutate(typical offastdom-promised), wrapping these writes would complete the read/write separation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mermaid/src/rendering-util/rendering-elements/edges.js` around lines 114 - 117, The `edges.js` rendering flow batches the read with `fastdom.measure`, but the subsequent `dv.attr('width', ...)` and `dv.attr('height', ...)` writes are still performed synchronously outside a mutate phase. Update the logic around the `bbox`/`transformBbox` assignment in the edge rendering path to wrap those attribute updates in `fastdom.mutate` (if available from the fastdom wrapper) so the read/write separation is preserved and follow-up writes stay batched.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@eslint.config.js`:
- Around line 74-85: The `@typescript-eslint/no-restricted-imports` rule in
`eslint.config.js` only blocks the bare `fastdom` import and can still be
bypassed with subpath imports. Update the restriction entry for `fastdom` so it
also matches and rejects subpath specifiers like
`fastdom/extensions/fastdom-promised.js`, keeping the
`rendering-util/fastdom.ts` wrapper as the only allowed entry point.
In `@packages/mermaid/src/rendering-util/rendering-elements/edges.js`:
- Around line 114-117: The `edges.js` rendering flow batches the read with
`fastdom.measure`, but the subsequent `dv.attr('width', ...)` and
`dv.attr('height', ...)` writes are still performed synchronously outside a
mutate phase. Update the logic around the `bbox`/`transformBbox` assignment in
the edge rendering path to wrap those attribute updates in `fastdom.mutate` (if
available from the fastdom wrapper) so the read/write separation is preserved
and follow-up writes stay batched.
In `@packages/mermaid/src/rendering-util/rendering-elements/shapes/util.ts`:
- Around line 85-86: The width and height attribute updates in the rendering
path are being applied immediately after the `fastdom.measure` read, so they
bypass write batching. Update the `dv.attr(...)` calls in the relevant shape
rendering helper to run inside `fastdom.mutate` when available, matching the
read/write separation used elsewhere such as in `edges.js` and the same helper’s
other `dv.attr` writes.
- Around line 80-93: The `fastdom.measure` sizing logic is duplicated in
`labelHelper` and `insertLabel`, with the same `injected.profiling &&
profiler.tickSync` branching for HTML and SVG measurement. Extract that shared
behavior into a helper like `measureLabelBBox(useHtml, div, text)` in the
rendering utilities, then have both functions call it so the
`getBoundingClientRect`/`getBBox` and profiling paths stay consistent in one
place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 467cc4a9-208c-46db-85e5-dfee104fa66f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.cspell/libraries.txteslint.config.jspackages/mermaid/package.jsonpackages/mermaid/src/rendering-util/createText.tspackages/mermaid/src/rendering-util/fastdom.tspackages/mermaid/src/rendering-util/rendering-elements/edges.jspackages/mermaid/src/rendering-util/rendering-elements/shapes/util.ts
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
| node.height = knownBounds.height; | ||
| return; | ||
| } | ||
| // TODO: Make this function `async` and use `fastdom.measure` to batch reflow |
There was a problem hiding this comment.
Shall this be fixed within this PR??
|
Thanks for this PR @aloisklink [sisyphos-bot] What's working well 🎉 Exact version pin on "fastdom": "1.0.12" is good supply-chain hygiene for a library that ends up in every user's bundle. 🎉 The ESLint no-restricted-imports rule forcing all code through the wrapper is excellent architecture. Centralizing scheduling means a future swap (different scheduler, debug instrumentation) is a 🎉 queueMicrotask over requestAnimationFrame is a clever tradeoff. rAF delays reads to the next frame (~16ms at 60fps); queueMicrotask batches without the artificial latency. The setTimeout(cb, 0) 🎉 The dead re-measure in createText.ts was correctly removed. The original code called getBoundingClientRect() a second time inside the if (bbox.width === width) block, but the result was assigned 🎉 Incremental scope is the right call here. Changing every DOM read site at once across shared rendering code would be a review and regression nightmare. Limiting to the three highest-impact Things to address 🟡 [important] Missing changeset — packages/mermaid gained a new runtime dependency and a reported 25–30% performance improvement on large diagrams. The PR marks changeset N/A, but CONTRIBUTING.md Nits 🟢 JSDoc typo in edges.js — /** @type {@DOMRect} / has a spurious @ before the type name. Should be /* @type {DOMRect} */. 🟢 ESLint restriction doesn't cover sub-path imports — the rule bans 'fastdom' but not 'fastdom/extensions/fastdom-promised.js'. A developer could import the promisified extension directly without a Security XSS sub-agent reviewed all changed files — no issues found. All fastdom.measure() callbacks contain only reads (getBoundingClientRect, getBBox). No DOM writes, no innerHTML, no eval-equivalent calls One forward-looking note: the wrapper exports the full fastdom object including .mutate(). No current code uses it, but if a future caller puts user-controlled content inside fastdom.mutate(), the |
2 participants
📑 Summary
Currently, Mermaid suffers from a lot of layout thrashing, by constantly measuring bounding boxes/widths, then updating the DOM, and measuring bounding boxes/widths again, forcing a lot of reflows.
Using a library like
fastdomcan be used to easily batch these, to avoid unnecessary reflows (assuming that changes can run in parallel).From my tests, a flowchart with 1000 labels can sometimes be 25-30%.
Closes #7872 (that PR also batches measurements, but without the
fastdomlibrary).See: https://github.com/wilsonpage/fastdom
📏 Design Decisions
The
fastdomlibrary was used to batch, so that we could add thestrict-fastdomtool later, which throws an error iffastdomis not used. I've made sure to pin the dependency to reduce the risk of any supply-chain attacks.I've also only updated the simplest and highest impact code calls in this PR. There are other places where we can update this too, but we can leave those for future PRs.
htmlLabels: falseis still a big slowdown, since the current line-wrapping algorithm for SVG labels might do multiple reflows for a single label, but I've got an idea on how to fix that!📋 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:.This PR introduces
fastdom-based batching to reduce layout thrashing in Mermaid’s HTML/SVG label rendering paths.Highlights
fastdomdependency (1.0.12) and a local wrapper module to centralize scheduling behavior.fastdom.measure(...)instead of synchronous DOM reads.addHtmlSpaninsertEdgeLabellabelHelper/insertLabelfastdomimports outside the wrapper.fastdom.Impact