perf: lazy-load mermaid to reduce main bundle size#164
Conversation
📝 WalkthroughWalkthroughMermaid rendering was changed to lazy, dynamic loading and async initialization; mermaid code-block detection now inspects the markdown AST ( Changes
Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/chat/markdownComponents.tsx (1)
173-181:⚠️ Potential issue | 🟠 MajorFix variable shadowing —
nodeis already declared in outer scope.The pipeline failure indicates that
nodeon line 173 shadowsnodefrom line 162. Rename the inner parameter.🔧 Proposed fix
// Extract text from nested <code> children for the copy button - const extractText = (node: React.ReactNode): string => { - if (typeof node === 'string') return node; - if (Array.isArray(node)) return node.map(extractText).join(''); - if (React.isValidElement(node) && node.props) { - const props = node.props as { children?: React.ReactNode }; + const extractText = (reactNode: React.ReactNode): string => { + if (typeof reactNode === 'string') return reactNode; + if (Array.isArray(reactNode)) return reactNode.map(extractText).join(''); + if (React.isValidElement(reactNode) && reactNode.props) { + const props = reactNode.props as { children?: React.ReactNode }; return extractText(props.children); } return ''; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/markdownComponents.tsx` around lines 173 - 181, The helper extractText currently declares a parameter named node that shadows an outer-scope variable; rename the parameter (for example to child or childNode) in the extractText function to avoid shadowing, update all internal uses (Array.isArray, typeof checks, React.isValidElement and props.children handling) to reference the new parameter name, and ensure the function signature and its recursive call use the new identifier consistently within extractText.
🧹 Nitpick comments (2)
src/renderer/components/chat/markdownComponents.tsx (1)
7-10: Move the lazy import after all static imports.The
React.lazy()call is interleaved with static imports. While not a blocking error, grouping all static imports together improves readability.import React from 'react'; import { api } from '@renderer/api'; import { CopyButton } from '@renderer/components/common/CopyButton'; import { PROSE_BODY } from '@renderer/constants/cssVariables'; - -const MermaidViewer = React.lazy(() => - import('./viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) -); import { highlightSearchInChildren, type SearchContext } from './searchHighlightUtils'; import type { Components } from 'react-markdown'; + +const MermaidViewer = React.lazy(() => + import('./viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/markdownComponents.tsx` around lines 7 - 10, The React.lazy declaration for MermaidViewer is placed before static imports; move the const MermaidViewer = React.lazy(...) line so all static imports (e.g., highlightSearchInChildren and SearchContext from searchHighlightUtils) appear first and then place the lazy import after them to keep import ordering consistent and grouped; ensure you only reorder lines (no other changes to MermaidViewer or import names).src/renderer/components/chat/viewers/MarkdownViewer.tsx (1)
36-39: Consider moving the lazy import after all static imports for consistency.Same suggestion as in
markdownComponents.tsx— group static imports together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx` around lines 36 - 39, Move the React.lazy import of MermaidViewer so that all static imports are grouped together: keep the import { highlightLine } from '../viewers/syntaxHighlighter' (and any other static imports) at the top, then declare const MermaidViewer = React.lazy(...) afterwards; update the order in MarkdownViewer.tsx to match the static-first ordering used in markdownComponents.tsx so static imports appear first and the lazy-loaded MermaidViewer follows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/chat/viewers/MermaidViewer.tsx`:
- Around line 18-26: The inline import() type annotations are forbidden: create
a top-level type alias (e.g., type MermaidModule = typeof import('mermaid'); or
type MermaidDefault = (typeof import('mermaid'))['default']) and then use that
alias for mermaidModule and the Promise return type of getMermaid instead of
inline import() types; update the declarations of mermaidModule,
lastMermaidTheme (unchanged), and the getMermaid signature to reference the new
alias (e.g., let mermaidModule: MermaidModule | null and async function
getMermaid(): Promise<MermaidDefault>), ensuring all references to
mermaidModule.default remain valid.
- Around line 28-41: The return type annotation on ensureMermaidInit is overly
complex; change the signature to return Promise<typeof import('mermaid')>
(matching the getMermaid() export) instead of Promise<(typeof
import('mermaid'))['default']>, and ensure the function still returns the m from
getMermaid() and that getMermaid()'s exported type remains consistent with
ensureMermaidInit.
---
Outside diff comments:
In `@src/renderer/components/chat/markdownComponents.tsx`:
- Around line 173-181: The helper extractText currently declares a parameter
named node that shadows an outer-scope variable; rename the parameter (for
example to child or childNode) in the extractText function to avoid shadowing,
update all internal uses (Array.isArray, typeof checks, React.isValidElement and
props.children handling) to reference the new parameter name, and ensure the
function signature and its recursive call use the new identifier consistently
within extractText.
---
Nitpick comments:
In `@src/renderer/components/chat/markdownComponents.tsx`:
- Around line 7-10: The React.lazy declaration for MermaidViewer is placed
before static imports; move the const MermaidViewer = React.lazy(...) line so
all static imports (e.g., highlightSearchInChildren and SearchContext from
searchHighlightUtils) appear first and then place the lazy import after them to
keep import ordering consistent and grouped; ensure you only reorder lines (no
other changes to MermaidViewer or import names).
In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx`:
- Around line 36-39: Move the React.lazy import of MermaidViewer so that all
static imports are grouped together: keep the import { highlightLine } from
'../viewers/syntaxHighlighter' (and any other static imports) at the top, then
declare const MermaidViewer = React.lazy(...) afterwards; update the order in
MarkdownViewer.tsx to match the static-first ordering used in
markdownComponents.tsx so static imports appear first and the lazy-loaded
MermaidViewer follows.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9e6e9d2-d273-4866-a8d3-f527ec5a5550
📒 Files selected for processing (3)
src/renderer/components/chat/markdownComponents.tsxsrc/renderer/components/chat/viewers/MarkdownViewer.tsxsrc/renderer/components/chat/viewers/MermaidViewer.tsx
There was a problem hiding this comment.
Pull request overview
This PR reduces the renderer’s initial JS payload by deferring Mermaid (previously a large static dependency) until a Mermaid diagram is actually rendered, and by lazy-loading the Mermaid viewer component in markdown rendering paths.
Changes:
- Lazy-load Mermaid itself via dynamic
import('mermaid')insideMermaidViewer. - Lazy-load
MermaidViewerwithReact.lazy+Suspensein both markdown component factories. - Replace the prior React element type check in
<pre>handling with an AST/class-basedlanguage-mermaiddetection.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/renderer/components/chat/viewers/MermaidViewer.tsx |
Switch Mermaid from static import to dynamic import with cached initialization keyed by theme. |
src/renderer/components/chat/viewers/MarkdownViewer.tsx |
Lazy-load MermaidViewer and update mermaid <pre> unwrapping logic to use AST-derived language-mermaid classes. |
src/renderer/components/chat/markdownComponents.tsx |
Same lazy-load + AST-based mermaid detection for the shared markdown components (with copy-button <pre> wrapper logic). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const MermaidViewer = React.lazy(() => | ||
| import('../viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) | ||
| ); | ||
| import { highlightLine } from '../viewers/syntaxHighlighter'; |
There was a problem hiding this comment.
simple-import-sort/imports (see eslint.config.js import-sorting rules) expects all import declarations to be grouped/sorted together. Having import { highlightLine ... } after the const MermaidViewer = React.lazy(...) statement will likely fail lint; move the highlightLine import above and keep the lazy wrapper after the last import (or extract it to a separate module).
| const MermaidViewer = React.lazy(() => | |
| import('../viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) | |
| ); | |
| import { highlightLine } from '../viewers/syntaxHighlighter'; | |
| import { highlightLine } from '../viewers/syntaxHighlighter'; | |
| const MermaidViewer = React.lazy(() => | |
| import('../viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) | |
| ); |
| if (lang === 'mermaid') { | ||
| return <MermaidViewer code={text} />; | ||
| return ( | ||
| <React.Suspense fallback={<code className="font-mono text-xs">{text}</code>}> |
There was a problem hiding this comment.
The Suspense fallback renders a bare <code> element; since mermaid source is typically multiline, newlines/indentation may collapse without white-space: pre/whitespace-pre (or a <pre> wrapper). Consider using a <pre>/code block-style fallback that preserves formatting so the initial render doesn't look broken while the lazy chunk loads.
| <React.Suspense fallback={<code className="font-mono text-xs">{text}</code>}> | |
| <React.Suspense | |
| fallback={ | |
| <pre className="font-mono text-xs whitespace-pre" style={{ color: COLOR_TEXT }}> | |
| <code>{text}</code> | |
| </pre> | |
| } | |
| > |
| | { properties?: { className?: string[] } } | ||
| | undefined; | ||
| const isMermaid = codeEl?.properties?.className?.includes('language-mermaid'); |
There was a problem hiding this comment.
codeEl?.properties?.className?.includes('language-mermaid') can produce false positives when className is a string and merely contains the substring (e.g. language-mermaid2). To avoid skipping the <pre> wrapper incorrectly, treat className as string | string[] and check for an exact token match (=== 'language-mermaid' in the array, or split string on whitespace and match tokens).
| | { properties?: { className?: string[] } } | |
| | undefined; | |
| const isMermaid = codeEl?.properties?.className?.includes('language-mermaid'); | |
| | { properties?: { className?: string | string[] } } | |
| | undefined; | |
| const codeClassName = codeEl?.properties?.className; | |
| const isMermaid = Array.isArray(codeClassName) | |
| ? codeClassName.some((token) => token === 'language-mermaid') | |
| : typeof codeClassName === 'string' | |
| ? codeClassName.split(/\s+/).some((token) => token === 'language-mermaid') | |
| : false; |
| const MermaidViewer = React.lazy(() => | ||
| import('./viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) | ||
| ); | ||
| import { highlightSearchInChildren, type SearchContext } from './searchHighlightUtils'; | ||
|
|
||
| import type { Components } from 'react-markdown'; |
There was a problem hiding this comment.
simple-import-sort/imports (eslint.config.js) expects imports to be grouped/sorted. Declaring const MermaidViewer = React.lazy(...) between import declarations will likely break lint; keep all imports together, then declare MermaidViewer below them (or extract the lazy import to a separate module).
| if (lang === 'mermaid') { | ||
| return <MermaidViewer code={text} />; | ||
| return ( | ||
| <React.Suspense fallback={<code className="block font-mono text-xs">{text}</code>}> |
There was a problem hiding this comment.
The Suspense fallback returns a <code> element without white-space: pre / whitespace-pre or a <pre> wrapper. For mermaid fences this will typically collapse newlines and indentation while the lazy component loads; use a block fallback that preserves formatting (e.g., <pre> + <code> or add whitespace-pre).
| <React.Suspense fallback={<code className="block font-mono text-xs">{text}</code>}> | |
| <React.Suspense | |
| fallback={ | |
| <pre className="font-mono text-xs whitespace-pre"> | |
| <code>{text}</code> | |
| </pre> | |
| } | |
| > |
| | { properties?: { className?: string[] } } | ||
| | undefined; | ||
| const isMermaid = codeEl?.properties?.className?.includes('language-mermaid'); |
There was a problem hiding this comment.
codeEl?.properties?.className?.includes('language-mermaid') can mis-detect when className is a string containing the substring (e.g. language-mermaid2). Consider checking exact class tokens (handle string | string[]) to avoid skipping the <pre> wrapper for non-mermaid code blocks.
| | { properties?: { className?: string[] } } | |
| | undefined; | |
| const isMermaid = codeEl?.properties?.className?.includes('language-mermaid'); | |
| | { properties?: { className?: string | string[] } } | |
| | undefined; | |
| const classNames = codeEl?.properties?.className; | |
| const classTokens = | |
| typeof classNames === 'string' | |
| ? classNames.split(/\s+/).filter(Boolean) | |
| : Array.isArray(classNames) | |
| ? classNames | |
| : []; | |
| const isMermaid = classTokens.includes('language-mermaid'); |
There was a problem hiding this comment.
Code Review
This pull request implements lazy loading for the MermaidViewer component to optimize the application's bundle size. It introduces React.Suspense for handling loading states and updates the markdown rendering logic to identify mermaid blocks via CSS classes. Review feedback suggests adding a missing block class to a fallback element for layout consistency and refactoring the dynamic import logic to store the promise, preventing redundant network requests during concurrent calls.
| if (lang === 'mermaid') { | ||
| return <MermaidViewer code={text} />; | ||
| return ( | ||
| <React.Suspense fallback={<code className="font-mono text-xs">{text}</code>}> |
There was a problem hiding this comment.
The fallback <code> element is missing the block class, which is present in the equivalent fallback in markdownComponents.tsx. Since this component replaces a block-level element (the <pre> wrapper is skipped for mermaid), it should be rendered as a block to maintain layout consistency while loading.
| <React.Suspense fallback={<code className="font-mono text-xs">{text}</code>}> | |
| <React.Suspense fallback={<code className="block font-mono text-xs">{text}</code>}> |
| let mermaidModule: typeof import('mermaid') | null = null; | ||
| let lastMermaidTheme: 'dark' | 'default' | null = null; | ||
|
|
||
| function ensureMermaidInit(isDark: boolean): void { | ||
| async function getMermaid(): Promise<(typeof import('mermaid'))['default']> { | ||
| if (!mermaidModule) { | ||
| mermaidModule = await import('mermaid'); | ||
| } | ||
| return mermaidModule.default; | ||
| } |
There was a problem hiding this comment.
To prevent multiple concurrent calls to import('mermaid') from triggering redundant network requests (even if the browser caches them), it's better to store the promise itself rather than the resolved module. This ensures that subsequent calls await the same pending promise.
| let mermaidModule: typeof import('mermaid') | null = null; | |
| let lastMermaidTheme: 'dark' | 'default' | null = null; | |
| function ensureMermaidInit(isDark: boolean): void { | |
| async function getMermaid(): Promise<(typeof import('mermaid'))['default']> { | |
| if (!mermaidModule) { | |
| mermaidModule = await import('mermaid'); | |
| } | |
| return mermaidModule.default; | |
| } | |
| let mermaidPromise: Promise<typeof import('mermaid')> | null = null; | |
| let lastMermaidTheme: 'dark' | 'default' | null = null; | |
| async function getMermaid(): Promise<(typeof import('mermaid'))['default']> { | |
| if (!mermaidPromise) { | |
| mermaidPromise = import('mermaid'); | |
| } | |
| return (await mermaidPromise).default; | |
| } |
Mermaid was statically imported, bundling ~900KB into the main renderer chunk. This caused V8 parse/compile overhead and GC pressure on every interaction in the production build (dev mode was unaffected since Vite serves modules on-demand). - Dynamic import mermaid in MermaidViewer.tsx (loaded on first diagram render) - React.lazy for MermaidViewer in markdownComponents.tsx and MarkdownViewer.tsx - Replace component type check with AST-based language-mermaid class detection Main bundle: 2,740KB → 1,840KB Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ef0c6b9 to
f0c00e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/components/chat/markdownComponents.tsx (1)
162-168: Extract a sharedisCodeElementtype guard.This inline probe is duplicated in
src/renderer/components/chat/viewers/MarkdownViewer.tsxat line 202, and the cast is still doing the real narrowing. A namedisCodeElementhelper keeps the HAST check in one place and matches the repo'sisXxxruntime-guard convention.♻️ Suggested refactor
+const isCodeElement = ( + child: unknown +): child is { tagName: 'code'; properties?: { className?: string | string[] } } => + typeof child === 'object' && child !== null && 'tagName' in child && child.tagName === 'code'; + // Detect mermaid: check if the <code> child has class "language-mermaid" - const codeEl = node?.children?.find((c) => 'tagName' in c && c.tagName === 'code') as - | { properties?: { className?: string[] } } - | undefined; + const codeEl = node?.children?.find(isCodeElement);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/chat/markdownComponents.tsx` around lines 162 - 168, Extract a shared runtime type guard named isCodeElement that checks a HAST node has tagName 'code' and an optional properties.className array, then replace the inline probe in pre (in markdownComponents.tsx) and the duplicate in MarkdownViewer.tsx (around the previous inline cast) to call isCodeElement(nodeChild) instead of doing the ad-hoc find+cast; update the isMermaid check to use isCodeElement result and then safely access properties.className.includes('language-mermaid'), removing the explicit cast usage and keeping the guard in one exported helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx`:
- Around line 167-171: The Suspense fallback in MarkdownViewer currently renders
an inline <code> element which collapses multi-line Mermaid diagrams; change the
fallback used in the <React.Suspense> wrapping MermaidViewer to the same
block-style, whitespace-preserving fallback used in
src/renderer/components/chat/markdownComponents.tsx (i.e., render a block
element that preserves whitespace such as a <pre> with the same
classes/structure used there) so multi-line diagrams remain readable while the
lazy import resolves; update the fallback in the React.Suspense inside
MarkdownViewer around MermaidViewer accordingly.
---
Nitpick comments:
In `@src/renderer/components/chat/markdownComponents.tsx`:
- Around line 162-168: Extract a shared runtime type guard named isCodeElement
that checks a HAST node has tagName 'code' and an optional properties.className
array, then replace the inline probe in pre (in markdownComponents.tsx) and the
duplicate in MarkdownViewer.tsx (around the previous inline cast) to call
isCodeElement(nodeChild) instead of doing the ad-hoc find+cast; update the
isMermaid check to use isCodeElement result and then safely access
properties.className.includes('language-mermaid'), removing the explicit cast
usage and keeping the guard in one exported helper.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 630aa2b4-3545-4379-b20b-767779e0bc9e
📒 Files selected for processing (3)
src/renderer/components/chat/markdownComponents.tsxsrc/renderer/components/chat/viewers/MarkdownViewer.tsxsrc/renderer/components/chat/viewers/MermaidViewer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/components/chat/viewers/MermaidViewer.tsx
| return ( | ||
| <React.Suspense fallback={<code className="font-mono text-xs">{text}</code>}> | ||
| <MermaidViewer code={text} /> | ||
| </React.Suspense> | ||
| ); |
There was a problem hiding this comment.
Render the Mermaid lazy fallback as a block with preserved whitespace.
Once pre returns Mermaid children directly, this fallback is the only UI during the first chunk load. As an inline <code> without whitespace preservation, multi-line diagrams collapse into unreadable text until the import resolves. src/renderer/components/chat/markdownComponents.tsx at Lines 135-137 has a parallel fallback, so this is a good place to share one block-style fallback.
💡 Minimal fix
- <React.Suspense fallback={<code className="font-mono text-xs">{text}</code>}>
+ <React.Suspense
+ fallback={
+ <code
+ className="block whitespace-pre-wrap font-mono text-xs"
+ style={{ color: COLOR_TEXT }}
+ >
+ {text}
+ </code>
+ }
+ >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/chat/viewers/MarkdownViewer.tsx` around lines 167 -
171, The Suspense fallback in MarkdownViewer currently renders an inline <code>
element which collapses multi-line Mermaid diagrams; change the fallback used in
the <React.Suspense> wrapping MermaidViewer to the same block-style,
whitespace-preserving fallback used in
src/renderer/components/chat/markdownComponents.tsx (i.e., render a block
element that preserves whitespace such as a <pre> with the same
classes/structure used there) so multi-line diagrams remain readable while the
lazy import resolves; update the fallback in the React.Suspense inside
MarkdownViewer around MermaidViewer accordingly.
Mermaid was statically imported, bundling ~900KB into the main renderer chunk. This caused V8 parse/compile overhead and GC pressure on every interaction in the production build (dev mode was unaffected since Vite serves modules on-demand).
Main bundle: 2,740KB → 1,840KB
Summary by CodeRabbit