Closed
Conversation
- Remove Suspense/lazy boundary around hero splash logo to prevent fallback→loaded layout shift - Add route loader to prefetch stats, blog posts, and showcases server-side (fire-and-forget so DB errors don't block the page) - Change font-display from swap to optional and preload Inter font to prevent text reflow on load - Add explicit dimensions to navbar logo images and marquee brand logos - Match Suspense fallback wrapper in navbar to loaded component structure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
👷 Deploy request for tanstack pending review.Visit the deploys page to approve it
|
📝 WalkthroughWalkthroughAdds explicit image dimensions and tightens logo sizing, adjusts a Suspense fallback, preloads Inter font, changes Inter font-display to optional, and adds a TanStack Router loader using Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant RouteLoader as Route Loader
participant QueryClient
participant DataAPI as Data Source
participant Renderer as Server Renderer
Browser->>RouteLoader: GET /
RouteLoader->>QueryClient: ensureQueryData(recentPostsQueryOptions)
QueryClient->>DataAPI: fetch recent posts (if missing)
DataAPI-->>QueryClient: recent posts data
QueryClient-->>RouteLoader: cached data ready
RouteLoader->>Renderer: render route (uses cached data)
Renderer-->>Browser: HTML response (prefetched data hydrated)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Stats and showcases queries need DATABASE_URL — when they fail on the server, the cached error state causes server/client render divergence. Only prefetch blog posts (file-based) in the route loader. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
prefetchQuery is fire-and-forget — server may finish loading before render while client starts in loading state, causing element mismatch (<a> vs <div>). Use ensureQueryData to guarantee consistent state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/routes/index.tsx (2)
61-66: Make the fire-and-forget prefetch explicit.Line 65 is intentionally non-blocking. Prefixing the call with
voidmakes that intent obvious and avoidsno-floating-promisesambiguity, while preserving TanStack Router/Query's non-awaited loader-prefetch behavior. (tanstack.com)Suggested diff
loader: ({ context: { queryClient } }) => { // Prefetch blog posts server-side (file-based, no DB needed). // Stats and showcases are DB-dependent — let them load client-side // to avoid hydration mismatches when DATABASE_URL isn't set. - queryClient.prefetchQuery(recentPostsQueryOptions) + void queryClient.prefetchQuery(recentPostsQueryOptions) },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/index.tsx` around lines 61 - 66, The loader’s call to queryClient.prefetchQuery is intentionally non-blocking but should be made explicit to avoid no-floating-promises lint issues; update the loader (the arrow function that destructures context: { queryClient } and calls queryClient.prefetchQuery) to prefix the call with void (i.e., use void queryClient.prefetchQuery(...)) so the intent of a fire-and-forget prefetch is clear while preserving TanStack Router/Query’s non-awaited behavior.
137-146: Consider an emptyaltfor the hero splash.With the adjacent
<h1>already announcingTanStack, Line 143 will likely read as duplicate output for screen-reader users. If this image is decorative brand art rather than unique content,alt=""is the better fit. (w3.org)Suggested diff
<NetlifyImage src="/images/logos/splash-light.png" width={500} height={500} quality={85} className="w-[300px] pt-8 xl:pt-0 xl:w-[400px] 2xl:w-[500px]" - alt="TanStack Logo" + alt="" loading="eager" fetchPriority="high" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/index.tsx` around lines 137 - 146, The hero splash image currently uses <NetlifyImage> with alt="TanStack", which duplicates the adjacent <h1>; change the alt prop to an empty string (alt="") on the NetlifyImage instance to mark it decorative and avoid redundant screen-reader output, keeping all other props (src, width, height, className, loading, fetchPriority) unchanged.
🤖 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/routes/index.tsx`:
- Around line 61-66: The loader’s call to queryClient.prefetchQuery is
intentionally non-blocking but should be made explicit to avoid
no-floating-promises lint issues; update the loader (the arrow function that
destructures context: { queryClient } and calls queryClient.prefetchQuery) to
prefix the call with void (i.e., use void queryClient.prefetchQuery(...)) so the
intent of a fire-and-forget prefetch is clear while preserving TanStack
Router/Query’s non-awaited behavior.
- Around line 137-146: The hero splash image currently uses <NetlifyImage> with
alt="TanStack", which duplicates the adjacent <h1>; change the alt prop to an
empty string (alt="") on the NetlifyImage instance to mark it decorative and
avoid redundant screen-reader output, keeping all other props (src, width,
height, className, loading, fetchPriority) unchanged.
Member
|
This took way too many liberties and was clearly vibe coded. Definitely some good stuff in here, but I'd like to see more granular fixes I can merge 1 by 1. |
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
2 participants
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.
Summary
React.Suspense/LazyBrandContextMenuwrapper around hero splash logo — eliminates fallback→loaded layout shift on page loadloaderwithensureQueryDatato prefetch blog posts server-side — fixes hydration mismatch (<a>vs<div>) that also occurs in productionfont-display: swap→optionaland preload Inter font to prevent text reflow CLSwidth/heightto navbar logo images and marquee brand logosBrandContextMenucomponentTest plan
DATABASE_URL)Summary by CodeRabbit
Performance Improvements
UI Updates
Style