perf: skip RecentPostsWidget fetch when its rail is hidden on mobile#1024
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tanstack-com | eefd557 | Commit Preview URL Branch Preview URL |
Jul 02 2026, 05:37 AM |
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA shared ChangesConditional RecentPostsWidget fetching
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/LibraryLayout.tsx (1)
886-886: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winUse a desktop query for the query gate
useMediaQuery('(max-width: 767.98px)')starts asfalse, soenabled={!shouldShowDocsPartnerSlot}istrueon the first render andRecentPostsWidgetcan still fetch on mobile before the media query flips. Match the blog route and gate this with a positive desktop query instead.🤖 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/components/LibraryLayout.tsx` at line 886, The query gate in LibraryLayout’s docs partner/recent posts logic is using a mobile max-width check that starts false, so RecentPostsWidget can fetch on the first mobile render before the media query updates. Change shouldShowDocsPartnerSlot to use a positive desktop query in LibraryLayout so enabled tracks desktop immediately, and keep the gating tied to that desktop-aware boolean.
🧹 Nitpick comments (1)
src/routes/blog.$.tsx (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a shared breakpoint constant instead of duplicated literal strings.
'(min-width: 768px)'here and'(max-width: 767.98px)'inLibraryLayout.tsxboth hand-encode the Tailwindmdbreakpoint (also referenced viaRightRail breakpoint="md"andhidden md:block). Centralizing this in one constant would reduce drift risk if the breakpoint ever changes.Also applies to: 78-78
🤖 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/routes/blog`.$.tsx at line 12, The responsive breakpoint is duplicated as literal media-query strings, which can drift from the Tailwind md breakpoint used elsewhere. Introduce and reuse a shared breakpoint constant in the components using useMediaQuery and LibraryLayout, and align RightRail breakpoint="md" and the md-based visibility classes to that same source so the breakpoint definition lives in one place.
🤖 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.
Outside diff comments:
In `@src/components/LibraryLayout.tsx`:
- Line 886: The query gate in LibraryLayout’s docs partner/recent posts logic is
using a mobile max-width check that starts false, so RecentPostsWidget can fetch
on the first mobile render before the media query updates. Change
shouldShowDocsPartnerSlot to use a positive desktop query in LibraryLayout so
enabled tracks desktop immediately, and keep the gating tied to that
desktop-aware boolean.
---
Nitpick comments:
In `@src/routes/blog`.$.tsx:
- Line 12: The responsive breakpoint is duplicated as literal media-query
strings, which can drift from the Tailwind md breakpoint used elsewhere.
Introduce and reuse a shared breakpoint constant in the components using
useMediaQuery and LibraryLayout, and align RightRail breakpoint="md" and the
md-based visibility classes to that same source so the breakpoint definition
lives in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd79e5f9-1bf4-4012-ab4a-02a739e82c85
📒 Files selected for processing (4)
src/components/LibraryLayout.tsxsrc/components/RecentPostsWidget.tsxsrc/routes/blog.$.tsxsrc/utils/useMediaQuery.ts
Summary
RecentPostsWidget(the "Latest Posts" box in the right rail) is mounted unconditionally on every docs page (LibraryLayout.tsx) and every blog post page (blog.$.tsx), with nopostsprop passed in either case. That means it always fires its ownuseQueryfetch on mount.The rail it lives in is hidden below the
mdbreakpoint via CSS only (hidden md:block—display:none, not a conditional unmount). So on mobile, the component was still mounting and still firing the fetch even though the box was never visible to the user.This fixes that by threading the same breakpoint the CSS already uses into the query's
enabledflag, so the fetch itself is skipped on mobile instead of just being hidden after the fact.Changes
src/utils/useMediaQuery.ts(new) — hoisted out ofLibraryLayout.tsxwhere it already existed as a private helper, soblog.$.tsxcan reuse it too. No behavior change to the existing call site.src/components/RecentPostsWidget.tsx— newenabled?: booleanprop (defaulttrue), threaded into the query asenabled: posts === undefined && enabled.blog.index.tsxalready passespostsexplicitly and is unaffected either way.src/components/LibraryLayout.tsx— passesenabled={!shouldShowDocsPartnerSlot}, reusing the existingshouldShowDocsPartnerSlotmedia query (max-width: 767.98px) already computed in this file for a different mobile-only element, instead of adding a secondmatchMedialistener.src/routes/blog.$.tsx— passesenabled={isDesktopViewport}via a newuseMediaQuery('(min-width: 768px)')call (no existing media query call here to reuse).Why this is safe
enabledflag changes, and it evaluatestrueon desktop just like before.useMediaQuerydefaults tofalseviauseState, deterministically, on both server and the first client render — server and client agree before the media query listener ever runs, so there's nothing for React to warn about.@tanstack/react-queryv5.100.11 source:enabled: false → truetriggers exactly one fetch, no re-fetch loop.criticandrubber-duckbefore implementation — no blocking issues from either.criticsuggested reusing the existingshouldShowDocsPartnerSlotvalue instead of adding a second listener inLibraryLayout.tsx, which is what's implemented.rubber-duckcaught thatblog.$.tsxneeded the same fix asLibraryLayout.tsx(an earlier plan draft had missed it).Scope note
This does not touch the request itself being a client-fetch-on-mount rather than SSR-loaded/prefetched data — that's a separate, larger question and out of scope here. This PR only stops the fetch from firing when its result can never be seen.
Summary by CodeRabbit