Experiment to speed up deeplink connection from WP.com#2375
Experiment to speed up deeplink connection from WP.com#2375katinthehatsite wants to merge 14 commits intotrunkfrom
Conversation
📊 Performance Test ResultsComparing 93a0ee7 vs trunk site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
|
The code needs to be polished quite a bit so I am leaving this PR as a draft. What I am looking for is the feedback on the approach before adding any further polishing touches - does it make sense? is there a different way we could optimize the speed of connection that could be simpler? One thing I dislike is creating a "fictional" site object that could potentially cause racing conditions and similar issues. Any opinions are welcome! One option as an alternative that comes to mind could be adding a loading state to Sync tab if the user is connecting a site from deeplink and it is taking time. This way, they could have a visual indication of what is happening and this could be a simple and fast improvement in terms of the UX. |
sejas
left a comment
There was a problem hiding this comment.
Thanks for exploring how to speed up the deeplink connection.
In general the approach makes sense to me. Loading an optimistic connection until we fetch more information.
My concern is if we display wrong data, like isStaging, or isPressable. With the current flow where the user purchase a new site, those will always be false, but in the future we could add more deeplinks in Calypso.
What if we show a loading skeleton until we fetch the information?
As mentioned below, we can optimize the fetch call to get only the information for that specific site.
| } | ||
|
|
||
| // Fetch full site data in background (don't await - parallel operation) | ||
| const refetchPromise = refetchWpComSites(); |
There was a problem hiding this comment.
Another optimization could be fetching only the information of the remoteSiteId. Currently fetching all the sites takes longer, specially for Automatticians.
There was a problem hiding this comment.
I found an endpoint that allowed to fetch just one site instead of all sites so I went with that solution.
| const minimalSite: SyncSite = { | ||
| id: remoteSiteId, | ||
| localSiteId: studioSiteId, | ||
| name: siteName || 'Loading site...', // Use provided name or placeholder |
There was a problem hiding this comment.
We need to translate the placeholder.
There was a problem hiding this comment.
I think it was not needed at the end after I added the loading placeholder so I removed it.
|
Thank you for review @sejas - I will take a look at the next steps today 👀 |
…p-connection-time
…p-connection-time
|
@sejas I made some adjustments on this branch based on your recommendations. Would you be able to give it another look? |
There was a problem hiding this comment.
@katinthehatsite , great work. Thanks for addressing my suggestions.
I tested it and it works much better with the loading skeleton. Thanks for improving this flow.
I left a couple of comments. Let me know what you think. I also noticed that the loading skeleton is visible even after the single site request resolves, so it seems in the end it uses the whole sites request to decide if should be loading or not. Or at least that was my impression.
I also noticed we try to use the siteurl and site title, but that information is not passed from the deep link. I think it's ok to remove those because the skeleton gives enough context IMO.
Firs time connection:
first-time-connection.mp4
Second time connection:
I also tested it by disconnecting and repeating the deep link.
second-time.mp4
apps/studio/src/hooks/sync-sites/use-listen-deep-link-connection.ts
Outdated
Show resolved
Hide resolved
apps/studio/src/modules/sync/components/sync-sites-modal-selector.tsx
Outdated
Show resolved
Hide resolved
I saw this as well before but thought it was resolved. Taking a look now 👀 Update: I see it on the reconnection but not on the initial connection 🤔 |
Related issues
Related to STU-1173
Proposed Changes
This PR explores the speeding up of the deeplink connection from WP.com e.g. connecting a WP.com site and opening a modal.
Testing Instructions
SynctabPublish sitebutton displays in the top right cornerPublish sitebuttonSynctabSynctabPushmodal is openPre-merge Checklist