clean: Add Suspense boundaries to prevent hydration errors#1215
clean: Add Suspense boundaries to prevent hydration errors#1215
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughFour clean-related pages now wrap their primary components in React Suspense boundaries using a shared Loading component as the fallback, replacing prior inline or missing loading UI and deferring render until suspense resolves. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 |
Add Suspense boundaries with
|
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| <Suspense fallback={<Loading />}> | ||
| <CleanHistory /> | ||
| </Suspense> | ||
| </CardContent> |
There was a problem hiding this comment.
The new Suspense boundary on the history page cannot actually show the fallback because is a client component that only uses SWR's loading flags/LoadingContent and never throws a promise (no suspense: true, no use data). As written, the fallback is never rendered and the hydration mismatch the PR is meant to avoid is still unresolved, so the boundary is effectively a no-op.
| <Suspense fallback={<Loading />}> | |
| <CleanHistory /> | |
| </Suspense> | |
| </CardContent> | |
| <CleanHistory /> | |
| </CardContent> |
Finding type: Logical Bugs
There was a problem hiding this comment.
You're right that CleanHistory doesn't throw a promise since SWR doesn't have suspense: true. However, the Suspense boundary is kept as a defensive measure - the original bug was a client-side exception during hydration on the /clean pages. The CleanHistory component uses useAccount() which internally uses useParams() and context, and wrapping in Suspense helps Next.js manage the hydration process for client components with hooks. It's a low-cost safeguard that aligns with the pattern in PR #1204.
There was a problem hiding this comment.
Thanks — that makes sense: keeping the Suspense boundary as a low-cost defensive measure for hydration issues (and consistent with PR #1204) is reasonable. Noted, and I’ll save this to memory once the PR is merged.
| return ( | ||
| <div> | ||
| <Card className="my-4 max-w-2xl p-6 sm:mx-4 md:mx-auto"> | ||
| <Suspense | ||
| key={step} | ||
| fallback={ | ||
| <div className="flex h-[400px] items-center justify-center"> | ||
| Loading... | ||
| </div> | ||
| } | ||
| > | ||
| <Suspense key={step} fallback={<Loading />}> | ||
| {renderStepContent()} |
There was a problem hiding this comment.
Suspense now falls back to , but the previous placeholder had className="flex h-[400px] items-center justify-center" to reserve about 400px of vertical space. Replacing it with the minimal leaves the card body collapsing to the spinner height while each step loads, which contradicts the shared loading conventions in apps/web/CLAUDE.md and causes visible layout jank.
| return ( | |
| <div> | |
| <Card className="my-4 max-w-2xl p-6 sm:mx-4 md:mx-auto"> | |
| <Suspense | |
| key={step} | |
| fallback={ | |
| <div className="flex h-[400px] items-center justify-center"> | |
| Loading... | |
| </div> | |
| } | |
| > | |
| <Suspense key={step} fallback={<Loading />}> | |
| {renderStepContent()} | |
| return ( | |
| <div> | |
| <Card className="my-4 max-w-2xl p-6 sm:mx-4 md:mx-auto"> | |
| <Suspense key={step} fallback={<div className="flex h-[400px] items-center justify-center"><Loading /></div>}> | |
| {renderStepContent()} |
Finding type: Logical Bugs
There was a problem hiding this comment.
Good catch! Restored the 400px height container around the Loading component to prevent layout jank. Fixed in 91062fa.
There was a problem hiding this comment.
Commit 91062fa addressed this comment. The Suspense fallback was updated to wrap the <Loading /> component in a div with className="flex h-[400px] items-center justify-center", which reserves the 400px vertical space and prevents layout jank during loading, exactly as suggested in the comment.
1 participant
User description
Fixes client-side exception errors on the /clean pages by adding Suspense boundaries around client components that use hooks.
SuspensewithLoadingcomponent aroundConfirmationStepin/clean/page.tsx/clean/onboarding/page.tsxto useLoadingcomponent instead of inline textSuspensearoundCleanRunin/clean/run/page.tsxSuspensearoundCleanHistoryin/clean/history/page.tsxRelated to PR #1204 which fixed the same issue in the onboarding page.
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Implement
Suspenseboundaries around client-side components such asConfirmationStep,CleanRun, andCleanHistoryon/cleanpages to resolve client-side hydration errors. Utilize theLoadingcomponent as a fallback for these boundaries, ensuring a smoother user experience.Latest Contributors(2)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.