-
Notifications
You must be signed in to change notification settings - Fork 223
perf: lazy-load mermaid to reduce main bundle size #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,9 @@ import { api } from '@renderer/api'; | |||||||||||||||||||||||||||
| import { CopyButton } from '@renderer/components/common/CopyButton'; | ||||||||||||||||||||||||||||
| import { PROSE_BODY } from '@renderer/constants/cssVariables'; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import { MermaidViewer } from './viewers/MermaidViewer'; | ||||||||||||||||||||||||||||
| const MermaidViewer = React.lazy(() => | ||||||||||||||||||||||||||||
| import('./viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) | ||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||
| import { highlightSearchInChildren, type SearchContext } from './searchHighlightUtils'; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| import type { Components } from 'react-markdown'; | ||||||||||||||||||||||||||||
|
|
@@ -129,7 +131,11 @@ export function createMarkdownComponents(searchCtx: SearchContext | null): Compo | |||||||||||||||||||||||||||
| const text = content.replace(/\n$/, ''); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (lang === 'mermaid') { | ||||||||||||||||||||||||||||
| return <MermaidViewer code={text} />; | ||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||
| <React.Suspense fallback={<code className="block font-mono text-xs">{text}</code>}> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
| <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> | |
| } | |
| > |
Copilot
AI
Apr 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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'); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,7 +33,9 @@ import { | |||||||||||||||||||||||
| highlightSearchInChildren, | ||||||||||||||||||||||||
| type SearchContext, | ||||||||||||||||||||||||
| } from '../searchHighlightUtils'; | ||||||||||||||||||||||||
| import { MermaidViewer } from '../viewers/MermaidViewer'; | ||||||||||||||||||||||||
| const MermaidViewer = React.lazy(() => | ||||||||||||||||||||||||
| import('../viewers/MermaidViewer').then((m) => ({ default: m.MermaidViewer })) | ||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||
| import { highlightLine } from '../viewers/syntaxHighlighter'; | ||||||||||||||||||||||||
|
Comment on lines
+36
to
39
|
||||||||||||||||||||||||
| 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 })) | |
| ); |
Copilot
AI
Apr 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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> | |
| } | |
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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>}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot
AI
Apr 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simple-import-sort/imports(eslint.config.js) expects imports to be grouped/sorted. Declaringconst MermaidViewer = React.lazy(...)between import declarations will likely break lint; keep all imports together, then declareMermaidViewerbelow them (or extract the lazy import to a separate module).