chore: upgrade Mermaid to the latest stable version - #5681
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMermaid rendering moved from ChangesMermaid rendering
Fetch test stub migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MDX
participant MermaidDiagram
participant Mermaid
participant DOMPurify
MDX->>MermaidDiagram: pass graph definition
MermaidDiagram->>Mermaid: load and render graph
Mermaid-->>MermaidDiagram: return SVG
MermaidDiagram->>DOMPurify: sanitize SVG
DOMPurify-->>MermaidDiagram: return sanitized SVG
MermaidDiagram-->>MDX: render SVG or error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
princerajpoot20
changed the title
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5681--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
components/MDX/MDX.tsx (1)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a loading fallback for the dynamic import.
Without a
loadingoption,next/dynamicrenders nothing until theMermaidDiagramchunk loads, which can cause a layout shift for pages containing diagrams.💡 Optional improvement
-const MermaidDiagram = dynamic(() => import('./MermaidDiagram'), { ssr: false }); +const MermaidDiagram = dynamic(() => import('./MermaidDiagram'), { + ssr: false, + loading: () => <div className="animate-pulse h-40 w-full rounded bg-gray-200 dark:bg-gray-700" /> +});🤖 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 `@components/MDX/MDX.tsx` at line 44, Update the MermaidDiagram dynamic import configuration to include a loading fallback via next/dynamic’s loading option. Use a suitable placeholder that preserves diagram space while the chunk loads, while keeping server-side rendering disabled.
🤖 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.
Inline comments:
In `@components/MDX/MermaidDiagram.tsx`:
- Around line 70-85: Initialize the theme state lazily in MermaidDiagram using
getMermaidTheme instead of hard-coding 'light', so the first render effect runs
with the site's actual theme and avoids an unnecessary corrective render. Keep
the existing MutationObserver update and cleanup behavior unchanged.
- Around line 127-129: Update the effect callback in MermaidDiagram so every
exit path has a consistent return shape: the early exit should explicitly return
the same no-cleanup value used by React effects, while the mounted cleanup path
continues returning its cleanup function. Preserve the existing mounted
assignment and cleanup behavior.
- Around line 96-132: Sanitize the SVG returned by mermaid.render before storing
it in state: in the render function, import DOMPurify from mermaid and pass
rendered through DOMPurify.sanitize with the SVG profile, then call setSvg with
the sanitized result. Keep the existing mounted check and error handling
unchanged.
---
Nitpick comments:
In `@components/MDX/MDX.tsx`:
- Line 44: Update the MermaidDiagram dynamic import configuration to include a
loading fallback via next/dynamic’s loading option. Use a suitable placeholder
that preserves diagram space while the chunk loads, while keeping server-side
rendering disabled.
🪄 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 Plus
Run ID: c7361161-f1bd-4805-937f-ee392e887e8b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
components/MDX/MDX.tsxcomponents/MDX/MermaidDiagram.tsxnext.config.mjspackage.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/MDX/MermaidDiagram.tsx (1)
109-116: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider surfacing a fallback message when rendering fails.
On a render error,
setSvg(null)clears the SVG and the error is only logged to the console (line 115). The user sees a blank<div>with no indication that the diagram failed to render. Consider tracking an error state and rendering a short fallback message so readers of the page know the diagram is unavailable rather than assuming a rendering delay.🤖 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 `@components/MDX/MermaidDiagram.tsx` around lines 109 - 116, The MermaidDiagram error path currently leaves a blank container after rendering fails. Add an error state in MermaidDiagram, set it in the catch block alongside clearing the SVG, and render a concise fallback message when that state is active so users know the diagram is unavailable.
🤖 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.
Inline comments:
In `@components/MDX/MermaidDiagram.tsx`:
- Around line 87-120: Update the graph branching in the MermaidDiagram render
flow to check the positive graph condition first, moving the existing
asynchronous render logic into that branch and retaining setSvg(null) for the
absent-graph path. Preserve the current rendering, error handling, and mounted
checks.
---
Nitpick comments:
In `@components/MDX/MermaidDiagram.tsx`:
- Around line 109-116: The MermaidDiagram error path currently leaves a blank
container after rendering fails. Add an error state in MermaidDiagram, set it in
the catch block alongside clearing the SVG, and render a concise fallback
message when that state is active so users know the diagram is unavailable.
🪄 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 Plus
Run ID: 3ca57637-564f-4e61-b7d0-3ac858590afe
📒 Files selected for processing (2)
components/MDX/MDX.tsxcomponents/MDX/MermaidDiagram.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- components/MDX/MDX.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
75-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the dependency versions selected for this PR.
^3.4.0and^11.4.1allow later minor releases when dependencies are resolved again. Mermaid output and parser behavior can then change without a source diff. Pin the resolveddompurifyandmermaidversions, and keep the lockfile aligned.Verify that CI uses the committed lockfile with a frozen install before retaining these ranges.
Based on learnings, this repository prefers exact versions or conservative ranges for reproducible builds.
Also applies to: 89-89
🤖 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 `@package.json` at line 75, Update the package.json entries for dompurify and mermaid to use the resolved exact versions instead of caret ranges, then regenerate or adjust the lockfile so it matches those versions. Verify CI performs a frozen install using the committed lockfile before finalizing the dependency changes.Source: Learnings
🤖 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.
Inline comments:
In `@components/MDX/MermaidDiagram.tsx`:
- Around line 89-90: Update the graph handling in MermaidDiagram to compute and
reuse a trimmed graph value before deciding whether to render, so
whitespace-only content follows the empty fallback instead of being passed to
Mermaid. In the empty-state cleanup around the svg reset, also reset hasError to
prevent a prior render failure from persisting; add regression coverage for
whitespace-only input and the failed-render-to-empty transition.
---
Nitpick comments:
In `@package.json`:
- Line 75: Update the package.json entries for dompurify and mermaid to use the
resolved exact versions instead of caret ranges, then regenerate or adjust the
lockfile so it matches those versions. Verify CI performs a frozen install using
the committed lockfile before finalizing the dependency changes.
🪄 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 Plus
Run ID: 27392067-cb29-4a4e-9592-0559afcd070c
📒 Files selected for processing (2)
components/MDX/MermaidDiagram.tsxpackage.json
|
netlify/edge-functions/tests/serve-definitions.test.ts is failing. It doesn't seem to be due to the changes in this PR. checking.. Reference Run: https://github.com/asyncapi/website/actions/runs/30749023695/job/91500185845?pr=5681 |
|
I verified this with a recent pipeline from another PR as well. Reference: https://github.com/asyncapi/website/actions/runs/30748413716/job/91508982939?pr=5592. That pipeline is now failing too. The reason the cache stopped working now is that the CI environment invalidated the Deno module cache. This could be due to recent changes we made in the CI configuration or simply periodic cache cleanup. Either way, the cache is now gone, so the build is failing because it can no longer fetch the module from Working on a fix. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
netlify/edge-functions/tests/serve-definitions.test.ts (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a reproducible lock for the JSR testing import.
netlify/edge-functions/tests/serve-definitions.test.tsimports unversionedjsr:@std/testing/mock``, and this repository does not providedeno.lock, `deno.json`, or `deno.jsonc`. Pin `@std/testing` in a `deno.lock`/Deno config or use a fully versioned remote specifier.🤖 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 `@netlify/edge-functions/tests/serve-definitions.test.ts` at line 3, Pin the `@std/testing` dependency used by the stub import in serve-definitions.test.ts so it resolves reproducibly. Add the dependency version through the repository’s Deno lock/config mechanism, or replace the unversioned jsr:`@std/testing/mock` specifier with a fully versioned one; do not leave the import unpinned.
🤖 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 `@netlify/edge-functions/tests/serve-definitions.test.ts`:
- Line 3: Pin the `@std/testing` dependency used by the stub import in
serve-definitions.test.ts so it resolves reproducibly. Add the dependency
version through the repository’s Deno lock/config mechanism, or replace the
unversioned jsr:`@std/testing/mock` specifier with a fully versioned one; do not
leave the import unpinned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 47d4d130-c7ad-4899-bcfa-05520790f622
📒 Files selected for processing (1)
netlify/edge-functions/tests/serve-definitions.test.ts
|
|
/rtm |
4 participants
Related #5633
Summary by CodeRabbit
New Features
Bug Fixes
Refactor