fix: avoid user package hydration mismatch#2981
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughChanges address search-provider hydration mismatches by deferring the settings read until mount, disabling server-side fetching for user packages, broadening the username page loading state, and adding hydration e2e coverage for pages with a non-default search provider. ChangesSearch provider hydration fix
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/composables/npm/useUserPackages.ts (1)
113-113: 🚀 Performance & Scalability | 🔵 TrivialCorrect fix, but note the SSR/SEO trade-off.
Disabling
serverfetch here fully resolves the hydration mismatch, but it also means the~[username]package list is no longer present in the server-rendered HTML — crawlers and no-JS clients will now see only a loading state until client-side fetch completes. This is consistent with the PR's stated approach, but as the linked issue notes, a longer-term fix of moving settings likesearchProviderto cookies would let this stay SSR-rendered while still avoiding the mismatch.🤖 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 `@app/composables/npm/useUserPackages.ts` at line 113, The hydration mismatch fix in useUserPackages is to disable the server-side fetch for the package search state by keeping the default emptySearchResponse fallback and setting the request to client-only. Update the useUserPackages composable so the search data for ~[username] is fetched only on the client, which prevents SSR from rendering a stale package list against client state. Keep this change localized to the package list fetch/config in useUserPackages, and note that it intentionally trades SSR content for hydration stability.app/composables/useSettings.ts (1)
199-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the manual mount flag with
useMounted()
useMounted()is a drop-in replacement for the currentshallowRef+onMountedpair and keeps the composable a bit cleaner.🤖 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 `@app/composables/useSettings.ts` around lines 199 - 212, `useSearchProvider` currently uses a manual `shallowRef` plus `onMounted` to track mount state. Replace that pattern with `useMounted()` in the same composable so `searchProvider` continues to read `DEFAULT_SETTINGS.searchProvider` before mount and `settings.value.searchProvider` after mount, while keeping the existing computed getter/setter behavior unchanged.
🤖 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.
Nitpick comments:
In `@app/composables/npm/useUserPackages.ts`:
- Line 113: The hydration mismatch fix in useUserPackages is to disable the
server-side fetch for the package search state by keeping the default
emptySearchResponse fallback and setting the request to client-only. Update the
useUserPackages composable so the search data for ~[username] is fetched only on
the client, which prevents SSR from rendering a stale package list against
client state. Keep this change localized to the package list fetch/config in
useUserPackages, and note that it intentionally trades SSR content for hydration
stability.
In `@app/composables/useSettings.ts`:
- Around line 199-212: `useSearchProvider` currently uses a manual `shallowRef`
plus `onMounted` to track mount state. Replace that pattern with `useMounted()`
in the same composable so `searchProvider` continues to read
`DEFAULT_SETTINGS.searchProvider` before mount and
`settings.value.searchProvider` after mount, while keeping the existing computed
getter/setter behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a9124468-b91c-45e1-976b-5b4d2758f874
📒 Files selected for processing (4)
app/composables/npm/useUserPackages.tsapp/composables/useSettings.tsapp/pages/~[username]/index.vuetest/e2e/hydration.spec.ts
gameroman
left a comment
There was a problem hiding this comment.
Could you address the Coderabbit's Nitpick comments please
|
I am dealing with it actively now :) |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/composables/useSettings.ts (1)
198-207: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGood SSR-safe hydration fix; consider a brief comment.
Swapping the manual
onMounted/shallowRefpattern for VueUse'suseMounted()is correct and SSR-safe (it internally guards withgetCurrentInstance()before registering the lifecycle hook), so the initial client render matches SSR output and avoids the hydration mismatch. Since the rationale isn't obvious from the code alone, a short comment would help future readers understand why mount-gating is needed here.📝 Suggested comment
const { settings } = useSettings() + // Fall back to the default value until mounted to avoid SSR/client hydration + // mismatches, since `settings` is sourced from localStorage (client-only). const isMounted = useMounted()As per coding guidelines, "Add comments only to explain complex logic or non-obvious implementations."
🤖 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 `@app/composables/useSettings.ts` around lines 198 - 207, The SSR-safe mount gating in useSearchProvider is correct, but the intent is not obvious from the computed getter that switches between settings.value.searchProvider and DEFAULT_SETTINGS.searchProvider based on useMounted(). Add a short inline comment near the mount check in useSearchProvider explaining that the mounted guard keeps the initial SSR/client render aligned and avoids hydration mismatch, so future readers understand why the value is deferred until mount.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@app/composables/useSettings.ts`:
- Around line 198-207: The SSR-safe mount gating in useSearchProvider is
correct, but the intent is not obvious from the computed getter that switches
between settings.value.searchProvider and DEFAULT_SETTINGS.searchProvider based
on useMounted(). Add a short inline comment near the mount check in
useSearchProvider explaining that the mounted guard keeps the initial SSR/client
render aligned and avoids hydration mismatch, so future readers understand why
the value is deferred until mount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 86bf9d05-a996-4cd2-b8a7-8ee7923a9ad8
📒 Files selected for processing (1)
app/composables/useSettings.ts
gameroman
left a comment
There was a problem hiding this comment.
Looks good 👍
Lets see what other maintainers think
Specifically the Coderabbit's comment about SEO
Close #1948, Close #2753
The
/~usernamepage rendered package search results on the server with the default search provider, while the client could hydrate with a different provider from localStorage. That could produce hydration mismatches and a visible provider swap for users who selected npm.According to #1948 (comment), I end up choosing to disable SSR for this page. Package list fetching and other related works are all done in client now.
🤖 Generated with Codex