Move pull and push states from SyncSitesProvider to a new Redux slice #2037

gcsecsey · 2025-11-07T12:12:44Z

Related issues

Fixes STU-711

Proposed Changes

Removed SyncSitesProvider completely
Added a new syncOperations Redux slice
Moved logic from use-sync-pull and use-sync-push hooks to Redux thunks
Kept the hooks as think wrappers of the tunks
Removed some unused callback props

Testing Instructions

Run npm start
Go to the Sync tab
Connect a site
Pull changes from the site and check that there are no regressions
Push changes to the site and check that there are no regressions

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors?

github-actions · 2025-11-07T12:45:07Z

📊 Performance Test Results

Comparing fb2546e vs trunk

site-editor

Metric	trunk	`fb2546e`	Diff	Change
load	8207.00 ms	6809.00 ms	-1398.00 ms	🟢 -17.0%

site-startup

Metric	trunk	`fb2546e`	Diff	Change
siteCreation	9069.00 ms	9086.00 ms	+17.00 ms	🔴 0.2%
siteStartup	3953.00 ms	3956.00 ms	+3.00 ms	🔴 0.1%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change

nightnei

Push: it seems it stucks at "Creating backup..." step. Tried a few times and also tried in trunk - worked quickly for me.

Pull: not related to the changes, since I reproduce the error in trunk too, but if you are working there - take a look please at this error when pull is finished, maybe it's a quck win:

…dio-refactor-pull-and-push-states-from-syncsitesprovider

gcsecsey · 2025-12-11T17:28:20Z

Push: it seems it stucks at "Creating backup..." step. Tried a few times and also tried in trunk - worked quickly for me.

Pull: not related to the changes, since I reproduce the error in trunk too, but if you are working there - take a look please at this error when pull is finished, maybe it's a quck win:

Thanks for reviewing this @nightnei! I took another look at this, and found that earlier we used a ref to keep track of the state, which let us read it back immediately after updating it. For now, I added the ref to the hooks that were previously in the usePullPushStates hook.

I'm also testing a simpler approach that updated the state objects directly where needed. If it works reliably, I'll update the PR with this approach.

fredrikekelund · 2025-12-12T10:10:36Z

@gcsecsey, I would challenge us to remove src/hooks/sync-sites/sync-sites-context.tsx completely as part of this refactor.

With this change, we are preserving functions like updatePushState in the React context, even though they are now essentially Redux action creators (or thunks, I guess). The logic around sync operations would be much easier to follow if we moved src/hooks/sync-sites/use-sync-push.ts and src/hooks/sync-sites/use-sync-pull.ts into src/stores/sync/sync-operations-slice.ts.

gcsecsey · 2025-12-12T12:34:32Z

@gcsecsey, I would challenge us to remove src/hooks/sync-sites/sync-sites-context.tsx completely as part of this refactor.

With this change, we are preserving functions like updatePushState in the React context, even though they are now essentially Redux action creators (or thunks, I guess). The logic around sync operations would be much easier to follow if we moved src/hooks/sync-sites/use-sync-push.ts and src/hooks/sync-sites/use-sync-pull.ts into src/stores/sync/sync-operations-slice.ts.

That's a good point @fredrikekelund, thanks! I'll try to move as much as possible to the Redux slice, and ping for another review.

Remove the SyncSitesProvider React context layer and migrate all components to use useSyncPull and useSyncPush hooks directly. Changes: - Extract getLastSyncTimeText to useLastSyncTimeText hook - Extract initialization logic to useInitializeSyncStates hook - Move useListenDeepLinkConnection and initialization to App component - Update all components to use hooks directly instead of context - Remove sync-sites-context.tsx

…dio-refactor-pull-and-push-states-from-syncsitesprovider

gcsecsey · 2025-12-23T14:56:27Z

I carried out the changes to move most of the logic into thunks within src/stores/sync/sync-operations-slice.ts. I experimented with removing the src/hooks/sync-sites/use-sync-push.ts and src/hooks/sync-sites/use-sync-pull.ts hooks completely too, and update the components to directly use the thunks, but I found this needed quite a bit of boilerplate in each component, so for the time being, I kept the hooks as thin wrappers.

@fredrikekelund could you please take another look when you're back? Thanks!

nightnei

Thanks for addressing feedback, works well now, no regressions 👍

