Refactor sync-support.ts #2359

nightnei · 2026-01-07T13:18:15Z

Proposed Changes

I worked around the sync-support module and didn't get how to reproduce cases, since the conditions are not clear.
See comments.

Testing Instructions

Assert that everything still works well:

Create jurassic.ninja site, install jetpack and conntect to dotcom, open Studio sync tab and assert that it's marked as unsupported.
Create Business site, open bulk-delete-test-sites, revert to simple, open Studio sync tab and assert that the site is marked as "Enable hosting features"
Test wp.com sites and assert that they are marked as "syncable" only for Business+ plan.
Test Pressable sites and assert that they are always marked as "syncable"

nightnei · 2026-01-07T13:18:48Z

src/modules/sync/lib/sync-support.ts

 	if ( ! site.capabilities?.manage_options ) {
 		return 'missing-permissions';
 	}
-	if ( isJetpackSite( site ) ) {


Why jetpack site is unsupported?
Jetpack is required. So I renamed to isUnsupported.

Similar as in my previous comment:

As far as I remember, we used this in the past to filter out self-hosted sites that are not hosted by us (via WPCOM and Pressable).

In that logic, "Jetpack site" referred to "self-hosted WordPress site".

nightnei · 2026-01-07T13:21:10Z

src/modules/sync/lib/sync-support.ts

 }

-export function isJetpackSite( site: SitesEndpointSite ): boolean {
-	return !! site.jetpack && ! isAtomicSite( site ) && ! isPressableSite( site );


Why detection of jetpack includes ! isAtomic && ! isPressable? Theoretically !! site.jetpack should be enough.

I took a look at history and now the function directly says that jetpack site, which is not atomic is unsupported, and the example is jurasic.ninja

Why detection of jetpack includes ! isAtomic && ! isPressable?

As far as I remember, we used this in the past to filter out self-hosted sites that are not hosted by us (via WPCOM and Pressable).

In that logic, "Jetpack site" referred to "self-hosted WordPress site".

The main point here - below we have a condition that Jetpack sites are unsupported:

if ( isJetpackSite( site ) ) { return 'unsupported'; }

But it's not true, since all supported sites must have Jetpack. I worked around this code, needed cases to reproduce it, and got stuck.

In that logic, "Jetpack site" referred to "self-hosted WordPress site".

Then it should be either isUnsupported or isSelfHosted, but not isJetpackSite.

nightnei · 2026-01-07T13:26:00Z

src/modules/sync/lib/sync-support.ts

 }

