perf: lazy-load mobile menu content to reduce RSC payload#17661
Conversation
Move mobile menu content from server component to client-side lazy-loaded component. This avoids shipping all navigation translations and language display info in the initial RSC payload since the mobile menu is hidden behind a click. Changes: - Add MobileMenuContent.tsx: Client component with the actual menu content - MobileMenuClient.tsx: Uses React.lazy() to load content on first open - MobileMenu/index.tsx: Simplified to just render MobileMenuClient - LvlAccordion.tsx: Remove async keyword that caused repeated Suspense - useLanguagesDisplayInfo.ts: Client hook to generate language info This saves ~82KB from the initial HTML payload by deferring navigation data until the user actually opens the mobile menu.
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Review: perf: lazy-load mobile menu content to reduce RSC payload
The core optimization is sound -- deferring mobile menu content from the RSC payload to a client-side lazy load is a real win for mobile users. The menu is behind a hamburger click, so there's no reason to SSR it. A few suggestions below.
1. Missing error boundary around lazy load (medium)
MobileMenuClient.tsx:68-72 -- If the dynamic import fails (flaky mobile network, CDN hiccup), React.lazy throws and propagates upward with no recovery path. This is a realistic failure mode on mobile.
Suggestion: wrap the Suspense in an error boundary with a retry mechanism so the menu isn't permanently broken on a transient network failure.
2. useLanguagesDisplayInfo should memoize its result (low-medium)
src/hooks/useLanguagesDisplayInfo.ts:20-31 -- This creates ~100 Intl.DisplayNames / Intl.NumberFormat objects (4 per locale x 25 locales) on every render. Intl constructors are among the most expensive synchronous JS operations. While re-renders should be infrequent thanks to PersistentPanel staying mounted, any parent context change would recompute everything.
Suggestion:
return useMemo(
() => (FILTERED_LOCALES as Lang[]).map((localeOption) =>
localeToDisplayInfo(localeOption, locale as Lang, t)
),
[locale, t]
)3. hasBeenOpened state is redundant with PersistentPanel (low)
MobileMenuClient.tsx:39-43 -- PersistentPanel already returns null until first open (its internal isMounted state), which means children never render until then. React.lazy only triggers the import when the component actually renders in the tree, not when JSX elements are created. So the hasBeenOpened guard is defense-in-depth rather than strictly necessary. Not harmful, but worth a comment noting the intent if kept.
4. Consider prefetching the chunk during idle time (low)
On slow 3G, the first menu open now has to download + parse the JS chunk before anything renders. The skeleton helps with perceived latency, but a requestIdleCallback-based prefetch (or <link rel="prefetch">) could eliminate the download penalty while keeping the RSC payload savings.
5. Skeleton doesn't match actual menu layout (nitpick)
MobileMenuClient.tsx:21-28 -- The skeleton shows 5 uniform rectangles, but the real menu has a header, tabbed nav sections, and a 3-column footer bar. This causes a visual jump on first open. Matching the skeleton to the real structure would smooth the transition.
Overall this is a well-executed optimization -- the tradeoff (slightly slower first open for significantly smaller initial payload) is the right call for a mobile menu. The error boundary is the main thing I'd want addressed before merge.
Reviewed by Claude (claude-opus-4-6)
wackerow
left a comment
wackerow
left a comment
There was a problem hiding this comment.
@pettinarip Thanks for this! Looks great overall, left the claude review above.. Curious your take on the Error boundary. The skeleton looks fine imo. Approving as-is, but ofc feel free to update if you think it's worth it.
|
@wackerow thanks for the review. I've applied most of them. Also, improved a bit the skeleton. |
wackerow
left a comment
wackerow
left a comment
There was a problem hiding this comment.
@pettinarip Looks good.. left a couple notes, but nothing that needs to block. Will leave it up to you but think this is good to go for now
| const lazyImport = () => import("./MobileMenuContent") | ||
| const MobileMenuContent = React.lazy(lazyImport) | ||
|
|
||
| function MobileMenuContentSkeleton() { |
| if (typeof window.requestIdleCallback === "function") { | ||
| const id = window.requestIdleCallback(() => lazyImport()) | ||
| return () => window.cancelIdleCallback(id) | ||
| } |
github-actions
Bot
added
content 🖋️
3 participants
Summary
Changes
React.lazy()to load content on first openasynckeyword that caused repeated SuspenseTest plan