Revert "chore: replace mermaid with beautiful-mermaid"#43
Conversation
This reverts commit 6fce8f2.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request updates the diagram rendering library from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/chronicle/src/components/mdx/mermaid.tsx (1)
19-19: Make Mermaid security settings explicit as a best practice.While Mermaid v11.13.0 defaults to
securityLevel: 'strict'(which is safe fordangerouslySetInnerHTMLusage), explicitly setting this at Line 19 clarifies intent and guards against unexpected future version changes.Suggested configuration
- mermaid.initialize({ startOnLoad: false, theme: 'default' }) + mermaid.initialize({ + startOnLoad: false, + theme: 'default', + securityLevel: 'strict', + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chronicle/src/components/mdx/mermaid.tsx` at line 19, The mermaid.initialize call should explicitly set securityLevel to 'strict' to harden against future defaults changing; update the mermaid.initialize invocation (the mermaid.initialize({ startOnLoad: false, theme: 'default' }) call) to include securityLevel: 'strict' in its options so the component always runs with strict security for any dangerouslySetInnerHTML usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/chronicle/src/components/mdx/mermaid.tsx`:
- Around line 14-29: The useEffect's inner async render function calls
mermaid.render without error handling, causing silent failures; wrap the
import/mermaid.initialize and mermaid.render calls inside a try/catch in the
render function (checking cancelled before updating) and on error call setSvg
with a sensible fallback (e.g., an error/SVG fallback or a user-visible message)
and log the error (console.error or process logger) so invalid diagram syntax or
other failures don't leave the component blank; keep references to mermaidId,
chart, cancelled and only update state via setSvg when not cancelled.
---
Nitpick comments:
In `@packages/chronicle/src/components/mdx/mermaid.tsx`:
- Line 19: The mermaid.initialize call should explicitly set securityLevel to
'strict' to harden against future defaults changing; update the
mermaid.initialize invocation (the mermaid.initialize({ startOnLoad: false,
theme: 'default' }) call) to include securityLevel: 'strict' in its options so
the component always runs with strict security for any dangerouslySetInnerHTML
usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc898b24-329d-4654-b76a-a90572a4daaa
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
packages/chronicle/package.jsonpackages/chronicle/src/components/mdx/mermaid.tsx
| useEffect(() => { | ||
| let cancelled = false | ||
|
|
||
| async function render() { | ||
| const { default: mermaid } = await import('mermaid') | ||
| mermaid.initialize({ startOnLoad: false, theme: 'default' }) | ||
| const { svg: rendered } = await mermaid.render( | ||
| mermaidId, | ||
| chart | ||
| ) | ||
| if (!cancelled) setSvg(rendered) | ||
| } | ||
|
|
||
| render() | ||
| return () => { cancelled = true } | ||
| }, [chart]) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Mermaid version 11.13.0, does mermaid.render reject/throw on invalid diagram syntax, and what is the recommended React useEffect error-handling pattern?
💡 Result:
For Mermaid 11.13.0: yes—mermaid.render rejects/throws on invalid diagram syntax. If you call the async variant (the recommended API in v11), invalid Mermaid text is surfaced as a rejected promise you should catch (mermaid’s “render failed” handling is documented, and the docs explicitly show wrapping mermaid.render in try/catch). Also, multiple Mermaid issues describe “Syntax error in text” occurring for invalid diagram text when using the render API, consistent with render failing rather than silently succeeding. Recommended React useEffect error-handling pattern: - Don’t rely on the synchronous call/“callback style” to capture errors. - Use an async effect (or define an async function inside the effect) and wrap the await in try/catch. - Track a local “cancelled” flag in the effect cleanup so you don’t call setState after unmount. - Optionally handle parseability separately with mermaid.parse before calling render, or rely on the promise rejection from render. Example pattern: useEffect( => { let cancelled = false; const run = async => { try { const { svg } = await mermaid.render('diagram-id', diagramText); if (!cancelled) setSvg(svg); } catch (err) { if (!cancelled) setError(err instanceof Error ? err.message : String(err)); } }; run; return => { cancelled = true; }; }, [diagramText]); This matches Mermaid’s documented approach to handling errors from render (try/catch around await) and the general React pattern of preventing state updates after unmount.
Citations:
- 1: https://mermaid-js-mermaid.mintlify.app/advanced/error-handling
- 2: Fix: Syntax error in class diagrams with nested generics and inline type definitions mermaid-js/mermaid#7480
- 3: https://stackoverflow.com/questions/79198425/syntax-error-in-textmermaid-version-11-4-0-when-using-mermaid-render
- 4: https://mermaid.js.org/config/setup/mermaid/interfaces/Mermaid.html
- 5: Allow to avoid inserting 'Syntax error' node to DOM on rendering error mermaid-js/mermaid#4358
Handle async Mermaid failures to avoid silent blank output.
At Line 27, render() is called without error handling. Mermaid v11.13.0's render() method throws on invalid diagram syntax and other failures. Without a try/catch block, the component fails silently with no fallback state.
Proposed fix
useEffect(() => {
let cancelled = false
async function render() {
- const { default: mermaid } = await import('mermaid')
- mermaid.initialize({ startOnLoad: false, theme: 'default' })
- const { svg: rendered } = await mermaid.render(
- mermaidId,
- chart
- )
- if (!cancelled) setSvg(rendered)
+ try {
+ const { default: mermaid } = await import('mermaid')
+ mermaid.initialize({ startOnLoad: false, theme: 'default' })
+ const { svg: rendered } = await mermaid.render(mermaidId, chart)
+ if (!cancelled) setSvg(rendered)
+ } catch {
+ if (!cancelled) setSvg('')
+ }
}
- render()
+ void render()
return () => { cancelled = true }
}, [chart])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| let cancelled = false | |
| async function render() { | |
| const { default: mermaid } = await import('mermaid') | |
| mermaid.initialize({ startOnLoad: false, theme: 'default' }) | |
| const { svg: rendered } = await mermaid.render( | |
| mermaidId, | |
| chart | |
| ) | |
| if (!cancelled) setSvg(rendered) | |
| } | |
| render() | |
| return () => { cancelled = true } | |
| }, [chart]) | |
| useEffect(() => { | |
| let cancelled = false | |
| async function render() { | |
| try { | |
| const { default: mermaid } = await import('mermaid') | |
| mermaid.initialize({ startOnLoad: false, theme: 'default' }) | |
| const { svg: rendered } = await mermaid.render(mermaidId, chart) | |
| if (!cancelled) setSvg(rendered) | |
| } catch { | |
| if (!cancelled) setSvg('') | |
| } | |
| } | |
| void render() | |
| return () => { cancelled = true } | |
| }, [chart]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/chronicle/src/components/mdx/mermaid.tsx` around lines 14 - 29, The
useEffect's inner async render function calls mermaid.render without error
handling, causing silent failures; wrap the import/mermaid.initialize and
mermaid.render calls inside a try/catch in the render function (checking
cancelled before updating) and on error call setSvg with a sensible fallback
(e.g., an error/SVG fallback or a user-visible message) and log the error
(console.error or process logger) so invalid diagram syntax or other failures
don't leave the component blank; keep references to mermaidId, chart, cancelled
and only update state via setSvg when not cancelled.
2 participants
Reverts #42