-export function needsTransfer( site: SitesEndpointSite ): boolean {
-	return ! isJetpackSite( site ) && ! isPressableSite( site ) && ! isAtomicSite( site );


If we read this condition, it says:

NOT !! site.jetpack && ! isAtomicSite( site ) && ! isPressableSite( site )
&&

&& ! isPressableSite( site ) && ! isAtomicSite( site )

It's dificult to undersathnd when it's actually true. I have read the history of comits and simplified to just ! site.jetpack && ! isAtomicSite( site ). And added the comment to how to reproduce the case.

I think this was a workaround to check for a Simple site (a site that is not Jetpack, not Atomic and not Pressable).

It was as long time again since I looked at the old logic, but I believe there was some edge case that resulted in this more complex check.

The main point here, that the condition is - ! ( !! site.jetpack && ! isAtomicSite( site ) && ! isPressableSite( site ) ) && ! isPressableSite( site ) && ! isAtomicSite( site ), and it's not possible to understand it. And I haven't found any use case for something like this. Also reading commits history - I haven't found any "Testing instruction" or some PR description which says about some unusual use case.

nightnei · 2026-01-07T13:28:25Z

src/modules/sync/lib/sync-support.ts

+	if ( isUnsupported( site ) && ! isPressableSite( site ) ) {
 		return 'unsupported';
 	}
 	if ( ! hasSupportedPlan( site ) && ! isPressableSite( site ) ) {


In this condition we keep isPressableSite here, outside the hasSupportedPlan and imo it's good idea, to be clear at the upper level that the plan is about only dotcom, and it's not about Pressable.
So I added the same approach to two other cases (isUnsupported and needsTransfer).
Also, it's for consistency.

wpmobilebot · 2026-01-07T13:34:23Z

📊 Performance Test Results

Comparing 33366f8 vs trunk

site-editor

Metric	trunk	`33366f8`	Diff	Change
load	7138.00 ms	6991.00 ms	-147.00 ms	🟢 -2.1%

site-startup

Metric	trunk	`33366f8`	Diff	Change
siteCreation	9082.00 ms	9069.00 ms	-13.00 ms	🟢 -0.1%
siteStartup	3957.00 ms	3956.00 ms	-1.00 ms	🟢 -0.0%

Results are median values from multiple test runs.

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

ivan-ottinger

Thanks for the improvements, Vova. This logic is indeed difficult to troubleshoot.

So far the changes look generally good to me and I haven't noticed any regressions so far. I have left a couple of comments.

Tomorrow I will give this a second look with fresh 🧠 . 🙂

src/modules/sync/lib/sync-support.ts

ivan-ottinger · 2026-01-08T14:00:02Z

src/modules/sync/lib/sync-support.ts

 }

-export function isJetpackSite( site: SitesEndpointSite ): boolean {
-	return !! site.jetpack && ! isAtomicSite( site ) && ! isPressableSite( site );


Why detection of jetpack includes ! isAtomic && ! isPressable?

As far as I remember, we used this in the past to filter out self-hosted sites that are not hosted by us (via WPCOM and Pressable).

In that logic, "Jetpack site" referred to "self-hosted WordPress site".

ivan-ottinger · 2026-01-08T14:27:07Z

src/modules/sync/lib/sync-support.ts

 }

-export function needsTransfer( site: SitesEndpointSite ): boolean {
-	return ! isJetpackSite( site ) && ! isPressableSite( site ) && ! isAtomicSite( site );


I think this was a workaround to check for a Simple site (a site that is not Jetpack, not Atomic and not Pressable).

It was as long time again since I looked at the old logic, but I believe there was some edge case that resulted in this more complex check.

ivan-ottinger · 2026-01-08T14:40:43Z

src/modules/sync/lib/sync-support.ts


-export function isJetpackSite( site: SitesEndpointSite ): boolean {
-	return !! site.jetpack && ! isAtomicSite( site ) && ! isPressableSite( site );
+// Sites hosted outside wp.com and Pressable (e.g. jurassic.ninja).


While reviewing the code I wanted to suggest moving isPressableSite check into the isUnsupported helper function directly, but then I read your other comment below:

In this condition we keep isPressableSite here, outside the hasSupportedPlan and imo it's good idea, to be clear at the upper level that the plan is about only dotcom, and it's not about Pressable.
So I added the same approach to two other cases (isUnsupported and needsTransfer).
Also, it's for consistency.

Sounds good to me. 👍🏼 With that I think we may want to update the comment above:

// Sites hosted outside wp.com and Pressable (e.g. jurassic.ninja).

As now it looks as if isUnsupported also checks for Pressable sites.

ivan-ottinger · 2026-01-08T14:41:46Z

src/modules/sync/lib/sync-support.ts

 	if ( ! site.capabilities?.manage_options ) {
 		return 'missing-permissions';
 	}
-	if ( isJetpackSite( site ) ) {


Similar as in my previous comment:

As far as I remember, we used this in the past to filter out self-hosted sites that are not hosted by us (via WPCOM and Pressable).

In that logic, "Jetpack site" referred to "self-hosted WordPress site".

ivan-ottinger

The changes looks good to me. 👍🏼 I have tested the Sync modal with different kinds of sites and did not observe any regressions:

katinthehatsite

I just wanted to approve this but it looks like it is already merged. The changes looked good to me, I did not see any regressions in the Connect your site modal.

Refactor sync-support.ts

33366f8

nightnei commented Jan 7, 2026

View reviewed changes

nightnei requested a review from a team January 7, 2026 13:37

nightnei self-assigned this Jan 7, 2026

nightnei added the Reviewer can merge label Jan 8, 2026

ivan-ottinger reviewed Jan 8, 2026

View reviewed changes

ivan-ottinger approved these changes Jan 9, 2026

View reviewed changes

nightnei merged commit 6ab139b into trunk Jan 9, 2026
25 checks passed

nightnei deleted the refactorSyncSupport branch January 9, 2026 14:10

katinthehatsite reviewed Jan 9, 2026

View reviewed changes

Refactor sync-support.ts #2359

Refactor sync-support.ts #2359

Uh oh!

Conversation

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

Uh oh!

Proposed Changes

Testing Instructions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nightnei Jan 7, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

nightnei Jan 8, 2026 • 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!

nightnei Jan 7, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nightnei Jan 7, 2026 • 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!

wpmobilebot commented Jan 7, 2026

📊 Performance Test Results

site-editor

site-startup

Uh oh!

ivan-ottinger left a comment

Choose a reason for hiding this comment

Uh oh!

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!

ivan-ottinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

katinthehatsite 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nightnei commented Jan 7, 2026 •

edited

Loading

nightnei Jan 7, 2026 •

edited

Loading

nightnei Jan 8, 2026 •

edited

Loading

nightnei Jan 7, 2026 •

edited

Loading

nightnei Jan 7, 2026 •

edited

Loading

katinthehatsite left a comment •

edited

Loading