fix: address settings-related hydration issues using prehydrate#1952
fix: address settings-related hydration issues using prehydrate#1952Adebesin-Cell wants to merge 3 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughRefactors the Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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
🧹 Nitpick comments (1)
app/composables/useSettings.ts (1)
77-88: Add explicit normalisation after merging persisted settings.The merge preserves stale/legacy values as-is. Please normalise constrained fields (for example
searchProvider,sidebar.collapsed, and numeric config bounds) before returning, so invalid old payloads cannot leak into runtime state.♻️ Suggested hardening
+function normaliseSettings(input: AppSettings): AppSettings { + return { + ...input, + searchProvider: input.searchProvider === 'npm' ? 'npm' : 'algolia', + sidebar: { + ...input.sidebar, + collapsed: Array.isArray(input.sidebar?.collapsed) + ? input.sidebar.collapsed.filter((v): v is string => typeof v === 'string') + : [], + }, + } +} + function readFromLocalStorage(): AppSettings { try { const raw = localStorage.getItem(STORAGE_KEY) if (raw) { const stored = JSON.parse(raw) - return { + return normaliseSettings({ ...DEFAULT_SETTINGS, ...stored, connector: { ...DEFAULT_SETTINGS.connector, ...stored.connector }, sidebar: { ...DEFAULT_SETTINGS.sidebar, ...stored.sidebar }, chartFilter: { ...DEFAULT_SETTINGS.chartFilter, ...stored.chartFilter }, - } + }) } } catch {} return { ...DEFAULT_SETTINGS } }Based on learnings: "defu only fills in null/undefined keys, so a stale persisted value survives the merge unchanged. Normalisation/migration of stale stored values must be done explicitly after hydration, not via defu defaults."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb8c5219-621a-43fb-9a9f-661ca5a5dd57
📒 Files selected for processing (2)
app/composables/useSettings.tsapp/utils/prehydrate.ts
| function readFromLocalStorage(): AppSettings { | ||
| try { | ||
| const raw = localStorage.getItem(STORAGE_KEY) | ||
| if (raw) { | ||
| const stored = JSON.parse(raw) | ||
| return normaliseSettings({ | ||
| ...DEFAULT_SETTINGS, | ||
| ...stored, | ||
| connector: { ...DEFAULT_SETTINGS.connector, ...stored.connector }, | ||
| sidebar: { ...DEFAULT_SETTINGS.sidebar, ...stored.sidebar }, | ||
| chartFilter: { ...DEFAULT_SETTINGS.chartFilter, ...stored.chartFilter }, | ||
| }) | ||
| } | ||
| } catch {} | ||
| return { ...DEFAULT_SETTINGS } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== DEFAULT_SETTINGS definition ==="
rg -n "DEFAULT_SETTINGS\s*=" app/composables/useSettings.ts -A 15
echo -e "\n=== normaliseSettings function ==="
rg -n "normaliseSettings" app/composables/useSettings.ts -B 2 -A 10
echo -e "\n=== Full readFromLocalStorage function ==="
sed -n '90,106p' app/composables/useSettings.ts
echo -e "\n=== useState initialisation ==="
sed -n '117,125p' app/composables/useSettings.ts
echo -e "\n=== CollapsibleSection.vue mutation ==="
rg -n "sidebar\.collapsed\s*=" app/components/CollapsibleSection.vue -B 3 -A 3Repository: npmx-dev/npmx.dev
Length of output: 96
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Locating useSettings.ts ==="
fd "useSettings" --type f
echo -e "\n=== Locating CollapsibleSection.vue ==="
fd "CollapsibleSection" --type f
echo -e "\n=== Repository structure ==="
ls -la | head -20Repository: npmx-dev/npmx.dev
Length of output: 1437
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full useSettings.ts content (checking size first) ==="
wc -l app/composables/useSettings.ts
echo -e "\n=== useSettings.ts - relevant sections ==="
cat -n app/composables/useSettings.tsRepository: npmx-dev/npmx.dev
Length of output: 9614
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CollapsibleSection.vue full content ==="
wc -l app/components/CollapsibleSection.vue
echo -e "\n=== CollapsibleSection.vue ==="
cat -n app/components/CollapsibleSection.vue
echo -e "\n=== Search for sidebar.collapsed mutations across codebase ==="
rg "sidebar\.collapsed" app --type=ts --type=vue -nRepository: npmx-dev/npmx.dev
Length of output: 4904
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for sidebar.collapsed mutations across codebase ==="
rg "sidebar\.collapsed" app -n
echo -e "\n=== Check if normaliseSettings creates new objects ==="
sed -n '77,88p' app/composables/useSettings.tsRepository: npmx-dev/npmx.dev
Length of output: 1095
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify mutations happen after hydration, not during ==="
rg "watch\|computed\|watcher" app/composables/useSettings.ts -n -A 2
echo -e "\n=== Check if any code path prevents the settings.value replacement ==="
sed -n '107,155p' app/composables/useSettings.ts
echo -e "\n=== Verify normaliseSettings creates independent objects ==="
python3 << 'EOF'
# Verify the flow logic
print("Flow analysis:")
print("1. readFromLocalStorage() with localStorage data:")
print(" - Creates NEW nested objects via spread")
print(" - Passes to normaliseSettings()")
print(" - normaliseSettings() creates NEW sidebar object")
print(" - Result: NO shared refs to DEFAULT_SETTINGS")
print()
print("2. readFromLocalStorage() without localStorage:")
print(" - Returns { ...DEFAULT_SETTINGS }")
print(" - Result: SHARED refs to DEFAULT_SETTINGS")
print()
print("3. useState init:")
print(" - Creates { ...DEFAULT_SETTINGS }")
print(" - Result: SHARED refs to DEFAULT_SETTINGS")
print()
print("4. After hydration:")
print(" - settings.value = stored (lines 134/137)")
print(" - Replaces the shallow copy completely")
print()
print("Risk window: During hydration, before lines 134/137")
print("But mutation handlers (like toggle) only run after hydration")
EOFRepository: npmx-dev/npmx.dev
Length of output: 120
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full sync section in useSettings.ts ==="
sed -n '118,150p' app/composables/useSettings.ts
echo -e "\n=== Check watchers/event handlers in CollapsibleSection.vue ==="
sed -n '40,62p' app/components/CollapsibleSection.vue
echo -e "\n=== Trace: Does shallow copy ever get mutated directly? ==="
echo "Looking for in-place array mutations on settings.value.sidebar.collapsed..."
rg "sidebar\.collapsed\.(push|pop|splice|shift)" appRepository: npmx-dev/npmx.dev
Length of output: 1984
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Confirm: Does app:mounted hook fire before or after interactive code? ==="
echo "Checking Nuxt documentation expectations and SSR/hydration lifecycle..."
echo ""
echo "Vue/Nuxt SSR hydration lifecycle:"
echo "1. Server renders initial HTML with default values"
echo "2. Client loads, hydrates with same defaults"
echo "3. app:mounted hook fires"
echo "4. After mount, event listeners become active"
echo ""
echo "So toggle() event handler CANNOT fire before app:mounted hook"
echo "Therefore, settings.value = stored (line 134) executes BEFORE any toggle() calls"
echo ""
echo "=== Verify normaliseSettings in successful localStorage path ==="
sed -n '90,105p' app/composables/useSettings.ts | head -20Repository: npmx-dev/npmx.dev
Length of output: 1149
Use deep copies or factory functions for default settings initialisation.
Lines 104 and 119 create shallow copies of DEFAULT_SETTINGS, leaving nested objects (connector, sidebar, chartFilter) with shared references. Whilst the current hydration strategy replaces these copies via app:mounted before mutation handlers execute, this design is unnecessarily fragile and depends on strict sequencing of the lifecycle.
The fallback path in readFromLocalStorage() (when localStorage is empty or errors) is particularly vulnerable. A dedicated factory function would ensure all code paths consistently create independent default instances, improving defensive robustness.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
1 participant
This is an attempt to fix a sudden flash and weird page display behavior as seen here
Screen.Recording.2026-03-05.at.22.32.44.mov
The fix swaps useLocalStorage for useState (SSR-safe), paired with a localStorage sync after mount. During hydration, server and client agree on the same default values. After mounting, user preferences are read from localStorage and applied.
The existing onPrehydrate script handles DOM-level patches (accent color, bg theme, collapsed sections, etc.) to prevent visual flash before hydration, and now also sets a data-search-provider attribute for the search provider.
Closes #1948
Test plan
AFTER FIX:
Screen.Recording.2026-03-05.at.22.32.58.mov