fix: review comments and coderabbit addressed

gusa4grr · gusa4grr · commit c51de184801f · 2026-03-11T00:20:20.000Z
diff --git a/app/app.vue b/app/app.vue
@@ -47,8 +47,8 @@ if (import.meta.server) {
   setJsonLd(createWebSiteSchema())
 }
 
-const keyboardShortcuts = useKeyboardShortcuts()
-const instantSearch = useInstantSearch()
+const keyboardShortcuts = useKeyboardShortcutsPreference()
+const instantSearch = useInstantSearchPreference()
 
 onKeyDown(
   '/',
diff --git a/app/components/AppHeader.vue b/app/components/AppHeader.vue
@@ -4,7 +4,7 @@ import type { NavigationConfig, NavigationConfigWithGroups } from '~/types'
 import { isEditableElement } from '~/utils/input'
 import { NPMX_DOCS_SITE } from '#shared/utils/constants'
 
-const keyboardShortcuts = useKeyboardShortcuts()
+const keyboardShortcuts = useKeyboardShortcutsPreference()
 const discord = useDiscordLink()
 
 withDefaults(
diff --git a/app/components/Button/Base.vue b/app/components/Button/Base.vue
@@ -26,7 +26,7 @@ const props = withDefaults(
 
 const el = useTemplateRef('el')
 
-const keyboardShortcutsEnabled = useKeyboardShortcuts()
+const keyboardShortcutsEnabled = useKeyboardShortcutsPreference()
 
 defineExpose({
   focus: () => el.value?.focus(),
diff --git a/app/components/Compare/PackageSelector.vue b/app/components/Compare/PackageSelector.vue
@@ -75,7 +75,7 @@ const resultIndexOffset = computed(() => (showNoDependencyOption.value ? 1 : 0))
 
 const numberFormatter = useNumberFormatter()
 
-const keyboardShortcuts = useKeyboardShortcuts()
+const keyboardShortcuts = useKeyboardShortcutsPreference()
 
 function addPackage(name: string) {
   if (packages.value.length >= maxPackages.value) return
diff --git a/app/components/DateTime.vue b/app/components/DateTime.vue
@@ -31,7 +31,7 @@ const props = withDefaults(
 
 const { locale } = useI18n()
 
-const relativeDates = useRelativeDates()
+const relativeDates = useRelativeDatesPreference()
 
 const dateFormatter = new Intl.DateTimeFormat(locale.value, {
   month: 'short',
diff --git a/app/components/Input/Base.vue b/app/components/Input/Base.vue
@@ -30,7 +30,7 @@ const emit = defineEmits<{
 
 const el = useTemplateRef('el')
 
-const keyboardShortcutsEnabled = useKeyboardShortcuts()
+const keyboardShortcutsEnabled = useKeyboardShortcutsPreference()
 
 defineExpose({
   focus: () => el.value?.focus(),
diff --git a/app/components/InstantSearch.vue b/app/components/InstantSearch.vue
@@ -1,5 +1,5 @@
 <script setup lang="ts">
-const instantSearch = useInstantSearch()
+const instantSearch = useInstantSearchPreference()
 
 onPrehydrate(el => {
   let userPreferences: Record<string, unknown> = {}
diff --git a/app/components/Link/Base.vue b/app/components/Link/Base.vue
@@ -66,7 +66,7 @@ const isLink = computed(() => props.variant === 'link')
 const isButton = computed(() => !isLink.value)
 const isButtonSmall = computed(() => props.size === 'small' && !isLink.value)
 const isButtonMedium = computed(() => props.size === 'medium' && !isLink.value)
-const keyboardShortcutsEnabled = useKeyboardShortcuts()
+const keyboardShortcutsEnabled = useKeyboardShortcutsPreference()
 </script>
 
 <template>
diff --git a/app/components/Package/WeeklyDownloadStats.vue b/app/components/Package/WeeklyDownloadStats.vue
@@ -210,7 +210,7 @@ const lastDatapoint = computed(() => dataset.value.at(-1)?.period ?? '')
 
 const isLoop = shallowRef(false)
 const showPulse = shallowRef(true)
-const keyboardShortcuts = useKeyboardShortcuts()
+const keyboardShortcuts = useKeyboardShortcutsPreference()
 
 const cheatCode = [
   'arrowup',
diff --git a/app/composables/useGlobalSearch.ts b/app/composables/useGlobalSearch.ts
@@ -5,7 +5,7 @@ import { debounce } from 'perfect-debounce'
 const pagesWithLocalFilter = new Set(['~username', 'org'])
 
 export function useGlobalSearch(place: 'header' | 'content' = 'content') {
-  const instantSearch = useInstantSearch()
+  const instantSearch = useInstantSearchPreference()
   const { searchProvider } = useSearchProvider()
   const searchProviderValue = computed(() => {
     const p = normalizeSearchParam(route.query.p)
diff --git a/app/composables/useLocalStorageHashProvider.ts b/app/composables/useLocalStorageHashProvider.ts
@@ -10,12 +10,12 @@ const defu = createDefu((object, key, value) => {
 
 export function useLocalStorageHashProvider<T extends object>(key: string, defaultValue: T) {
   const provider = createLocalStorageProvider<T>(key)
-  const data = ref<T>(defaultValue)
+  const data = ref<T>(structuredClone(defaultValue))
 
   onMounted(() => {
     const stored = provider.get()
     if (stored) {
-      data.value = defu(stored, defaultValue)
+      data.value = defu(stored, structuredClone(defaultValue))
     }
   })
 
@@ -24,7 +24,7 @@ export function useLocalStorageHashProvider<T extends object>(key: string, defau
   }
 
   function reset() {
-    data.value = { ...defaultValue }
+    data.value = structuredClone(defaultValue)
     provider.remove()
   }
 
diff --git a/app/composables/usePackageListPreferences.ts b/app/composables/usePackageListPreferences.ts
@@ -41,22 +41,6 @@ export function usePackageListPreferences() {
     },
   })
 
-  // One-time migration: replace legacy 'all' with the current maximum page size
-  watch(
-    isHydrated,
-    hydrated => {
-      if (!hydrated) {
-        return
-      }
-
-      if ((preferences.value.pageSize as unknown) === 'all') {
-        preferences.value.pageSize = Math.max(...PAGE_SIZE_OPTIONS) as PageSize
-        save()
-      }
-    },
-    { immediate: true },
-  )
-
   const pageSize = computed({
     get: () => preferences.value.pageSize,
     set: (value: PageSize) => {
diff --git a/app/composables/useUserPreferencesProvider.ts b/app/composables/useUserPreferencesProvider.ts
@@ -54,6 +54,20 @@ export function useUserPreferencesProvider(
   const isSynced = computed(() => status.value === 'synced')
   const hasError = computed(() => status.value === 'error')
 
+  async function syncWithServer(): Promise<void> {
+    const serverResult = await loadFromServer()
+
+    // If the server load failed, keep current local preferences untouched
+    if (hasError.value) return
+
+    const { merged, shouldPushToServer } = mergePreferences(preferences.value, serverResult)
+    if (shouldPushToServer) {
+      scheduleSync(preferences.value)
+    } else if (!arePreferencesEqual(preferences.value, merged)) {
+      preferences.value = merged
+    }
+  }
+
   async function initSync(): Promise<void> {
     if (syncInitialized || import.meta.server) return
     syncInitialized = true
@@ -62,13 +76,7 @@ export function useUserPreferencesProvider(
     setupBeforeUnload(() => preferences.value)
 
     if (isAuthenticated.value) {
-      const serverResult = await loadFromServer()
-      const { merged, shouldPushToServer } = mergePreferences(preferences.value, serverResult)
-      if (shouldPushToServer) {
-        scheduleSync(preferences.value)
-      } else if (!arePreferencesEqual(preferences.value, merged)) {
-        preferences.value = merged
-      }
+      await syncWithServer()
     }
 
     watch(
@@ -83,13 +91,7 @@ export function useUserPreferencesProvider(
 
     watch(isAuthenticated, async newIsAuth => {
       if (newIsAuth) {
-        const serverResult = await loadFromServer()
-        const { merged, shouldPushToServer } = mergePreferences(preferences.value, serverResult)
-        if (shouldPushToServer) {
-          scheduleSync(preferences.value)
-        } else if (!arePreferencesEqual(preferences.value, merged)) {
-          preferences.value = merged
-        }
+        await syncWithServer()
       }
     })
   }
@@ -105,3 +107,13 @@ export function useUserPreferencesProvider(
     initSync,
   }
 }
+
+/**
+ * Reset module-level singleton state. Test-only — do not use in production code.
+ */
+export function __resetPreferencesForTest(): void {
+  if (import.meta.test) {
+    dataRef = null
+    syncInitialized = false
+  }
+}
diff --git a/app/composables/useUserPreferencesSync.client.ts b/app/composables/useUserPreferencesSync.client.ts
@@ -24,6 +24,8 @@ let pendingSavePromise: Promise<boolean> | null = null
 let hasPendingChanges = false
 let debounceTimeoutId: ReturnType<typeof setTimeout> | null = null
 let syncedResetTimeoutId: ReturnType<typeof setTimeout> | null = null
+let beforeUnloadRegistered = false
+let routeGuardRegistered = false
 
 function getSyncState(): PreferencesSyncState {
   if (!syncStateInstance) {
@@ -142,7 +144,8 @@ export function useUserPreferencesSync() {
     }
 
     // Network error — fall back to defaults, don't flag as new user
-    state.status.value = 'idle'
+    state.status.value = 'error'
+    state.error.value = 'Failed to load preferences from server'
     return { preferences: { ...DEFAULT_USER_PREFERENCES }, isNewUser: false }
   }
 
@@ -159,6 +162,9 @@ export function useUserPreferencesSync() {
   }
 
   function setupRouteGuard(getPreferences: () => UserPreferences): void {
+    if (routeGuardRegistered) return
+    routeGuardRegistered = true
+
     router.beforeEach(async (_to, _from, next) => {
       if (hasPendingChanges && isAuthenticated.value) {
         void flushPendingSync(getPreferences())
@@ -168,7 +174,8 @@ export function useUserPreferencesSync() {
   }
 
   function setupBeforeUnload(getPreferences: () => UserPreferences): void {
-    if (import.meta.server) return
+    if (import.meta.server || beforeUnloadRegistered) return
+    beforeUnloadRegistered = true
 
     window.addEventListener('beforeunload', () => {
       if (hasPendingChanges && isAuthenticated.value) {
diff --git a/app/composables/userPreferences/useAccentColor.ts b/app/composables/userPreferences/useAccentColor.ts
@@ -17,6 +17,10 @@ export function useAccentColor() {
   })
 
   function setAccentColor(id: AccentColorId | null) {
+    if (import.meta.server) {
+      preferences.value.accentColorId = id
+      return
+    }
     if (id) {
       document.documentElement.style.setProperty('--accent-color', `var(--swatch-${id})`)
     } else {
diff --git a/app/composables/userPreferences/useBackgroundTheme.ts b/app/composables/userPreferences/useBackgroundTheme.ts
@@ -11,6 +11,10 @@ export function useBackgroundTheme() {
   const { preferences } = useUserPreferencesState()
 
   function setBackgroundTheme(id: BackgroundThemeId | null) {
+    if (import.meta.server) {
+      preferences.value.preferredBackgroundTheme = id
+      return
+    }
     if (id) {
       document.documentElement.dataset.bgTheme = id
     } else {
diff --git a/app/composables/userPreferences/useInstantSearch.ts b/app/composables/userPreferences/useInstantSearch.ts
@@ -1,10 +1,12 @@
-export const useInstantSearch = createSharedComposable(function useInstantSearch() {
-  const { preferences } = useUserPreferencesState()
+export const useInstantSearchPreference = createSharedComposable(
+  function useInstantSearchPreference() {
+    const { preferences } = useUserPreferencesState()
 
-  return computed({
-    get: () => preferences.value.instantSearch ?? true,
-    set: (value: boolean) => {
-      preferences.value.instantSearch = value
-    },
-  })
-})
+    return computed({
+      get: () => preferences.value.instantSearch ?? true,
+      set: (value: boolean) => {
+        preferences.value.instantSearch = value
+      },
+    })
+  },
+)
diff --git a/app/composables/userPreferences/useKeyboardShortcuts.ts b/app/composables/userPreferences/useKeyboardShortcuts.ts
@@ -1,20 +1,22 @@
-export const useKeyboardShortcuts = createSharedComposable(function useKeyboardShortcuts() {
-  const { preferences } = useUserPreferencesState()
-  const enabled = computed(() => preferences.value.keyboardShortcuts ?? true)
+export const useKeyboardShortcutsPreference = createSharedComposable(
+  function useKeyboardShortcutsPreference() {
+    const { preferences } = useUserPreferencesState()
+    const enabled = computed(() => preferences.value.keyboardShortcuts ?? true)
 
-  if (import.meta.client) {
-    watch(
-      enabled,
-      value => {
-        if (value) {
-          delete document.documentElement.dataset.kbdShortcuts
-        } else {
-          document.documentElement.dataset.kbdShortcuts = 'false'
-        }
-      },
-      { immediate: true },
-    )
-  }
+    if (import.meta.client) {
+      watch(
+        enabled,
+        value => {
+          if (value) {
+            delete document.documentElement.dataset.kbdShortcuts
+          } else {
+            document.documentElement.dataset.kbdShortcuts = 'false'
+          }
+        },
+        { immediate: true },
+      )
+    }
 
-  return enabled
-})
+    return enabled
+  },
+)
diff --git a/app/composables/userPreferences/useRelativeDates.ts b/app/composables/userPreferences/useRelativeDates.ts
@@ -1,4 +1,4 @@
-export function useRelativeDates() {
+export function useRelativeDatesPreference() {
   const { preferences } = useUserPreferencesState()
   return computed(() => preferences.value.relativeDates)
 }
diff --git a/app/pages/package/[[org]]/[name].vue b/app/pages/package/[[org]]/[name].vue
@@ -700,7 +700,7 @@ const codeLink = computed((): RouteLocationRaw | null => {
   }
 })
 
-const keyboardShortcuts = useKeyboardShortcuts()
+const keyboardShortcuts = useKeyboardShortcutsPreference()
 
 onKeyStroke(
   e => keyboardShortcuts.value && isKeyWithoutModifiers(e, '.') && !isEditableElement(e.target),
diff --git a/app/pages/search.vue b/app/pages/search.vue
@@ -467,7 +467,7 @@ function focusSearchInput() {
   searchInput?.focus()
 }
 
-const keyboardShortcuts = useKeyboardShortcuts()
+const keyboardShortcuts = useKeyboardShortcutsPreference()
 
 function handleResultsKeydown(e: KeyboardEvent) {
   if (!keyboardShortcuts.value) {
diff --git a/app/pages/settings.vue b/app/pages/settings.vue
@@ -6,7 +6,7 @@ const { isAuthenticated, isSyncing, isSynced, hasError } = useUserPreferencesSyn
 const { colorModePreference, setColorMode } = useColorModePreference()
 const { locale, locales, setLocale: setNuxti18nLocale } = useI18n()
 const { currentLocaleStatus, isSourceLocale } = useI18nStatus()
-const keyboardShortcutsEnabled = useKeyboardShortcuts()
+const keyboardShortcutsEnabled = useKeyboardShortcutsPreference()
 
 // Escape to go back (but not when focused on form elements or modal is open)
 onKeyStroke(
diff --git a/server/utils/preferences/user-preferences-store.ts b/server/utils/preferences/user-preferences-store.ts
@@ -12,26 +12,20 @@ export class UserPreferencesStore {
     return result ?? null
   }
 
-  async set(did: string, preferences: UserPreferences): Promise<void> {
+  async set(did: string, preferences: UserPreferences): Promise<UserPreferences> {
     const withTimestamp: UserPreferences = {
       ...preferences,
       updatedAt: new Date().toISOString(),
     }
     await this.storage.setItem(did, withTimestamp)
+    return withTimestamp
   }
 
   async merge(did: string, partial: Partial<UserPreferences>): Promise<UserPreferences> {
     const existing = await this.get(did)
     const base = existing ?? { ...DEFAULT_USER_PREFERENCES }
 
-    const merged: UserPreferences = {
-      ...base,
-      ...partial,
-      updatedAt: new Date().toISOString(),
-    }
-
-    await this.set(did, merged)
-    return merged
+    return this.set(did, { ...base, ...partial })
   }
 
   async delete(did: string): Promise<void> {
diff --git a/shared/schemas/userPreferences.ts b/shared/schemas/userPreferences.ts
diff --git a/test/nuxt/components/DateTime.spec.ts b/test/nuxt/components/DateTime.spec.ts
diff --git a/test/nuxt/components/compare/FacetRow.spec.ts b/test/nuxt/components/compare/FacetRow.spec.ts
diff --git a/test/nuxt/composables/use-install-command.spec.ts b/test/nuxt/composables/use-install-command.spec.ts
diff --git a/test/nuxt/composables/use-keyboard-shortcuts.spec.ts b/test/nuxt/composables/use-keyboard-shortcuts.spec.ts
diff --git a/test/nuxt/composables/use-package-list-preferences.spec.ts b/test/nuxt/composables/use-package-list-preferences.spec.ts

Original file line number	Diff line number	Diff line change
`@@ -47,8 +47,8 @@ if (import.meta.server) {`
`47`	`47`	`setJsonLd(createWebSiteSchema())`
`48`	`48`	`}`
`49`	`49`
`50`		`-const keyboardShortcuts = useKeyboardShortcuts()`
`51`		`-const instantSearch = useInstantSearch()`
	`50`	`+const keyboardShortcuts = useKeyboardShortcutsPreference()`
	`51`	`+const instantSearch = useInstantSearchPreference()`
`52`	`52`
`53`	`53`	`onKeyDown(`
`54`	`54`	`'/',`