Surface reading-plan pull failures too, fix StrictMode false sync
Build and Push Docker Image / build-and-push (push) Failing after 21s
Build and Push Docker Image / build-and-push (push) Failing after 21s
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
b14dbb52cc
commit
986d2e9331
2 files changed
+38
-11
No files matched your search
+5
-5
@@ -7,7 +7,7 @@ import { useProjectLibrary } from './hooks/useProjectLibrary.js';
|
|||||||
import { useReadingPlans } from './hooks/useReadingPlans.js';
|
import { useReadingPlans } from './hooks/useReadingPlans.js';
|
||||||
import { useSharedView } from './hooks/useSharedView.js';
|
import { useSharedView } from './hooks/useSharedView.js';
|
||||||
import HomePage from './pages/HomePage.jsx';
|
import HomePage from './pages/HomePage.jsx';
|
||||||
import { getRemoteReadingProgress, saveRemoteProject } from './syncService.js';
|
import { saveRemoteProject } from './syncService.js';
|
||||||
import {
|
import {
|
||||||
bookOptions, buildExportHtml, loadProjectIndex, makeId, parseBibleChapter, switchStorageUser,
|
bookOptions, buildExportHtml, loadProjectIndex, makeId, parseBibleChapter, switchStorageUser,
|
||||||
} from './utils/bibleUtils.jsx';
|
} from './utils/bibleUtils.jsx';
|
||||||
@@ -49,10 +49,10 @@ const App = () => {
|
|||||||
.then((r) => r.ok ? r.json() : null)
|
.then((r) => r.ok ? r.json() : null)
|
||||||
.then((serverInk) => { if (serverInk) reader.mergeServerInk(serverInk); })
|
.then((serverInk) => { if (serverInk) reader.mergeServerInk(serverInk); })
|
||||||
.catch(() => {});
|
.catch(() => {});
|
||||||
// Same for reading plan progress
|
// Same for reading plan progress — pullServerReadingProgress surfaces
|
||||||
getRemoteReadingProgress().then((result) => {
|
// a failed GET via the same error banner a failed push uses, instead
|
||||||
if (result.ok) readingPlans.mergeServerReadingProgress(result.data);
|
// of silently leaving stale/empty local state in place.
|
||||||
});
|
readingPlans.pullServerReadingProgress();
|
||||||
},
|
},
|
||||||
onSignOut: () => {
|
onSignOut: () => {
|
||||||
setProjectIndex(switchStorageUser(null));
|
setProjectIndex(switchStorageUser(null));
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
// `state` (see below) so sync success/failure can actually be tracked and
|
// `state` (see below) so sync success/failure can actually be tracked and
|
||||||
// shown, instead of being a fire-and-forget call buried inside setState.
|
// shown, instead of being a fire-and-forget call buried inside setState.
|
||||||
import { useCallback, useEffect, useRef, useState } from 'react';
|
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';
|
import { localDateString, registerCustomPlans } from '../utils/readingPlans.js';
|
||||||
|
|
||||||
const STORAGE_KEY = 'readingPlanProgress';
|
const STORAGE_KEY = 'readingPlanProgress';
|
||||||
@@ -36,7 +36,11 @@ export function useReadingPlans() {
|
|||||||
const [state, setState] = useState(loadLocal);
|
const [state, setState] = useState(loadLocal);
|
||||||
const [syncStatus, setSyncStatus] = useState(''); // '' | 'syncing' | 'synced' | 'error'
|
const [syncStatus, setSyncStatus] = useState(''); // '' | 'syncing' | 'synced' | 'error'
|
||||||
const [syncError, setSyncError] = useState('');
|
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
|
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
|
// Keeps the readingPlans.js lookup (getPlan/getPlanSchedule) in sync with
|
||||||
@@ -49,9 +53,8 @@ export function useReadingPlans() {
|
|||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
try { localStorage.setItem(STORAGE_KEY, JSON.stringify(state)); } catch {}
|
try { localStorage.setItem(STORAGE_KEY, JSON.stringify(state)); } catch {}
|
||||||
|
|
||||||
if (isFirstRender.current) {
|
if (state === initialStateRef.current) {
|
||||||
isFirstRender.current = false;
|
return; // nothing's actually changed since load — don't sync just because we mounted
|
||||||
return;
|
|
||||||
}
|
}
|
||||||
if (skipNextSyncRef.current) {
|
if (skipNextSyncRef.current) {
|
||||||
skipNextSyncRef.current = false;
|
skipNextSyncRef.current = false;
|
||||||
@@ -184,16 +187,39 @@ export function useReadingPlans() {
|
|||||||
|
|
||||||
// Server wins over whatever's local at sign-in.
|
// Server wins over whatever's local at sign-in.
|
||||||
const mergeServerReadingProgress = useCallback((serverData) => {
|
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 = {
|
const next = {
|
||||||
progress: Array.isArray(serverData.progress) ? serverData.progress : [],
|
progress: Array.isArray(serverData.progress) ? serverData.progress : [],
|
||||||
customPlans: Array.isArray(serverData.customPlans) ? serverData.customPlans : [],
|
customPlans: Array.isArray(serverData.customPlans) ? serverData.customPlans : [],
|
||||||
activity: Array.isArray(serverData.activity) ? serverData.activity : [],
|
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;
|
skipNextSyncRef.current = true;
|
||||||
setState(next);
|
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).
|
// Manual retry after a failed sync (surfaced next to the sync error).
|
||||||
const retryReadingPlanSync = useCallback(() => {
|
const retryReadingPlanSync = useCallback(() => {
|
||||||
setSyncStatus('syncing');
|
setSyncStatus('syncing');
|
||||||
@@ -224,5 +250,6 @@ export function useReadingPlans() {
|
|||||||
removeCustomPlan,
|
removeCustomPlan,
|
||||||
updateCustomPlan,
|
updateCustomPlan,
|
||||||
mergeServerReadingProgress,
|
mergeServerReadingProgress,
|
||||||
|
pullServerReadingProgress,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user