Hotfix: SSR HTML rendering issue on dev mode - #240
Conversation
|
@MarcosProWork is attempting to deploy a commit to the Volvox Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesTheme toggle rendering
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
| Filename | Overview |
|---|---|
| src/components/navigation.tsx | Adopts an SSR-stable dual-icon theme toggle, but the changed rendering behavior lacks regression coverage. |
| src/components/blog/blog-post-header.tsx | Adopts the same dual-icon strategy and stabilizes the accessible label without updating tests. |
Prompt To Fix All With AI
### Issue 1
src/components/navigation.tsx:266-267
**Theme behavior lacks regression coverage**
The new dual-icon SSR strategy and the related label change in `blog-post-header.tsx` have no corresponding tests, so regressions in server markup, theme-based icon visibility, or the accessible label will pass the test suite undetected.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "Hotfix: SSR HTML rendering issue on dev ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <Moon weight="fill" className="h-5 w-5 block dark:hidden" /> | ||
| <Sun weight="fill" className="h-5 w-5 hidden dark:block" /> |
There was a problem hiding this comment.
The new dual-icon SSR strategy and the related label change in blog-post-header.tsx have no corresponding tests, so regressions in server markup, theme-based icon visibility, or the accessible label will pass the test suite undetected.
Rule Used: What: Require adding/updating/deleting tests to ma... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/navigation.tsx
Line: 266-267
Comment:
**Theme behavior lacks regression coverage**
The new dual-icon SSR strategy and the related label change in `blog-post-header.tsx` have no corresponding tests, so regressions in server markup, theme-based icon visibility, or the accessible label will pass the test suite undetected.
**Rule Used:** What: Require adding/updating/deleting tests to ma... ([source](https://app.greptile.com/volvox/-/custom-context?memory=d6acce3d-3556-4bfc-b39f-f945c8c677e6))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![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
🤖 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 `@src/components/blog/blog-post-header.tsx`:
- Around line 61-66: Add data-testid="theme-toggle" to the Button wrapping the
theme-toggle icons in the blog post header, while preserving its existing
aria-label and icon rendering.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 48bbd43e-a111-43e1-a4cd-e4ca8f9ae07c
📒 Files selected for processing (2)
src/components/blog/blog-post-header.tsxsrc/components/navigation.tsx
| aria-label="Toggle theme" | ||
| > | ||
| {resolvedTheme === "light" ? ( | ||
| <Moon weight="fill" className="h-5 w-5" /> | ||
| ) : ( | ||
| <Sun weight="fill" className="h-5 w-5" /> | ||
| )} | ||
| {/* Both icons render on the server; the `.dark` class decides | ||
| which is visible, so SSR and hydration always match. */} | ||
| <Moon weight="fill" className="h-5 w-5 block dark:hidden" /> | ||
| <Sun weight="fill" className="h-5 w-5 hidden dark:block" /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required test ID to this theme toggle.
This Button is a key theme-toggle element, but it does not expose data-testid="theme-toggle". Add the test ID so SSR and hydration regression tests can target this instance consistently.
As per coding guidelines, src/**/*.tsx requires data-testid attributes for key elements, including theme-toggle.
Proposed fix
aria-label="Toggle theme"
+ data-testid="theme-toggle"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| aria-label="Toggle theme" | |
| > | |
| {resolvedTheme === "light" ? ( | |
| <Moon weight="fill" className="h-5 w-5" /> | |
| ) : ( | |
| <Sun weight="fill" className="h-5 w-5" /> | |
| )} | |
| {/* Both icons render on the server; the `.dark` class decides | |
| which is visible, so SSR and hydration always match. */} | |
| <Moon weight="fill" className="h-5 w-5 block dark:hidden" /> | |
| <Sun weight="fill" className="h-5 w-5 hidden dark:block" /> | |
| aria-label="Toggle theme" | |
| data-testid="theme-toggle" | |
| > | |
| {/* Both icons render on the server; the `.dark` class decides | |
| which is visible, so SSR and hydration always match. */} | |
| <Moon weight="fill" className="h-5 w-5 block dark:hidden" /> | |
| <Sun weight="fill" className="h-5 w-5 hidden dark:block" /> |
🤖 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/blog/blog-post-header.tsx` around lines 61 - 66, Add
data-testid="theme-toggle" to the Button wrapping the theme-toggle icons in the
blog post header, while preserving its existing aria-label and icon rendering.
Source: Coding guidelines
1 participant
Summary
Fixes a React hydration mismatch thrown by the theme-toggle button in the site navigation.
useTheme()fromnext-themesreturnsresolvedTheme === undefinedduring SSR - the active theme lives inlocalStorageand isn't knowable until the client script runs. Both toggle buttons branched on it at render time:So the server always took the
Sunbranch, while the client's first render resolved the real theme and producedMoon. Same element, different<path d=...>→ hydration mismatch.Fix
Render both icons unconditionally and let the
.darkclass on<html>decide which is visible.next-themessets that class via a pre-paint inline script, so the correct icon is showing before first paint — no flash, and the server and client HTML are byte-identical.This is the same SSR-safe pattern already used by
ThemeToggleinsrc/components/theme-toggle.tsxand by the light/dark logo swap insrc/components/products/product-hero.tsx— this PR brings the two remaining holdouts in line with it.toggleThemestill readsresolvedTheme, which is correct: it only runs on click, long after mount.Changes
src/components/navigation.tsx— the component in the reported stack trace. CSS-based icon swap.src/components/blog/blog-post-header.tsx— same latent bug, plus itsaria-labelbranched onresolvedThemetoo (an attribute-level mismatch on top of the element one); now a static"Toggle theme". Note this component currently has no importers, so the fix is pre-emptive.Notes
src/components/ui/sonner.tsxalso readsresolvedTheme, but Sonner'sToasterrenders nothing server-side, so it is not a mismatch source. Left alone.Verification
pnpm typecheck— cleanpnpm lint— no new findings in the touched files (pre-existing warnings elsewhere are unrelated)block dark:hidden/hidden dark:block, confirming the server output no longer depends onresolvedTheme