From e8fe66f91cdf91bccaa92ecaf2db91c97d0b588c Mon Sep 17 00:00:00 2001 From: Chad Curtis Date: Tue, 24 Feb 2026 20:24:43 -0600 Subject: [PATCH] Fix settings toggles resetting due to sync race conditions Three bugs caused settings toggles to get stuck or reset: 1. useEncryptedSettings: onSuccess was updating the parsedSettings cache under the old event ID, causing NostrSync to re-apply the stale remote event when the new one arrived with a different ID. Fixed by using the signed event's ID for both cache entries. 2. ContentTypeRow/SubKindRow: handleToggle was building the Nostr payload from the local React feedSettings state, which lags behind on rapid successive toggles. Fixed by merging against the Nostr-cached settings (nostrSettings?.feedSettings) as the base instead. 3. NostrSync: lastSyncedTimestamp ref reset to 0 on every page reload, so the 10-second recentlyWritten() guard was the only protection against stale relay events overwriting local changes. Fixed by seeding the ref with the current remote lastSync on first settings load, skipping application of any event that isn't strictly newer than what was already in the cache. --- src/components/ContentSettings.tsx | 16 +++++++++++---- src/components/NostrSync.tsx | 33 ++++++++++++++++++++++-------- src/hooks/useEncryptedSettings.ts | 10 ++++++--- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/components/ContentSettings.tsx b/src/components/ContentSettings.tsx index 626b3987..1474b662 100644 --- a/src/components/ContentSettings.tsx +++ b/src/components/ContentSettings.tsx @@ -236,13 +236,17 @@ function KindBadge({ kind }: { kind: number }) { function SubKindRow({ sub, parentEnabled }: { sub: SubKindDef; parentEnabled: boolean }) { const { feedSettings, updateFeedSettings } = useFeedSettings(); - const { updateSettings } = useEncryptedSettings(); + const { updateSettings, settings: nostrSettings } = useEncryptedSettings(); const { user } = useCurrentUser(); const handleToggle = async (key: string, value: boolean) => { updateFeedSettings({ [key]: value }); if (user) { - const updatedFeedSettings = { ...feedSettings, [key]: value }; + // Merge against the Nostr-cached feed settings (most up-to-date persisted + // state) rather than the local React state, which may lag behind when + // multiple toggles fire in quick succession. + const baseFeed = nostrSettings?.feedSettings ?? feedSettings; + const updatedFeedSettings = { ...baseFeed, [key]: value }; await updateSettings.mutateAsync({ feedSettings: updatedFeedSettings }); } }; @@ -280,7 +284,7 @@ function SubKindRow({ sub, parentEnabled }: { sub: SubKindDef; parentEnabled: bo function ContentTypeRow({ def }: { def: ExtraKindDef }) { const { feedSettings, updateFeedSettings } = useFeedSettings(); - const { updateSettings } = useEncryptedSettings(); + const { updateSettings, settings: nostrSettings } = useEncryptedSettings(); const { user } = useCurrentUser(); const icon = ICONS[def.route ?? String(def.kind)] ?? ; const hasSubKinds = !!def.subKinds; @@ -289,7 +293,11 @@ function ContentTypeRow({ def }: { def: ExtraKindDef }) { const handleToggle = async (key: string, value: boolean) => { updateFeedSettings({ [key]: value }); if (user) { - const updatedFeedSettings = { ...feedSettings, [key]: value }; + // Merge against the Nostr-cached feed settings (most up-to-date persisted + // state) rather than the local React state, which may lag behind when + // multiple toggles fire in quick succession. + const baseFeed = nostrSettings?.feedSettings ?? feedSettings; + const updatedFeedSettings = { ...baseFeed, [key]: value }; await updateSettings.mutateAsync({ feedSettings: updatedFeedSettings }); } }; diff --git a/src/components/NostrSync.tsx b/src/components/NostrSync.tsx index d94adcd3..fc497f0a 100644 --- a/src/components/NostrSync.tsx +++ b/src/components/NostrSync.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useNostr } from '@nostrify/react'; import { useCurrentUser } from '@/hooks/useCurrentUser'; import { useAppContext } from '@/hooks/useAppContext'; @@ -17,9 +17,13 @@ export function NostrSync() { const { user } = useCurrentUser(); const { config, updateConfig } = useAppContext(); const { settings: encryptedSettings, isLoading: settingsLoading, recentlyWritten } = useEncryptedSettings(); - - // Track the last synced settings timestamp to prevent re-syncing the same data + + // Track the last synced settings timestamp to prevent re-syncing the same data. + // Seeded to the remote lastSync on first load so that a stale relay event + // (older than what useInitialSync already applied) does not overwrite local + // settings after a page reload. const lastSyncedTimestamp = useRef(0); + const [seededTimestamp, setSeededTimestamp] = useState(false); useEffect(() => { if (!user) return; @@ -73,16 +77,27 @@ export function NostrSync() { useEffect(() => { if (!user || settingsLoading || !encryptedSettings) return; - // Don't overwrite local config if we just saved settings + // Get the remote sync timestamp + const remoteSync = encryptedSettings.lastSync || 0; + + // On first load, seed the ref with the current remote timestamp so that + // we don't re-apply settings that were already applied by useInitialSync. + // This prevents stale relay events from overwriting local changes after a + // page reload (where lastWriteTs module variable resets to 0). + if (!seededTimestamp) { + lastSyncedTimestamp.current = remoteSync; + setSeededTimestamp(true); + return; + } + + // Don't overwrite local config if we just saved settings (short-circuit for + // the immediate write window, e.g. before the new event propagates back). if (recentlyWritten()) { console.log('Skipping settings sync - recent write'); return; } - // Get the remote sync timestamp - const remoteSync = encryptedSettings.lastSync || 0; - - // Only sync if we haven't already synced this exact timestamp + // Skip if the remote snapshot is older than what we last applied. if (remoteSync <= lastSyncedTimestamp.current) { return; } @@ -127,7 +142,7 @@ export function NostrSync() { // Return the same reference if nothing changed to prevent re-render return changed ? updates : current; }); - }, [user, encryptedSettings, settingsLoading, updateConfig, recentlyWritten]); + }, [user, encryptedSettings, settingsLoading, updateConfig, recentlyWritten, seededTimestamp]); return null; } \ No newline at end of file diff --git a/src/hooks/useEncryptedSettings.ts b/src/hooks/useEncryptedSettings.ts index c42fa219..beb395cf 100644 --- a/src/hooks/useEncryptedSettings.ts +++ b/src/hooks/useEncryptedSettings.ts @@ -156,15 +156,19 @@ export function useEncryptedSettings() { console.error('Failed to publish encrypted settings:', error); }); - return updatedSettings; + return { updatedSettings, signedEvent }; }, // Update cache in-place instead of refetching, which avoids // NostrSync re-running and causing a re-render loop. // Do NOT invalidate the encryptedSettings query here — doing so triggers a // relay refetch that can return the old event before the new one propagates, // which causes NostrSync to overwrite the theme the user just selected. - onSuccess: (data) => { - queryClient.setQueryData(['parsedSettings', query.data?.id], data); + // + // Use the signed event's ID (not the old query event ID) so the parsed + // settings cache entry is keyed correctly and NostrSync picks it up. + onSuccess: ({ updatedSettings, signedEvent }) => { + queryClient.setQueryData(['encryptedSettings', user?.pubkey], signedEvent); + queryClient.setQueryData(['parsedSettings', signedEvent.id], updatedSettings); }, });