fix(mermaid): Mermaid lazy render when tab becomes visible#12281
fix(mermaid): Mermaid lazy render when tab becomes visible#12281yoshibase wants to merge 1 commit into
Conversation
|
Hi @yoshibase! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
slorber
left a comment
There was a problem hiding this comment.
That doesn't pass our CI checks and also I don't think it's the correct generalized solution.
| function useMermaidId(): string { | ||
| return useId(); | ||
| // Mermaid marker IDs must be unique per diagram; sanitize React useId(). | ||
| return useId().replace(/[^a-zA-Z0-9_-]/g, ''); |
There was a problem hiding this comment.
show me why we need this with a bug repro
| if (!element) { | ||
| return undefined; | ||
| } | ||
| const observer = new IntersectionObserver(([entry]) => { |
There was a problem hiding this comment.
Using an IntersectionObserver maybe works for tabs, but doesn't really this the concurrency problem described here: #8357 (comment)
We are looking for a general solution, not a solution that works only in some specific conditions.
2 mermaid diagrams rendered side-by-side and entering the viewport at the same time should not lead to a rendering bug
Lazy-render Mermaid diagrams when they enter the viewport so hidden tab panels do not produce incomplete SVG markers. Fixes facebook#8357.
e996ddc to
e076572
Compare
|
Thanks for the detailed review. I've expanded the PR test plan with concrete repro steps for #8357 (tabs + DevTools SVG marker inspection) and noted the concurrency gap you pointed out. I'm reworking the implementation — the IntersectionObserver approach is too narrow and I don't have a solid repro for the No change pushed to the code approach in this update; focusing on documenting repro/verification first while I investigate a generalized fix. |
|
Pausing this PR — IO approach does not fix concurrent-render case. Will revisit with render-queue or close. |
|
Closing — IntersectionObserver approach does not address the concurrent-render case discussed in review. Will revisit separately if I can land a render-queue fix. |
Summary
Fixes #8357
Mermaid diagrams inside inactive tab panels render with missing arrow markers. The root cause is rendering while the SVG is hidden (zero-size container), not a Mermaid syntax issue.
Note: @slorber's review pointed out that the IntersectionObserver approach here is too narrow (tabs only) and does not address concurrent renders of multiple visible diagrams (#8357 comment). I am keeping this PR open while I rework the approach. The test plan below documents how to reproduce the original bug and how I verified the tab case locally.
Reproducing the bug (on main)
Known-good reference on docusaurus.io: https://docusaurus.io/tests/pages/diagrams#mermaid-in-tabs
To reproduce in a fresh site:
@docusaurus/theme-mermaidindocusaurus.config.js.npm startand open the page in Chrome.What to look for in DevTools
<marker>definitions in<defs>andmarker-endattributes on<path>elements.display: none/ zero size.Test plan for this PR's approach (tab visibility)
Preview: deploy preview for PR #12281 (Netlify bot comment on the PR)
Side-by-side diagrams (concurrency case — known gap)
@slorber noted two diagrams entering the viewport at once can still race. To check that case:
graph LRdiagrams in the same tab, one above the other (no tabs).I have not claimed this PR fixes that concurrency case — only the hidden-tab render timing.
Status
useId()sanitization unless a concrete repro is shown (per inline review)