From 482f18cb23a7236399bcfd6cbbae1fcfc3165c5c Mon Sep 17 00:00:00 2001 From: Stefan du Fresne Date: Tue, 20 Jan 2026 21:31:23 +0000 Subject: [PATCH] Refactor SyncManager to remove stale queue and ready signal useEffects - Create socketRegistry to make socket accessible outside React components - Add syncMiddleware that emits docUpdate when markStale action is dispatched - Move socket ready signal directly into handleFullSync completion using socketRef - Remove two reactive useEffects that were causing cascading state updates This makes the sync flow more explicit: docs are emitted immediately when marked stale (if connected), and the ready signal is emitted as part of sync completion rather than as a reactive cascade. Co-Authored-By: Claude Opus 4.5 --- src/client/features/Sync/SyncManager.tsx | 48 ++++++++-------------- src/client/features/Sync/socketRegistry.ts | 16 ++++++++ src/client/features/Sync/syncSlice.ts | 28 +++++++++++++ src/client/store.ts | 5 ++- 4 files changed, 64 insertions(+), 33 deletions(-) create mode 100644 src/client/features/Sync/socketRegistry.ts diff --git a/src/client/features/Sync/SyncManager.tsx b/src/client/features/Sync/SyncManager.tsx index 11b33d3..86eb7ac 100644 --- a/src/client/features/Sync/SyncManager.tsx +++ b/src/client/features/Sync/SyncManager.tsx @@ -1,6 +1,6 @@ // TODO: drop axios and just use fetch import axios from 'axios'; -import { useEffect, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { io, type Socket } from 'socket.io-client'; import type { Requests } from '../../../server/sync/types'; @@ -16,8 +16,8 @@ import { update } from '../../state/docsSlice'; import { useDispatch, useSelector } from '../../store'; import { checkForUpdate } from '../Update/updateSlice'; import { selectIsGuest, setUserAsUnauthenticated } from '../User/userSlice'; +import { registerSocket } from './socketRegistry'; import { - cleanStale, completeSync, requestSync, State, @@ -85,13 +85,16 @@ function SyncManager() { const isGuest = useSelector(selectIsGuest); const handle = db(user); - const stale = useSelector((state) => state.sync.stale); const state = useSelector((state) => state.sync.state); const [socket, setSocket] = useState( undefined as unknown as Socket, ); + // Ref to access current socket from sync handlers without adding socket to dependencies + const socketRef = useRef(socket); + socketRef.current = socket; + useEffect(() => { // PERF: make this work more in parallel, benchmark it to see if it makes a difference etc const handleFullSync = async () => { @@ -187,6 +190,15 @@ function SyncManager() { } dispatch(completeSync()); + + // Signal ready and transition to connected state directly after sync completes + // Using socketRef to access current socket without adding it to useEffect dependencies + const currentSocket = socketRef.current; + if (currentSocket?.connected) { + debug('sync complete, emitting ready signal'); + currentSocket.emit('ready'); + dispatch(socketConnected()); + } } catch (e) { if (axios.isAxiosError(e) && e.response?.status === 401) { debug('sync failed as user is no longer authenticated'); @@ -208,34 +220,6 @@ function SyncManager() { } }, [handle, dispatch, state]); - useEffect(() => { - const processStaleQueue = () => { - const docs = Object.values(stale); - // Don't emit if we're disconnected. Otherwise we'll queue up a bunch of emits that could contradict each other. - // Once we reconnect a full sync will occur anyway - // TODO: and when we are State.connected, i.e. full "ready" - if (docs.length && socket && socket.connected && state === State.connected) { - debug(`stale server update for ${docs.length} docs`); - - socket.emit('docUpdate', docs); - } - - // Always clean the docs we're aware of out though. Again, they aren't needed in the stale queue if we're offline - dispatch(cleanStale(docs)); - }; - - processStaleQueue(); - }, [dispatch, socket, stale, state]); - - useEffect(() => { - if (socket?.connected && state === State.completed) { - socket.emit('ready'); - dispatch(socketConnected()); - } else { - // TODO: emit a not ready or disconnect to force socket communication only while not syncing - } - }, [dispatch, socket, state]); - // we are using the trigger of this to // close the socket, so we can't also pass in the socket because when it's created it would be // deleted again straight away @@ -244,6 +228,7 @@ function SyncManager() { const initSocket = () => { if (socket) { socket.close(); + registerSocket(null); } if (isGuest) { @@ -281,6 +266,7 @@ function SyncManager() { }); setSocket(localSocket); + registerSocket(localSocket); }; initSocket(); diff --git a/src/client/features/Sync/socketRegistry.ts b/src/client/features/Sync/socketRegistry.ts new file mode 100644 index 0000000..775c45b --- /dev/null +++ b/src/client/features/Sync/socketRegistry.ts @@ -0,0 +1,16 @@ +import type { Socket } from 'socket.io-client'; +import type { ClientToServerEvents, ServerToClientEvents } from '../../../shared/types'; + +type SyncSocket = Socket; + +// Simple registry to make the socket accessible outside of React components +// This allows middleware to emit socket events without prop drilling +let registeredSocket: SyncSocket | null = null; + +export function registerSocket(socket: SyncSocket | null): void { + registeredSocket = socket; +} + +export function getSocket(): SyncSocket | null { + return registeredSocket; +} diff --git a/src/client/features/Sync/syncSlice.ts b/src/client/features/Sync/syncSlice.ts index 113b7b5..f68ef4a 100644 --- a/src/client/features/Sync/syncSlice.ts +++ b/src/client/features/Sync/syncSlice.ts @@ -1,5 +1,10 @@ +import type { Middleware } from '@reduxjs/toolkit'; import { createSlice } from '@reduxjs/toolkit'; import type { Doc, DocId } from '../../../shared/types'; +import { debugClient } from '../../globals'; +import { getSocket } from './socketRegistry'; + +const debug = debugClient('sync'); type DocMap = Record; type SerializableError = { name: string; message: string }; @@ -92,3 +97,26 @@ export const { socketDisconnected, } = syncSlice.actions; export default syncSlice.reducer; + +// Middleware to emit socket events when docs are marked stale +export const syncMiddleware: Middleware = (storeApi) => (next) => (action) => { + const result = next(action); + + if (syncSlice.actions.markStale.match(action)) { + const state = storeApi.getState() as { sync: SyncState }; + const socket = getSocket(); + + // Only emit if we're fully connected (not during initial sync) + if (socket?.connected && state.sync.state === State.connected) { + const doc = action.payload; + debug(`emitting docUpdate for ${doc._id}`); + socket.emit('docUpdate', [doc]); + } + + // Always clean stale docs after processing + // If offline, the full sync on reconnect will handle them + storeApi.dispatch(cleanStale([action.payload])); + } + + return result; +}; diff --git a/src/client/store.ts b/src/client/store.ts index 07ec01e..e970990 100644 --- a/src/client/store.ts +++ b/src/client/store.ts @@ -3,7 +3,7 @@ import * as RR from 'react-redux'; import debugSlice, { debugMiddleware } from './features/Debug/debugSlice'; import pageSlice from './features/Page/pageSlice'; -import syncSlice from './features/Sync/syncSlice'; +import syncSlice, { syncMiddleware } from './features/Sync/syncSlice'; import updateSlice from './features/Update/updateSlice'; import userReducer from './features/User/userSlice'; import docsSlice from './state/docsSlice'; @@ -18,7 +18,8 @@ function createStore() { debug: debugSlice, update: updateSlice, }, - middleware: (getDefaultMiddleware) => getDefaultMiddleware().concat(debugMiddleware), + middleware: (getDefaultMiddleware) => + getDefaultMiddleware().concat(debugMiddleware, syncMiddleware), // devTools: process.env.NODE_ENV !== 'production', }); }