…s-from-syncsitesprovider Resolve merge conflicts: - use-sync-push.ts: Keep Redux-based approach with thunks and useSyncPolling - use-add-site.test.tsx: Update mock to use separate useSyncPull hook - sync-connected-sites.tsx: Merge isUploadingPaused feature with optional chaining - index.test.tsx: Use useSyncPull mock instead of useSyncSites - add-site.test.tsx: Update mock for separate hook pattern Also fix: - sync-operations-slice.ts: Add missing selectedSite.id parameter to pushArchive call 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

gcsecsey · 2026-01-05T13:35:12Z

I tested the changes again after resolving conflicts, the pull/push functionality still works as before. We can proceed with merging this as soon as the project process is 🟢 again.

fredrikekelund · 2026-01-06T11:14:35Z

Trunk is open for merge again. I'll happily review this PR this afternoon, though

gcsecsey · 2026-01-06T11:17:40Z

Trunk is open for merge again. I'll happily review this PR this afternoon, though

Thanks @fredrikekelund, that'd be great!

fredrikekelund · 2026-01-06T14:12:42Z

Looking into this now. At first glance, I'm a little skeptical to keeping src/hooks/sync-sites/use-sync-pull.ts and src/hooks/sync-sites/use-sync-push.ts as thin wrappers, but maybe I'll be swayed when looking more closely at the code.

To me, the end goal is to move "business logic" into Redux slices and have components focused on consuming the Redux-derived state, with as little data crunching as possible. The useSyncPull and useSyncPush hooks don't exactly contradict that, so maybe it just irks me that they seem like leftovers at first glance…

Anyway, you mentioned the need for additional boilerplate in components if we removed those hooks, @gcsecsey. Is that stuff like useAppDispatch and useAuth calls, or something else? The boilerplate in useSyncPull and useSyncPush looks manageable to me, if that's anything to judge by.

This reverts commit 55b1c33.

gcsecsey · 2026-01-06T16:59:03Z

Looking into this now. At first glance, I'm a little skeptical to keeping src/hooks/sync-sites/use-sync-pull.ts and src/hooks/sync-sites/use-sync-push.ts as thin wrappers, but maybe I'll be swayed when looking more closely at the code.

To me, the end goal is to move "business logic" into Redux slices and have components focused on consuming the Redux-derived state, with as little data crunching as possible. The useSyncPull and useSyncPush hooks don't exactly contradict that, so maybe it just irks me that they seem like leftovers at first glance…

Anyway, you mentioned the need for additional boilerplate in components if we removed those hooks, @gcsecsey. Is that stuff like useAppDispatch and useAuth calls, or something else? The boilerplate in useSyncPull and useSyncPush looks manageable to me, if that's anything to judge by.

Yes, exactly, I wanted to avoid having to add these as dependencies to each component. I think the boilerplate would be the useAppDispatch, useAuth, and pushStatesProgressInfo hooks, as these are needed when dispatching the pullSite or pushSite thunks currently. I also migrated some functions like isAnySitePushing to selectors, and the hook now just wraps this call to avoid the useRootSelector dependency on each call site.

fredrikekelund

Sorry for not following up on my review last week, @gcsecsey. I'm coming back to this now!

I think there's a solid foundation in sync-operations-slice.ts, but there's room to simplify the implementation, not least by following some Redux patterns more closely.

I'll submit multiple reviews to make it a bit easier to digest. Starting with sync-operations-slice.ts and pushSiteThunk primarily.

fredrikekelund · 2026-01-14T13:21:21Z

src/stores/sync/sync-operations-slice.ts

+const isKeyCancelled = ( key: string | undefined ): boolean => {
+	return key === 'cancelled';
+};
+
+const isKeyFailed = ( key: string | undefined ): boolean => {
+	return key === 'failed';
+};
+
+const isKeyFinished = ( key: string | undefined ): boolean => {
+	return key === 'finished';
+};


These functions shouldn't be needed in this file. We should just rely on well-typed string props and compare against known values. I.e. state.pullStates[ key ].status.key already has an enum type.

fredrikekelund · 2026-01-14T13:26:23Z

src/stores/sync/sync-operations-slice.ts

+		dispatch(
+			syncOperationsActions.clearPushState( { selectedSiteId: selectedSite.id, remoteSiteId } )
+		);
+		void dispatch(
+			syncOperationsThunks.clearPushState( { selectedSiteId: selectedSite.id, remoteSiteId } )
+		);


The clearPushState thunk could also be responsible for updating the state. Seems logical to consolidate that and not depend on the consumer to remember to also update the state if calling the thunk.

fredrikekelund · 2026-01-14T13:35:35Z

