From 986d2e933194db67afc8b496f8476f363c7f25b9 Mon Sep 17 00:00:00 2001 From: nmemmert <67961292+nmemmert@users.noreply.github.com> Date: Wed, 23 Sep 2026 14:47:51 -0400 Subject: [PATCH] Surface reading-plan pull failures too, fix StrictMode false sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sign-in pull (GET /api/reading-plans -> merge) had the same silent- failure gap the push side had before the last fix: a failed fetch just left local state as-is with zero indication anything was wrong. Moved it into the hook as pullServerReadingProgress(), which reports failures through the same error banner/retry the push path already uses. Also found and fixed a real bug while testing this: the "skip syncing on the very first render" guard used a boolean ref, which React 18 StrictMode defeats by double-invoking effects in dev — the second invocation saw the flag already flipped and fired a spurious sync of stale localStorage data before the app had even confirmed sign-in, occasionally producing a false "couldn't sync" banner on fresh loads. Replaced it with a reference-equality check against the state object captured at mount, which both of StrictMode's synthetic invocations see identically. Caught this in useReadingPlans.js's own test run, not manual testing. Co-Authored-By: Claude Sonnet 5 --- src/App.jsx | 10 ++++----- src/hooks/useReadingPlans.js | 39 ++++++++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/App.jsx b/src/App.jsx index ef959de..d7b2695 100644 --- a/src/App.jsx +++ b/src/App.jsx @@ -7,7 +7,7 @@ import { useProjectLibrary } from './hooks/useProjectLibrary.js'; import { useReadingPlans } from './hooks/useReadingPlans.js'; import { useSharedView } from './hooks/useSharedView.js'; import HomePage from './pages/HomePage.jsx'; -import { getRemoteReadingProgress, saveRemoteProject } from './syncService.js'; +import { saveRemoteProject } from './syncService.js'; import { bookOptions, buildExportHtml, loadProjectIndex, makeId, parseBibleChapter, switchStorageUser, } from './utils/bibleUtils.jsx'; @@ -49,10 +49,10 @@ const App = () => { .then((r) => r.ok ? r.json() : null) .then((serverInk) => { if (serverInk) reader.mergeServerInk(serverInk); }) .catch(() => {}); - // Same for reading plan progress - getRemoteReadingProgress().then((result) => { - if (result.ok) readingPlans.mergeServerReadingProgress(result.data); - }); + // Same for reading plan progress — pullServerReadingProgress surfaces + // a failed GET via the same error banner a failed push uses, instead + // of silently leaving stale/empty local state in place. + readingPlans.pullServerReadingProgress(); }, onSignOut: () => { setProjectIndex(switchStorageUser(null)); diff --git a/src/hooks/useReadingPlans.js b/src/hooks/useReadingPlans.js index 02c8b48..4080298 100644 --- a/src/hooks/useReadingPlans.js +++ b/src/hooks/useReadingPlans.js @@ -5,7 +5,7 @@ // `state` (see below) so sync success/failure can actually be tracked and // shown, instead of being a fire-and-forget call buried inside setState. import { useCallback, useEffect, useRef, useState } from 'react'; -import { saveRemoteReadingProgress } from '../syncService.js'; +import { getRemoteReadingProgress, saveRemoteReadingProgress } from '../syncService.js'; import { localDateString, registerCustomPlans } from '../utils/readingPlans.js'; const STORAGE_KEY = 'readingPlanProgress'; @@ -36,7 +36,11 @@ export function useReadingPlans() { const [state, setState] = useState(loadLocal); const [syncStatus, setSyncStatus] = useState(''); // '' | 'syncing' | 'synced' | 'error' const [syncError, setSyncError] = useState(''); - const isFirstRender = useRef(true); + // The exact object loadLocal() produced at mount. Compared by reference (not + // a boolean "first render" flag) so React 18 StrictMode's dev-only double + // effect invocation can't defeat the skip-on-load guard below — both + // invocations see the same unchanged `state` reference either way. + const initialStateRef = useRef(state); const skipNextSyncRef = useRef(false); // set right before a server merge, so we don't PUT straight back what we just GOT // Keeps the readingPlans.js lookup (getPlan/getPlanSchedule) in sync with @@ -49,9 +53,8 @@ export function useReadingPlans() { useEffect(() => { try { localStorage.setItem(STORAGE_KEY, JSON.stringify(state)); } catch {} - if (isFirstRender.current) { - isFirstRender.current = false; - return; + if (state === initialStateRef.current) { + return; // nothing's actually changed since load — don't sync just because we mounted } if (skipNextSyncRef.current) { skipNextSyncRef.current = false; @@ -184,16 +187,39 @@ export function useReadingPlans() { // Server wins over whatever's local at sign-in. const mergeServerReadingProgress = useCallback((serverData) => { - if (!serverData || typeof serverData !== 'object' || Array.isArray(serverData)) return; + if (!serverData || typeof serverData !== 'object' || Array.isArray(serverData)) { + console.warn('Reading plan pull: server returned unexpected data', serverData); + return; + } const next = { progress: Array.isArray(serverData.progress) ? serverData.progress : [], customPlans: Array.isArray(serverData.customPlans) ? serverData.customPlans : [], activity: Array.isArray(serverData.activity) ? serverData.activity : [], }; + // Temporary diagnostic — remove once cross-device sync is confirmed fixed. + console.log( + `Reading plans pulled from server: ${next.progress.length} in progress, ${next.customPlans.length} custom plan(s).`, + next, + ); skipNextSyncRef.current = true; setState(next); }, []); + // Fetches the server's copy at sign-in. Unlike the old inline version of + // this call, a failed GET is now visible (same error banner as a failed + // push) instead of silently leaving you on stale/empty local state — which + // looks identical to "you have no reading plans" with zero indication why. + const pullServerReadingProgress = useCallback(async () => { + const result = await getRemoteReadingProgress(); + if (result.ok) { + mergeServerReadingProgress(result.data); + } else { + setSyncStatus('error'); + setSyncError(formatSyncError(result)); + console.error('Reading plan pull failed:', result.status, result.error); + } + }, [mergeServerReadingProgress]); + // Manual retry after a failed sync (surfaced next to the sync error). const retryReadingPlanSync = useCallback(() => { setSyncStatus('syncing'); @@ -224,5 +250,6 @@ export function useReadingPlans() { removeCustomPlan, updateCustomPlan, mergeServerReadingProgress, + pullServerReadingProgress, }; }