Repository navigation
Split webviews bundle by surface (code splitting) - #5613
Merged
Merged
Conversation
Move the webviews React build off Vite library mode + inlineDynamicImports to a single-entry app build that emits per-surface chunks. main.tsx becomes a slim dispatcher that dynamically imports only the active surface, so the agent session no longer ships the diff viewer and vice versa. The diff syntax-highlighting vendor (@pierre/diffs + shiki grammars) collapses into one lazy diff-vendor chunk loaded only by the diff surface. Left split, shiki emits ~300 grammar files that both duplicate the vendored diff worker grammars and push the diff viewer custom scheme's per-token allowlist toward its 1024-file cap; per-grammar laziness is a follow-up. Agent session payload drops from the 11.6MB monolith to a 0.6KB entry + 277KB shared vendor + 360KB surface chunk. Both serving paths already handle sibling chunks: the diff viewer custom scheme registers every emitted .js/.mjs, and the agent-session file load grants read access to the whole output directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThe PR refactors webviews from single-app routing to surface-specific mount functions with dynamic imports and splits the bundle into optimized chunks. A new style injection utility and surface bootstrap functions enable conditional mounting. The Vite build is reconfigured to emit stable named chunks with manual dependency splitting, and the verification script validates React Compiler optimization across all outputs. ChangesWebview Surface Mount Refactor and Build Optimization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 17 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (17 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
💡 Codex Review Keep diff-vendor out of the entry module
When ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Contributor
Greptile SummaryThis PR replaces the monolithic Vite library build (single 11.6 MB
Confidence Score: 5/5Safe to merge — purely frontend bundling and committed static assets; both serving paths already handle sibling ESM chunks and the code split is correctly implemented in the committed output. The compiled main.mjs statically imports only vendor.mjs (the preload helper), confirming diff-vendor is never fetched on the agent-session page. The manualChunks vite/preload-helper pin resolves the previously flagged entry-point static pull of diff-vendor. No Swift or CLI changes, no runtime behavior change beyond which chunks load per surface. No files require special attention. The committed chunks match the described split and the React Compiler guard has been updated to scan the new chunk layout. Important Files Changed
Sequence DiagramsequenceDiagram
participant Host as macOS Host (Swift)
participant Entry as main.mjs (0.6 KB)
participant Vendor as vendor.mjs (277 KB)
participant AgentChunk as agentSessionSurface.mjs (360 KB)
participant DiffChunk as diffSurface.mjs (108 KB)
participant DiffVendor as diff-vendor.mjs (10.3 MB)
Host->>Entry: loadFileURL / custom scheme
Entry->>Vendor: static import (__vitePreload helper + React/router)
Entry->>Entry: resolveWebviewKind()
alt Agent Session page
Entry->>AgentChunk: dynamic import()
AgentChunk-->>Entry: mountAgentSessionSurface()
Note over DiffVendor: Never loaded
else Diff Viewer page
Entry->>DiffChunk: dynamic import()
DiffChunk->>DiffVendor: "static import (@pierre/diffs + shiki)"
DiffChunk-->>Entry: mountDiffSurface()
end
Reviews (3): Last reviewed commit: "Use stable webview chunk names to bound ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // serving paths already handle sibling chunks: the diff viewer custom | ||
| // scheme registers every emitted `.js`/`.mjs`, and the agent-session file | ||
| // load grants read access to the whole output directory. | ||
| modulePreload: false, |
Contributor
There was a problem hiding this comment.
modulePreload: false disables <link rel="modulepreload"> injection in HTML, but Vite still emits its __vitePreload runtime helper for dynamic imports. That helper ends up as an export of diff-vendor, causing main.mjs to statically import that 10.3 MB chunk on every page load. Using { polyfill: false } explicitly tells Vite to omit the helper entirely, removing the cross-chunk static dependency.
Suggested change
| modulePreload: false, | |
| modulePreload: { polyfill: false }, |
Comment on lines
+23
to
31
| if (resolveWebviewKind() === "agent-session") { | ||
| void import("./surfaces/agentSessionSurface").then((surface) => { | ||
| surface.mountAgentSessionSurface(rootElement); | ||
| }); | ||
| } else { | ||
| void import("./surfaces/diffSurface").then((surface) => { | ||
| surface.mountDiffSurface(rootElement); | ||
| }); | ||
| const initialStatus = initialDiffViewerStatus(config, label); | ||
| document.body.dataset.filesHidden = "false"; | ||
| applyDiffViewerStatusToDocument(initialStatus); | ||
| return { config, initialStatus }; | ||
| } | ||
|
|
||
| function setupAgentSession() { | ||
| installStyles("agent-session", agentSessionStyles); | ||
| applyCodexDocumentMetadata(); | ||
| document.documentElement.dataset.cmuxWebviewKind = "agent-session"; | ||
| document.body.dataset.cmuxWebviewKind = "agent-session"; | ||
| } | ||
|
|
||
| function RoutedWebview() { | ||
| if (webviewKind === "agent-session") { | ||
| return <AgentSessionApp />; | ||
| } | ||
| if (!diffRuntime) { | ||
| throw new Error("Missing cmux diff viewer runtime"); | ||
| } | ||
| return <App config={diffRuntime.config} initialStatus={diffRuntime.initialStatus} />; | ||
| } | ||
|
|
||
| const router = createWebviewsRouter(RoutedWebview); | ||
|
|
||
| declare module "@tanstack/react-router" { | ||
| interface Register { | ||
| router: typeof router; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
void import(…).then(…) call swallows the rejection silently. The previous synchronous setup would have thrown immediately. Adding a .catch ensures the failure surfaces as an unhandled error rather than leaving a blank webview with no diagnostic.
Suggested change
| if (resolveWebviewKind() === "agent-session") { | |
| void import("./surfaces/agentSessionSurface").then((surface) => { | |
| surface.mountAgentSessionSurface(rootElement); | |
| }); | |
| } else { | |
| void import("./surfaces/diffSurface").then((surface) => { | |
| surface.mountDiffSurface(rootElement); | |
| }); | |
| const initialStatus = initialDiffViewerStatus(config, label); | |
| document.body.dataset.filesHidden = "false"; | |
| applyDiffViewerStatusToDocument(initialStatus); | |
| return { config, initialStatus }; | |
| } | |
| function setupAgentSession() { | |
| installStyles("agent-session", agentSessionStyles); | |
| applyCodexDocumentMetadata(); | |
| document.documentElement.dataset.cmuxWebviewKind = "agent-session"; | |
| document.body.dataset.cmuxWebviewKind = "agent-session"; | |
| } | |
| function RoutedWebview() { | |
| if (webviewKind === "agent-session") { | |
| return <AgentSessionApp />; | |
| } | |
| if (!diffRuntime) { | |
| throw new Error("Missing cmux diff viewer runtime"); | |
| } | |
| return <App config={diffRuntime.config} initialStatus={diffRuntime.initialStatus} />; | |
| } | |
| const router = createWebviewsRouter(RoutedWebview); | |
| declare module "@tanstack/react-router" { | |
| interface Register { | |
| router: typeof router; | |
| } | |
| } | |
| if (resolveWebviewKind() === "agent-session") { | |
| import("./surfaces/agentSessionSurface") | |
| .then((surface) => { | |
| surface.mountAgentSessionSurface(rootElement); | |
| }) | |
| .catch((err) => { | |
| throw err; | |
| }); | |
| } else { | |
| import("./surfaces/diffSurface") | |
| .then((surface) => { | |
| surface.mountDiffSurface(rootElement); | |
| }) | |
| .catch((err) => { | |
| throw err; | |
| }); | |
| } |
The slim entry statically imported the 10MB diff-vendor chunk because Rollup co-located Vite's dynamic-import preload helper there, so opening an agent session eagerly fetched the diff/shiki bundle and defeated the split. Pin the preload helper to the always-shared vendor chunk via manualChunks so the entry only statically imports vendor; diff-vendor is now imported solely by the diff surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Content-hashed chunk names orphaned a new ~10MB diff-vendor copy in the diff viewer's long-lived /tmp/cmux-diff-viewer-$uid/assets/cmux-webviews-app cache on every rebuild, since nothing prunes that dir and the copy step overwrites by size+mtime. Drop the content hash so chunk names are stable and overwrite in place (matching the prior single main.mjs behavior). The bundle is served via the diff viewer custom scheme and a versioned app-bundle file load, so content-hash cache-busting is not needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 8, 2026
Resolve the only conflict (the generated webviews bundle) by regenerating it from the merged source with scripts/build-webviews-app.sh. main #5613 code-split the webviews bundle into per-surface chunks; the diff surface-fill CSS now lives in chunks/diffSurface.mjs. Verified with build-webviews-app.sh --check (exit 0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 8, 2026
This was referenced Jun 10, 2026
This was referenced Jun 14, 2026
Closed
This was referenced Jun 24, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves the
webviews/React build off Vite library mode +inlineDynamicImports(one 11.6 MBmain.mjs) to a single-entry app build that emits per-surface chunks.main.tsxis now a slim dispatcher that dynamically imports only the active surface, so the agent session no longer ships the diff viewer and vice versa.The diff syntax-highlighting vendor (
@pierre/diffs+ shiki grammars) collapses into one lazydiff-vendorchunk loaded only by the diff surface. Left fully split, shiki emits ~300 grammar files that both duplicate the vendored diff worker grammars and push the diff viewer custom scheme's per-token allowlist toward its 1024-file cap. Per-grammar lazy loading (and de-duplicating against the worker copy) is a follow-up once that cap is revisited.This is groundwork for adding a Monaco editor surface as its own lazy chunk without bloating the shared diff bundle.
Result
Output now (was: single
main.mjs11.6 MB):main.mjs(entry)vendor(React, router)agentSessionSurfacediffSurfacediff-vendor(@pierre + shiki)Agent session payload: 11.6 MB -> ~0.6 KB entry + 277 KB shared + 360 KB surface.
Why this is safe
No Swift/CLI changes needed. Both serving paths already handle sibling chunks:
CmuxDiffViewerURLSchemeHandler) registers every emitted.js/.mjsvia the CLI's recursive asset enumeration, and ESM sub-import + worker chunk loading through the scheme is already proven in production (the diff worker does 306 dynamicimport()s of./chunks/*.mjsshiki grammars).loadFileURLgrants read access to the whole output directory.File count stays well under the 1024-file allowlist cap (5 webviews files + 709 vendored pierre files).
Verification
bun run typecheck,bun run lint:ci,bun run verify:tanstack-routerpass../scripts/build-webviews-app.sh --checkpasses (deterministic chunk hashes, reproducible committed output).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Bundling and committed static assets only; runtime behavior and native loaders are unchanged, with CI guarding reproducible builds.
Overview
Replaces the webviews Vite library build (single ~11.6 MB
main.mjsviainlineDynamicImports) with a single-entry app build that code-splits by surface.main.tsxis now a small bootstrap thatimport()s onlyagentSessionSurfaceordiffSurfaceat runtime. Each surface module mounts its own UI and installs its CSS viainstallWebviewStyles. RollupmanualChunkshoist shared React/router intovendor, isolate@pierre/diffs+ shiki into lazydiff-vendor(loaded only by the diff surface), and pin Vite’s preload helper tovendorso the agent session never statically pulls the diff vendor bundle. Chunk filenames stay stable (unhashed) to match how the diff viewer caches assets under/tmp.The React Compiler guard now scans
main.mjsandchunks/*.mjs. Committed output underResources/markdown-viewer/webviews-appis regenerated; no Swift/CLI changes—existing custom-scheme andloadFileURLpaths already serve sibling ESM chunks.Reviewed by Cursor Bugbot for commit d36d185. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Split the webviews bundle by surface and lazy-load each UI on demand. Added stable chunk names to avoid diff-viewer cache bloat; agent session payload drops from 11.6 MB to ~640 KB and the diff-only vendor stays in a lazy
diff-vendorchunk.Refactors
inlineDynamicImportswith a single-entry app build that emits per-surface chunks.main.tsxnow dispatches via dynamicimport()toagentSessionSurfaceordiffSurface.manualChunksfor sharedvendorand diff-onlydiff-vendor(@pierre/diffs+shiki); pinned Vite’s preload helper tovendorsodiff-vendoris only loaded by the diff surface.surfaces/*and an inline style installer so each surface ships its own CSS.main.mjsandchunks/*.mjs.main.mjsandchunks/*.mjsnames to overwrite in place and prevent orphaned ~10 MBdiff-vendorcopies in the diff-viewer asset cache.Migration
Written for commit d36d185. Summary will update on new commits.
Summary by CodeRabbit