src/stores/sync/sync-operations-slice.ts

+		updatePushStateWithIpc(
+			dispatch,
+			selectedSite.id,
+			remoteSiteId,
+			{
+				remoteSiteId,
+				status: pushStatesProgressInfo.creatingBackup,
+				selectedSite,
+				remoteSiteUrl,
+			},
+			isKeyFailed,
+			isKeyFinished
+		);


Suggested change

updatePushStateWithIpc(

dispatch,

selectedSite.id,

remoteSiteId,

{

remoteSiteId,

status: pushStatesProgressInfo.creatingBackup,

selectedSite,

remoteSiteUrl,

},

isKeyFailed,

isKeyFinished

);

dispatch(

syncOperationsActions.updatePushState( {

selectedSiteId: selectedSite.id,

remoteSiteId,

state: {

remoteSiteId,

status: pushStatesProgressInfo.creatingBackup,

selectedSite,

remoteSiteUrl,

},

} )

);

A couple of things here:

Let's remove updatePushStateWithIpc.

The calls to getIpcApi().addSyncOperation and getIpcApi().clearSyncOperation should be an effect of state updates, they shouldn't have to be managed by whoever is updating the state. In other words, we should subscribe to state changes and call getIpcApi().addSyncOperation and getIpcApi().clearSyncOperation as needed.

Let's simplify the signature of updatePushState. Passing selectedSiteId and state.selectedSite, remoteSiteId twice, and remoteSiteUrl twice seems redundant. updatePushState could be smarter internally about passing the right data to the right place.

subscribe to state changes

Here's an example of that:

studio/src/stores/index.ts

Lines 56 to 68 in 704107a

// Save chat messages to local storage

listenerMiddleware.startListening( {

predicate( action, currentState, previousState ) {

return currentState.chat.messagesDict !== previousState.chat.messagesDict;

},

effect( action, listenerApi ) {

const state = listenerApi.getState();

localStorage.setItem(

LOCAL_STORAGE_CHAT_MESSAGES_KEY,

JSON.stringify( state.chat.messagesDict )

);

},

} );

fredrikekelund · 2026-01-14T13:43:32Z

src/stores/sync/sync-operations-slice.ts

+		let archivePath: string, archiveSizeInBytes: number;
+
+		try {
+			const result = await getIpcApi().exportSiteForPush( selectedSite.id, operationId, {
+				optionsToSync: options?.optionsToSync,
+				specificSelectionPaths: options?.specificSelectionPaths,
+			} );
+			( { archivePath, archiveSizeInBytes } = result );
+		} catch ( error ) {
+			if ( error instanceof Error && error.message === 'Export aborted' ) {
+				updatePushStateWithIpc(
+					dispatch,
+					selectedSite.id,
+					remoteSiteId,
+					{ status: pushStatesProgressInfo.cancelled },
+					isKeyFailed,
+					isKeyFinished
+				);
+				throw error; // Signal cancellation
+			}
+
+			Sentry.captureException( error );
+			updatePushStateWithIpc(
+				dispatch,
+				selectedSite.id,
+				remoteSiteId,
+				{ status: pushStatesProgressInfo.failed },
+				isKeyFailed,
+				isKeyFinished
+			);
+			getIpcApi().showErrorMessageBox( {
+				title: sprintf( __( 'Error pushing to %s' ), connectedSite.name ),
+				message: __(
+					'An error occurred while pushing the site. If this problem persists, please contact support.'
+				),
+				error,
+				showOpenLogs: true,
+			} );
+			throw error;
+		}


Suggested change

let archivePath: string, archiveSizeInBytes: number;

try {

const result = await getIpcApi().exportSiteForPush( selectedSite.id, operationId, {

optionsToSync: options?.optionsToSync,

specificSelectionPaths: options?.specificSelectionPaths,

} );

( { archivePath, archiveSizeInBytes } = result );

} catch ( error ) {

if ( error instanceof Error && error.message === 'Export aborted' ) {

updatePushStateWithIpc(

dispatch,

selectedSite.id,

remoteSiteId,

{ status: pushStatesProgressInfo.cancelled },

isKeyFailed,

isKeyFinished

);

throw error; // Signal cancellation

}

Sentry.captureException( error );

updatePushStateWithIpc(

dispatch,

selectedSite.id,

remoteSiteId,

{ status: pushStatesProgressInfo.failed },

isKeyFailed,

isKeyFinished

);

getIpcApi().showErrorMessageBox( {

title: sprintf( __( 'Error pushing to %s' ), connectedSite.name ),

message: __(

'An error occurred while pushing the site. If this problem persists, please contact support.'

),

error,

showOpenLogs: true,

} );

throw error;

}

const { archivePath, archiveSizeInBytes } = await getIpcApi().exportSiteForPush(

selectedSite.id,

operationId,

{

optionsToSync: options?.optionsToSync,

specificSelectionPaths: options?.specificSelectionPaths,

}

);

Let's unwrap this:

The updatePushStateWithIpc calls could be moved to extraReducers. We would use builder.addCase( pushSiteThunk.rejected to listen for failures.

The error modal (as triggered by getIpcApi().showErrorMessageBox) is also a side effect of the state update. Let's try moving it to a state subscription callback.

fredrikekelund · 2026-01-14T13:45:26Z

src/stores/sync/sync-operations-slice.ts

+			getIpcApi().showErrorMessageBox( {
+				title: sprintf( __( 'Error pushing to %s' ), connectedSite.name ),
+				message: __(
+					'The site is too large to push. Please reduce the size of the site and try again.'
+				),
+			} );


Suggested change

getIpcApi().showErrorMessageBox( {

title: sprintf( __( 'Error pushing to %s' ), connectedSite.name ),

message: __(

'The site is too large to push. Please reduce the size of the site and try again.'

),

} );

This call isn't particularly bulky, but if we consider the error modals a side effect of state updates elsewhere (i.e., that they should be triggered by a state subscription listener), then let's be consistent in that approach.

fredrikekelund · 2026-01-14T13:54:35Z

src/stores/sync/sync-operations-slice.ts

+		// Check if cancelled before upload
+		const state = getState();
+		const currentPushState = syncOperationsSelectors.selectPushState(
+			selectedSite.id,
+			remoteSiteId
+		)( state );
+		if (
+			! currentPushState ||
+			! currentPushState.status ||
+			isKeyCancelled( currentPushState.status.key )
+		) {
+			await getIpcApi().removeExportedSiteTmpFile( archivePath );
+			throw new Error( 'Push cancelled' );
+		}


Could we use something more similar to this approach? https://redux-toolkit.js.org/api/createAsyncThunk#canceling-while-running

fredrikekelund · 2026-01-14T13:54:51Z

src/stores/sync/sync-operations-slice.ts

+			// Check if cancelled after upload
+			const stateAfterUpload = getState();
+			const pushStateAfterUpload = syncOperationsSelectors.selectPushState(
+				selectedSite.id,
+				remoteSiteId
+			)( stateAfterUpload );
+
+			if ( isKeyCancelled( pushStateAfterUpload?.status.key ) ) {
+				await getIpcApi().removeExportedSiteTmpFile( archivePath );
+				throw new Error( 'Push cancelled' );
+			}


See my previous comment about aborting the thunk.

fredrikekelund · 2026-01-14T14:00:17Z

src/stores/sync/sync-operations-slice.ts

+					remoteSiteUrl,
+				};
+			} else {
+				throw response;


Suggested change

throw response;

throw new Error( response.error );

This seems more appropriate.

fredrikekelund · 2026-01-14T14:00:50Z

src/stores/sync/sync-operations-slice.ts

+			Sentry.captureException( error );
+			updatePushStateWithIpc(
+				dispatch,
+				selectedSite.id,
+				remoteSiteId,
+				{ status: pushStatesProgressInfo.failed },
+				isKeyFailed,
+				isKeyFinished
+			);


Let's move this to an extraReducers handler.

fredrikekelund · 2026-01-14T14:02:08Z

src/stores/sync/sync-operations-slice.ts

+			getIpcApi().showErrorMessageBox( {
+				title: sprintf( __( 'Error pushing to %s' ), connectedSite.name ),
+				message: getErrorFromResponse( error ),
+			} );


Let's move this to a state subscription handler.

gcsecsey · 2026-01-14T14:13:08Z

Thanks @fredrikekelund for following up with the review! 🙌 First and foremost, I'll try to resolve all conflicts with trunk, then I'll start working on these changes.

fredrikekelund

Comments related to pullSiteThunk

fredrikekelund · 2026-01-14T14:14:17Z

src/stores/sync/sync-operations-slice.ts

+		// Clear existing state
+		dispatch(
+			syncOperationsActions.clearPullState( { selectedSiteId: selectedSite.id, remoteSiteId } )
+		);
+		void dispatch(
+			syncOperationsThunks.clearPullState( { selectedSiteId: selectedSite.id, remoteSiteId } )
+		);


Again, let's make the clearPullState thunk update the state. Using the extraReducers API might be the best way to accomplish that.

fredrikekelund · 2026-01-14T14:15:03Z

src/stores/sync/sync-operations-slice.ts

+		dispatch(
+			syncOperationsActions.updatePullState( {
+				selectedSiteId: selectedSite.id,
+				remoteSiteId,
+				state: {
+					backupId: null,
+					status: pullStatesProgressInfo[ 'in-progress' ],
+					downloadUrl: null,
+					remoteSiteId,
+					remoteSiteUrl,
+					selectedSite,
+				},
+			} )
+		);


Same as for the updatePushState action creator, let's see if we can simplify the function signature to reduce the repetition here.

fredrikekelund · 2026-01-14T14:15:24Z

src/stores/sync/sync-operations-slice.ts

+		// Add sync operation for tracking
+		const stateId = generateStateId( selectedSite.id, remoteSiteId );
+		getIpcApi().addSyncOperation( stateId );


Let's move this to a state subscription handler.

fredrikekelund · 2026-01-14T14:15:49Z

src/stores/sync/sync-operations-slice.ts

+			Sentry.captureException( error );
+			dispatch(
+				syncOperationsActions.updatePullState( {
+					selectedSiteId: selectedSite.id,
+					remoteSiteId,
+					state: {
+						status: pullStatesProgressInfo.failed,
+					},
+				} )
+			);


Let's move this to an extraReducers callback.

fredrikekelund · 2026-01-14T14:16:13Z

src/stores/sync/sync-operations-slice.ts

+			getIpcApi().showErrorMessageBox( {
+				title: sprintf( __( 'Error pulling from %s' ), connectedSite.name ),
+				message: __( 'Studio was unable to connect to WordPress.com. Please try again.' ),
+			} );


Let's move this to a state subscription handler.

fredrikekelund · 2026-01-14T14:25:15Z

src/stores/sync/sync-operations-slice.ts

+		let status: PushStateProgressInfo = pushStatesProgressInfo.creatingRemoteBackup;
+		if ( response.success && response.status === 'finished' ) {


Suggested change

let status: PushStateProgressInfo = pushStatesProgressInfo.creatingRemoteBackup;

if ( response.success && response.status === 'finished' ) {

if ( ! response.success ) {

throw new Error( response.error );

}

let status: PushStateProgressInfo;

switch ( response.status ) {

case 'initial_backup_started':

status = pushStatesProgressInfo.creatingRemoteBackup;

break;

case 'finished':

Let's make this a switch statement and add a case for initial_backup_started. This will allow TS to understand that we've exhausted all possible values of response.status when we call getPushStatusWithProgress

fredrikekelund · 2026-01-14T14:26:57Z

src/hooks/use-sync-states-progress-info.ts

AFAICT, the getPushStatusWithProgress function exported by this hook isn't used anywhere. Let's remove it

Same for getPullStatusWithProgress

And for getBackupStatusWithProgress

I also think we could remove isKeyPulling, isKeyPushing, isKeyFinished, isKeyFailed, isKeyCancelled, and isKeyUploadingPaused. They're only used in src/modules/sync/components/sync-connected-sites.tsx and could easily be replaced with plain equality comparisons.

isKeyImporting isn't used anywhere. Let's remove it.

fredrikekelund · 2026-01-14T14:28:07Z

src/stores/sync/sync-operations-slice.ts

+			getIpcApi().showNotification( {
+				title: currentPushState.selectedSite.name,
+				body: sprintf(
+					// translators: %s is the site url without the protocol.
+					__( '%s has been updated' ),
+					getHostnameFromUrl( currentPushState.remoteSiteUrl )
+				),
+			} );


Side effect. Let's try to move this to a state subscription listener.

fredrikekelund · 2026-01-14T14:29:09Z

src/stores/sync/sync-operations-slice.ts

+// Helper function to calculate push status with progress (inlined from useSyncStatesProgressInfo)
+const getPushStatusWithProgress = (
+	status: PushStateProgressInfo,
+	response: ImportResponse,
+	pushStatesProgressInfo: Record< PushStateProgressInfo[ 'key' ], PushStateProgressInfo >
+): PushStateProgressInfo => {
+	if ( status.key === pushStatesProgressInfo.creatingRemoteBackup.key ) {
+		const progressRange =
+			pushStatesProgressInfo.applyingChanges.progress -
+			pushStatesProgressInfo.creatingRemoteBackup.progress;
+
+		// This step will increase the progress to the next step progressively based on the backup_progress
+		return {
+			...status,
+			progress:
+				pushStatesProgressInfo.creatingRemoteBackup.progress +
+				progressRange * ( response.backup_progress / 100 ),
+		};
+	}
+
+	// This step will increase the progress to the next step progressively based on the import_progress
+	if (
+		status.key === pushStatesProgressInfo.applyingChanges.key &&
+		response.import_progress < 100
+	) {
+		const progressRange =
+			pushStatesProgressInfo.finishing.progress - pushStatesProgressInfo.applyingChanges.progress;
+		return {
+			...status,
+			progress:
+				pushStatesProgressInfo.applyingChanges.progress +
+				progressRange * ( response.import_progress / 100 ),
+		};
+	}
+	return status;
+};


Let's inline this logic in pollPushProgressThunk. I believe it will end up shorter and easier to read.

fredrikekelund · 2026-01-14T14:32:23Z

src/stores/sync/sync-operations-slice.ts

+		updatePushStateWithIpc(
+			dispatch,
+			selectedSiteId,
+			remoteSiteId,
+			{ status },
+			isKeyFailed,
+			isKeyFinished
+		);


Suggested change

updatePushStateWithIpc(

dispatch,

selectedSiteId,

remoteSiteId,

{ status },

isKeyFailed,

isKeyFinished

);

dispatch(

syncOperationsActions.updatePushState( {

selectedSiteId,

remoteSiteId,

state: { status },

} )

);

Again, let's remove updatePushStateWithIpc

fredrikekelund

Comments related to pollPullBackupThunk and completePullThunk

fredrikekelund · 2026-01-14T14:53:53Z

src/stores/sync/sync-operations-slice.ts

+		// Check if state exists and is not cancelled
+		const state = getState();
+		const currentPullState = syncOperationsSelectors.selectPullState(
+			selectedSiteId,
+			remoteSiteId
+		)( state );
+
+		if (
+			! currentPullState ||
+			! currentPullState.status ||
+			isKeyCancelled( currentPullState.status.key )
+		) {
+			return;
+		}


Same as for pollPushProgressThunk, we should trigger (or perhaps register/unregister a setInterval/setTimeout callback) in a state subscription callback. If we do that, we should be able to remove this logic that starts the thunk by checking if we should cancel the operation.

fredrikekelund · 2026-01-14T14:57:26Z

src/stores/sync/sync-operations-slice.ts

+				// Backup completed, trigger completion thunk
+				await dispatch(
+					syncOperationsThunks.completePull( {
+						selectedSiteId,
+						remoteSiteId,
+						downloadUrl,
+						pullStatesProgressInfo,
+					} )
+				).unwrap();


completePull doesn't actually stand on its own legs. I.e., this is the only place we call that thunk. I argue we should inline the logic from this function into this thunk. It'll clarify the overall API in this file.

fredrikekelund · 2026-01-14T14:58:14Z

src/stores/sync/sync-operations-slice.ts

+// Helper function to calculate backup status with progress (inlined from useSyncStatesProgressInfo)
+const getBackupStatusWithProgress = (
+	hasBackupCompleted: boolean,
+	pullStatesProgressInfo: Record< PullStateProgressInfo[ 'key' ], PullStateProgressInfo >,
+	response: SyncBackupResponse
+): PullStateProgressInfo => {
+	const frontendStatus = hasBackupCompleted
+		? pullStatesProgressInfo.downloading.key
+		: response.status;
+	let newProgressInfo: PullStateProgressInfo | null = null;
+	if ( response.status === 'in-progress' ) {
+		newProgressInfo = {
+			...pullStatesProgressInfo[ frontendStatus ],
+			// Update progress from the initial value to the new step proportionally to the response.progress
+			// on every update of the response.progress
+			progress:
+				IN_PROGRESS_INITIAL_VALUE + IN_PROGRESS_TO_DOWNLOADING_STEP * ( response.percent / 100 ),
+		};
+	}
+	const statusWithProgress = newProgressInfo || pullStatesProgressInfo[ frontendStatus ];
+
+	return statusWithProgress;
+};


Same as with getPushStatusWithProgress, I would just inline this logic in pollPullBackupThunk.

fredrikekelund · 2026-01-14T14:58:45Z

src/stores/sync/sync-operations-slice.ts

+		} catch ( error ) {
+			console.error( 'Failed to fetch backup status:', error );
+			throw error;
+		}


Let's move this error handling to extraReducers

fredrikekelund · 2026-01-14T14:59:07Z

src/stores/sync/sync-operations-slice.ts

+				// Update IPC sync operation
+				const stateId = generateStateId( selectedSiteId, remoteSiteId );
+				getIpcApi().addSyncOperation( stateId );


Side effect. Let's move to a state subscription callback.

fredrikekelund · 2026-01-14T15:01:53Z

src/stores/sync/sync-operations-slice.ts

+			// Check if cancelled after download
+			const stateAfterDownload = getState();
+			const pullStateAfterDownload = syncOperationsSelectors.selectPullState(
+				selectedSiteId,
+				remoteSiteId
+			)( stateAfterDownload );
+
+			if (
+				! pullStateAfterDownload ||
+				! pullStateAfterDownload.status ||
+				isKeyCancelled( pullStateAfterDownload.status.key )
+			) {
+				return;
+			}


Again, let's try this strategy for aborting thunks.

fredrikekelund · 2026-01-14T15:02:28Z

src/stores/sync/sync-operations-slice.ts

+			// Show notification
+			getIpcApi().showNotification( {
+				title: selectedSite.name,
+				body: sprintf(
+					// translators: %s is the site url without the protocol.
+					__( 'Studio site has been updated from %s' ),
+					getHostnameFromUrl( remoteSiteUrl )
+				),
+			} );


Side effect. Let's move to a state subscription listener.

fredrikekelund · 2026-01-14T15:03:25Z

src/stores/sync/sync-operations-slice.ts

+			Sentry.captureException( error );
+			dispatch(
+				syncOperationsActions.updatePullState( {
+					selectedSiteId,
+					remoteSiteId,
+					state: {
+						status: pullStatesProgressInfo.failed,
+					},
+				} )
+			);


Let's move this to extraReducers. Again, using a notation like this: builder.addCase( pollPullBackupThunk.rejected

fredrikekelund · 2026-01-14T15:03:39Z

src/stores/sync/sync-operations-slice.ts

+			getIpcApi().showErrorMessageBox( {
+				title: sprintf( __( 'Error pulling from %s' ), selectedSite.name ),
+				message: __( 'Failed to check backup file size. Please try again.' ),
+			} );


Side effect. Let's move to a state subscription handler.

fredrikekelund · 2026-01-14T15:04:18Z

src/stores/sync/sync-operations-slice.ts

+		} catch ( error ) {
+			console.error( 'Backup completion failed:', error );
+
+			// Check if cancelled
+			const errorState = getState();
+			const pullStateOnError = syncOperationsSelectors.selectPullState(
+				selectedSiteId,
+				remoteSiteId
+			)( errorState );
+
+			if (
+				pullStateOnError &&
+				pullStateOnError.status &&
+				isKeyCancelled( pullStateOnError.status.key )
+			) {
+				return;
+			}


Let's remove this. Errors can be handled per the strategy outlined below.

…dio-refactor-pull-and-push-states-from-syncsitesprovider

Migrate the changes of PR #2332

fredrikekelund · 2026-01-14T15:32:29Z

src/hooks/sync-sites/use-initialize-sync-states.ts

Let's make this a thunk in src/stores/sync/sync-operations-slice.ts. Because the auth details are still not stored in Redux, we can dispatch the thunk from src/components/app.tsx and pass the client from the useAuth return value.

fredrikekelund · 2026-01-14T15:35:31Z

src/hooks/sync-sites/use-sync-polling.ts

If we can trigger the polling thunks from state subscription callbacks, then we should be able to remove this hook.

fredrikekelund · 2026-01-14T15:39:36Z

If we move the logic for triggering polling thunks to state subscription callbacks, that should unblock us from removing useSyncPull and useSyncPush, as the remaining functions exported from those hooks are basically just Redux wrappers.

Let's focus on simplifying sync-operations-slice.ts and implementing the patterns I suggested in my reviews. Hopefully, that will unlock relatively straightforward removals of those hooks 🤞

add new slice for pull and push states

7cf45db

gcsecsey requested a review from a team November 7, 2025 12:12

wojtekn assigned gcsecsey Nov 11, 2025

nightnei reviewed Nov 18, 2025

View reviewed changes

gcsecsey added 2 commits December 11, 2025 16:33

Merge branch 'trunk' of github.com:Automattic/studio into stu-711-stu…

1c23381

…dio-refactor-pull-and-push-states-from-syncsitesprovider

update useSyncPull and useSyncPush hooks

7d2d1b0

gcsecsey added 5 commits December 12, 2025 13:34

add selectors from hook to slice

fe07c5e

move clear/cancel operations to thunks

a989798

move pushsite to thunk

5640cbb

move pullsite to thunk

dbf3f4f

extract polling logic to separate hook

6b8f4c0

gcsecsey marked this pull request as draft December 17, 2025 14:30

gcsecsey added 12 commits December 22, 2025 12:29

move polling logic to thunk

a284a77

remove unused onPullSuccess/onPushSuccess props

be60973

Merge branch 'trunk' of github.com:Automattic/studio into stu-711-stu…

3c5ca95

…dio-refactor-pull-and-push-states-from-syncsitesprovider

remove unused actions

ff2184b

fix unused reference after merge

ec936a0

update tests

f60e93b

add null checks

c5750d0

Merge branch 'trunk' of github.com:Automattic/studio into stu-711-stu…

860a1b0

…dio-refactor-pull-and-push-states-from-syncsitesprovider

fix linter errors

d278105

add runtime checks

f82a301

add optional chaining to progress data checks

4872b64

gcsecsey requested a review from fredrikekelund December 23, 2025 14:56

gcsecsey marked this pull request as ready for review December 23, 2025 15:11

nightnei approved these changes Dec 23, 2025

View reviewed changes

gcsecsey mentioned this pull request Jan 6, 2026

Sync: Show file upload progress in the UI #2332

Merged

Merge trunk and resolve conflicts

55b1c33

Revert "Merge trunk and resolve conflicts"

3ceedf4

This reverts commit 55b1c33.

fredrikekelund reviewed Jan 14, 2026

View reviewed changes

gcsecsey added 2 commits January 14, 2026 15:18

Merge branch 'trunk' of github.com:Automattic/studio into stu-711-stu…

0277edc

…dio-refactor-pull-and-push-states-from-syncsitesprovider

Add back upload progress indicator

fb2546e

Migrate the changes of PR #2332

fredrikekelund reviewed Jan 14, 2026

View reviewed changes

	// Save chat messages to local storage
	listenerMiddleware.startListening( {
	predicate( action, currentState, previousState ) {
	return currentState.chat.messagesDict !== previousState.chat.messagesDict;
	},
	effect( action, listenerApi ) {
	const state = listenerApi.getState();
	localStorage.setItem(
	LOCAL_STORAGE_CHAT_MESSAGES_KEY,
	JSON.stringify( state.chat.messagesDict )
	);
	},
	} );

		let status: PushStateProgressInfo = pushStatesProgressInfo.creatingRemoteBackup;
		if ( response.success && response.status === 'finished' ) {

-		let status: PushStateProgressInfo = pushStatesProgressInfo.creatingRemoteBackup;
-		if ( response.success && response.status === 'finished' ) {
+		if ( ! response.success ) {
+			throw new Error( response.error );
+		}
+		let status: PushStateProgressInfo;
+		switch ( response.status ) {
+			case 'initial_backup_started':
+				status = pushStatesProgressInfo.creatingRemoteBackup;
+				break;
+			case 'finished':

Move pull and push states from SyncSitesProvider to a new Redux slice #2037

Are you sure you want to change the base?

Move pull and push states from SyncSitesProvider to a new Redux slice #2037

Uh oh!

Conversation

gcsecsey commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issues

Proposed Changes

Testing Instructions

Pre-merge Checklist

Uh oh!

github-actions bot commented Nov 7, 2025 • edited by wpmobilebot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Performance Test Results

site-editor

site-startup

Uh oh!

nightnei left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gcsecsey commented Dec 11, 2025

Uh oh!

fredrikekelund commented Dec 12, 2025

Uh oh!

gcsecsey commented Dec 12, 2025

Uh oh!

gcsecsey commented Dec 23, 2025

Uh oh!

nightnei left a comment

Choose a reason for hiding this comment

Uh oh!

gcsecsey commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredrikekelund commented Jan 6, 2026

Uh oh!

gcsecsey commented Jan 6, 2026

Uh oh!

fredrikekelund commented Jan 6, 2026

Uh oh!

gcsecsey commented Jan 6, 2026

Uh oh!

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gcsecsey commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredrikekelund left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

gcsecsey commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited by wpmobilebot

Loading

nightnei left a comment •

edited

Loading

gcsecsey commented Jan 5, 2026 •

edited

Loading

gcsecsey commented Jan 14, 2026 •

edited

Loading

fredrikekelund Jan 14, 2026 •

edited

Loading