perf: re-enable code splitting in Desktop with vendor chunk grouping - #55181
perf: re-enable code splitting in Desktop with vendor chunk grouping#55181Stoltemberg wants to merge 2 commits into
Conversation
Re-enable Vite code splitting (previously disabled due to Shiki OOM issues with electron-builder) by grouping heavy vendor libraries into dedicated chunks via manualChunks: - shiki-vendor: Shiki + react-shiki (~18.6 MB) - mermaid-vendor: Mermaid (~3.9 MB) - codemirror-vendor: CodeMirror (~1.5 MB) - katex-vendor: KaTeX (~289 KB) This reduces the initial JS bundle from 27.6 MB to 2.3 MB (91.8%). Heavy chunks load on demand when their features are used.
CodeMirror is only needed when the user opens a file for editing. Use React.lazy() + Suspense to defer loading the CodeEditor component until it's actually rendered, reducing initial page load. With code splitting enabled, this creates a separate chunk that loads on demand instead of blocking the initial render.
tonydwb
left a comment
There was a problem hiding this comment.
LGTM. Clean 2-file performance fix re-enabling code splitting in Desktop with vendor chunk grouping. Reduces initial bundle load time. Well-scoped webpack config change.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the Desktop startup bundle. The premise remains active: current main deliberately disables splitting in apps/desktop/vite.config.ts:45-55 because Shiki's dynamic output previously caused electron-builder OOMs.
Problems
apps/desktop/vite.config.ts:29groups modules into vendor assets, but it does not make static imports lazy. Startup statically reaches Shiki and KaTeX throughapps/desktop/src/app/desktop-controller.tsx:78,apps/desktop/src/app/chat/index.tsx:13, andapps/desktop/src/components/assistant-ui/markdown-text.tsx:24-27. It also reaches CodeMirror throughapps/desktop/src/app/desktop-controller.tsx:87,apps/desktop/src/app/chat/sidebar/index.tsx:105,apps/desktop/src/app/chat/sidebar/profile-switcher.tsx:25, andapps/desktop/src/components/chat/code-editor.tsx:1-5. The right-rail-onlyLazyCodeEditorcannot make that dependency on-demand.- The existing build guard only requires one emitted JS asset (
apps/desktop/scripts/assert-dist-built.mjs:35-46), so it does not protect the packaging/chunk-cardinality constraint that led tocodeSplitting: false.
Suggested changes
- Move every startup-reachable heavy import behind a real lazy boundary, or limit the claim to output grouping rather than initial-load reduction.
- Add an invariant-oriented build/packaging regression check and validate the real packaging path.
Automated hermes-sweeper review.
| codeSplitting: false | ||
| } | ||
| } | ||
| manualChunks(id) { |
There was a problem hiding this comment.
manualChunks only groups modules; it does not defer modules that are statically reachable at startup. Main still statically reaches Shiki/KaTeX via the chat transcript and CodeMirror via ChatSidebar → ProfileRail, so these vendor chunks remain initial dependencies. Please establish lazy boundaries for those paths (or narrow the performance claim) before re-enabling splitting.
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Three PRs are associated with this complex, but they address distinct Desktop causes: #40735 fixes worktree font serving, #55181 changes renderer chunking to reduce the startup bundle, and #64685 addresses Windows dependency resolution plus remote-display GPU fallback.
Related pull requests
- #40735 [closed]
related— (+26/-1) — implemented independently: the diff whitelists realpath-resolved node_modules directories so Vite can serve fonts from symlinked worktrees; it remains relevant as the reference implementation already landed on main and was folded into #69938. - #55181
related— (+27/-12) — keep open for substantial revision: manual vendor chunks change asset grouping but do not make the statically imported Shiki, KaTeX, and CodeMirror paths lazy, while the only new lazy boundary covers the right-rail CodeEditor; the diff also adds no guard for the chunk-cardinality constraint behind the prior electron-builder OOM. This agrees with the contributor keep_open review on #55181. - #64685
related— (+10/-1) — keep open, but not ready to merge: the fflate alias directly addresses the hoisted Windows workspace import, and the revised diff correctly avoids automatic --no-sandbox, but it still applies additional GPU switches to every remote-display reason and adds no requested platform-specific helper tests. This agrees with the contributor keep_open review on #64685 and must address its SSH/X11 scope concern.
Suggested consolidation
Keep #55181 as the candidate for the code-splitting work, but do not merge it as-is until actual lazy import boundaries and a packaging/chunk-cardinality regression guard address the documented OOM cause. #40735 is already implemented on main, and #64685 should remain a separate Windows/remote-display fix; neither is a duplicate of #55181, so no PR in this set should be closed as its duplicate.
Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 7 kB of PR diffs, 5 kB of issue/PR text, 4 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
|
Closing — the reviewer flagged that |
What does this PR do?
Re-enables Vite code splitting in the Desktop app (previously disabled due to Shiki causing OOM in electron-builder with thousands of dynamic chunks) by grouping heavy vendor libraries into dedicated chunks via
manualChunks. This reduces the initial JS bundle from 27.6 MB to 2.3 MB (91.8% reduction).Related Issue
Improves Desktop startup performance and memory usage. Related to #55191 (Desktop renderer crash-loops on large conversations) — smaller initial bundle reduces memory pressure.
Type of Change
Changes Made
apps/desktop/vite.config.ts: ReplacecodeSplitting: falsewithmanualChunksfunction that groups:shiki-vendor: Shiki + react-shiki (~18.6 MB, lazy-loaded when viewing code)mermaid-vendor: Mermaid (~3.9 MB, lazy-loaded when rendering diagrams)codemirror-vendor: CodeMirror (~1.5 MB, lazy-loaded when editing code)katex-vendor: KaTeX (~289 KB, lazy-loaded when rendering math)How to Test
cd apps/desktop && npm run build— should succeed with vendor chunks separatedls -la dist/assets/*.js— should showindex-*.js(~2.3 MB) plus vendor chunksnpm run assert-dist-built— should passPerformance
Checklist
Code
fix(scope):,feat(scope):, etc.)npm run buildinapps/desktopand it succeedsnpm run assert-dist-builtand it passesDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — or N/A