Skip to content

Lazy load the editor bundle#535

Open
yusufm wants to merge 1 commit intosiddharthvaddem:mainfrom
yusufm:codex/lazy-load-editor
Open

Lazy load the editor bundle#535
yusufm wants to merge 1 commit intosiddharthvaddem:mainfrom
yusufm:codex/lazy-load-editor

Conversation

@yusufm
Copy link
Copy Markdown
Contributor

@yusufm yusufm commented May 3, 2026

Summary

  • lazy-load VideoEditor and the shortcuts dialog only for editor windows
  • keep launch/HUD windows on the smaller initial renderer bundle
  • fix the linted hook dependency used by CI

Testing

  • npm run lint
  • npm run build-vite
  • npm run test:browser

Summary by CodeRabbit

  • Performance Improvements

    • Optimized app initialization with lazy-loaded components for faster startup times.
  • Refactor

    • Improved cleanup handling in screen recorder functionality for enhanced reliability.

@yusufm yusufm requested a review from siddharthvaddem as a code owner May 3, 2026 21:21
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 3, 2026

📝 Walkthrough

Walkthrough

App.tsx now lazy-loads VideoEditor and ShortcutsConfigDialog components with dynamic imports and wraps them in a Suspense boundary. useScreenRecorder.ts converts a countdown overlay helper function into a stable useCallback and updates its effect dependency array.

Changes

Component Code Splitting

Layer / File(s) Summary
Dynamic Import Setup
src/App.tsx
VideoEditor and ShortcutsConfigDialog replaced with React.lazy() definitions using .then() to expose named exports as defaults.
Suspense Boundary
src/App.tsx
Editor-mode content wrapped in Suspense with h-screen bg-background fallback div; replaces direct component rendering.

Hook Cleanup Memoization

Layer / File(s) Summary
Memoized Cleanup Helper
src/hooks/useScreenRecorder.ts
safeHideCountdownOverlay converted to useCallback with stable reference for async electron API interaction and error logging.
Dependency Array Update
src/hooks/useScreenRecorder.ts
Effect cleanup dependency array updated to [teardownMedia, safeHideCountdownOverlay] to properly coordinate with memoized callback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • siddharthvaddem

Poem

🎬 components now lazy-load with grace,
suspense keeps fallback in its place,
cleanup callbacks memoized so tight,
countdown overlays hiding at night,
code splits clean, at 2am we're alright ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Lazy load the editor bundle' directly matches the main change: lazy-loading VideoEditor and ShortcutsConfigDialog to reduce initial bundle size.
Description check ✅ Passed The description covers the core changes and includes testing steps, but doesn't follow the template structure with formal sections like 'Motivation', 'Type of Change', or 'Related Issue(s)'.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share
Review rate limit: 4/8 reviews remaining, refill in 23 minutes and 40 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/App.tsx (1)

64-70: ⚡ Quick win

Split the suspense boundary for the dialog.

Right now the editor shell is blocked until both lazy chunks resolve, so the shortcuts dialog still sits on the critical path for first paint. Kinda defeats part of the bundle win.

♻️ Small tweak to keep the editor rendering independently
 				case "editor":
 					return (
 						<ShortcutsProvider>
 							<Suspense fallback={<div className="h-screen bg-background" />}>
 								<VideoEditor />
 							</Suspense>
-							<ShortcutsConfigDialog />
+							<Suspense fallback={null}>
+								<ShortcutsConfigDialog />
+							</Suspense>
 						</ShortcutsProvider>
 					);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 64 - 70, The Suspense currently wraps both
<VideoEditor /> and <ShortcutsConfigDialog />, blocking editor render until the
dialog chunk loads; move the Suspense boundary so <VideoEditor /> renders
immediately and only <ShortcutsConfigDialog /> is lazy-wrapped. Concretely,
inside the "editor" case keep <ShortcutsProvider> as the parent, render
<VideoEditor /> directly, then add a <Suspense fallback={...}> that only returns
<ShortcutsConfigDialog />; ensure the fallback preserves layout (e.g., same
height/background) and that the import/name ShortcutsConfigDialog remains the
lazy component being suspense-wrapped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/App.tsx`:
- Around line 64-70: The Suspense currently wraps both <VideoEditor /> and
<ShortcutsConfigDialog />, blocking editor render until the dialog chunk loads;
move the Suspense boundary so <VideoEditor /> renders immediately and only
<ShortcutsConfigDialog /> is lazy-wrapped. Concretely, inside the "editor" case
keep <ShortcutsProvider> as the parent, render <VideoEditor /> directly, then
add a <Suspense fallback={...}> that only returns <ShortcutsConfigDialog />;
ensure the fallback preserves layout (e.g., same height/background) and that the
import/name ShortcutsConfigDialog remains the lazy component being
suspense-wrapped.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6452a542-3131-4339-9562-076f88d1429c

📥 Commits

Reviewing files that changed from the base of the PR and between 7e00cdb and 42c596d.

📒 Files selected for processing (2)
  • src/App.tsx
  • src/hooks/useScreenRecorder.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant