perf: route-level code splitting, honest bundle budget, Lighthouse audit (M9) - #11
Merged
Conversation
The whole app shipped as one bundle, so opening the landing page still downloaded the board, the react-chessboard library and the engine wrapper that only the game modes need. Load each route lazily behind a Suspense fallback, keeping just the home page eager so the landing paint — the one Lighthouse measures — needs no extra chunk. The initial JavaScript drops from about 212 kB to 155 kB gzipped; the mode chunks now arrive only when their route is opened.
With the routes split out, the guard's old "every .js file" sum no longer described the first load. Read the manifest instead and count only the entry and its static imports, reporting the lazy route chunks separately. The budget can now return to 200 kB — the promise made when M10 raised it to 230 for the Supabase client — with real headroom at 155 kB.
Exercise the initial-value fallback, reads of stored values, value and updater writes, and the two guarded paths — malformed JSON and a failing write — that must degrade to in-memory state rather than break the page.
The size guard now measures the entry and its static imports from the manifest; say so, so the table's "Initial JavaScript" is not read as the whole app's JS.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe app now lazy-loads non-home routes with a shared Suspense fallback. Vite emits a manifest used by bundle-size checks to separate initial assets from lazy chunks. Routing tests await lazy content, and ChangesLazy route loading and bundle accounting
Local storage hook validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Router
participant Suspense
participant ReactLazy
Router->>Suspense: render matched route
Suspense->>ReactLazy: load route chunk
ReactLazy-->>Suspense: provide page component
Suspense-->>Router: render page or RouteFallback
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/App.tsx (1)
38-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a
LazyRoutehelper to remove the repeatedSuspenseboilerplate.The
<Suspense fallback={<RouteFallback />}>...</Suspense>wrapper is duplicated across all 8 lazy routes. A small helper cuts the repetition and makes adding future lazy routes less error-prone (e.g. forgetting the fallback).♻️ Suggested refactor
+function LazyRoute({ Component }: { Component: React.ComponentType }) { + return ( + <Suspense fallback={<RouteFallback />}> + <Component /> + </Suspense> + ) +} + export default function App() { ... - <Route - path={ROUTES.coach} - element={ - <Suspense fallback={<RouteFallback />}> - <CoachPage /> - </Suspense> - } - /> + <Route path={ROUTES.coach} element={<LazyRoute Component={CoachPage} />} />Apply the same pattern to the other lazy routes (wrap
LeaderboardPagewithRequireAutharound<LazyRoute Component={LeaderboardPage} />).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 38 - 111, Extract a LazyRoute helper in App.tsx that accepts the lazy page component and renders it inside Suspense with RouteFallback. Replace the repeated Suspense wrappers for the eight lazy routes, including wrapping LazyRoute for LeaderboardPage with RequireAuth, while preserving each route path and existing authentication behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-bundle-size.mjs`:
- Around line 68-71: Update the lazyJs filter to exclude files matching the
existing /stockfish/i pattern, so Stockfish chunks are counted only by the
stockfish budget group and not in the informational lazy route chunk count or
size. Keep the existing JavaScript and initialJs filtering behavior unchanged.
---
Nitpick comments:
In `@src/App.tsx`:
- Around line 38-111: Extract a LazyRoute helper in App.tsx that accepts the
lazy page component and renders it inside Suspense with RouteFallback. Replace
the repeated Suspense wrappers for the eight lazy routes, including wrapping
LazyRoute for LeaderboardPage with RequireAuth, while preserving each route path
and existing authentication behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51c8678f-c60e-4bef-90b6-975e5b86e9dd
📒 Files selected for processing (7)
README.mdscripts/check-bundle-size.mjssrc/App.test.tsxsrc/App.tsxsrc/components/Layout/RouteFallback.tsxsrc/hooks/useLocalStorage.test.tsvite.config.ts
The engine worker is a lazily loaded .js outside the initial set, so it slipped into the informational "lazy route chunk" count and size — double counted against its own budget and mislabelled as a route. Exclude it from that tally.
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.
Final module — Tests & Optimisation (specification M9): "optimisation chargement Stockfish (lazy), tests hooks chess, audit Lighthouse performance/accessibilité".
Optimisation
Suspensefallback. The whole app used to ship as one bundle, so the landing page downloaded the board, react-chessboard and the engine wrapper that only the game modes need. Home stays eager so the Lighthouse-measured landing paint needs no extra chunk..js, so it measures the true first load. Budget returned to 200 kB (the value before M10 raised it to 230 for the Supabase client), with headroom at 155 kB. Lazy chunks are reported separately.Tests
useLocalStoragecoverage: initial-value fallback, stored reads, value/updater writes, and the two guarded paths (malformed JSON, failing write) that must degrade to in-memory state. The core chess hooks (useChessGame,useChessClock,usePuzzleSession,useHuntGame) were already covered.Lighthouse audit (local, after splitting)
Verification
npm run cigreen;lhci autorungreen.Summary by CodeRabbit