Skip to content

Studio: Add site does not duplicate sites#2415

Merged
katinthehatsite merged 4 commits intotrunkfrom
fix/duplicate-sites-added-to-sidebar
Jan 20, 2026
Merged

Studio: Add site does not duplicate sites#2415
katinthehatsite merged 4 commits intotrunkfrom
fix/duplicate-sites-added-to-sidebar

Conversation

@katinthehatsite
Copy link
Contributor

Related issues

Fixes STU-1210

Proposed Changes

This PR ensures that when you add a site from Add site in the sidebar, only one site gets added and there is no duplication.

Testing Instructions

  • Pull the changes from this branch
  • Start Studio with npm start
  • In the sidebar, click on Add site
  • Observe that once the site is added, it only gets added once and you don't see any duplicate sites

Pre-merge Checklist

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

@katinthehatsite katinthehatsite self-assigned this Jan 19, 2026
@katinthehatsite katinthehatsite requested a review from a team January 19, 2026 10:22
Copy link
Contributor

@bcotrim bcotrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @katinthehatsite for looking into this issue.
We already do a similar check in the main process before emitting the site-event event to the UI.
Have you confirmed what is the issue with that check? It would be preferable to keep the logic in the main process and leave the UI as simple as possible, in my opinion.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 19, 2026

📊 Performance Test Results

Comparing bc0c681 vs trunk

site-editor

Metric trunk bc0c681 Diff Change
load 2942.00 ms 2895.00 ms -47.00 ms 🟢 -1.6%

site-startup

Metric trunk bc0c681 Diff Change
siteCreation 7089.00 ms 7076.00 ms -13.00 ms 🟢 -0.2%
siteStartup 3945.00 ms 3946.00 ms +1.00 ms 🔴 0.0%

Results are median values from multiple test runs.

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

@bcotrim
Copy link
Contributor

bcotrim commented Jan 19, 2026

Thanks @katinthehatsite for looking into this issue. We already do a similar check in the main process before emitting the site-event event to the UI. Have you confirmed what is the issue with that check? It would be preferable to keep the logic in the main process and leave the UI as simple as possible, in my opinion.

I forgot to add the link to the existing check:
https://github.com/Automattic/studio/blob/trunk/src/modules/cli/lib/cli-events-subscriber.ts#L42

@katinthehatsite katinthehatsite force-pushed the fix/duplicate-sites-added-to-sidebar branch from 8d5bef1 to 9dfaf8c Compare January 19, 2026 11:15
@katinthehatsite katinthehatsite force-pushed the fix/duplicate-sites-added-to-sidebar branch from 473448a to dc22e10 Compare January 19, 2026 11:24
@katinthehatsite
Copy link
Contributor Author

We already do a similar check in the main process before emitting the site-event event to the UI.
Have you confirmed what is the issue with that check? It would be preferable to keep the logic in the main process and leave the UI as simple as possible, in my opinion.

I see, thanks for adding the link @bcotrim . I looked into the check and it seems that it exited the main process correctly but did not notify the renderer process about the exit properly. I made some changes to src/modules/cli/lib/cli-events-subscriber.ts - let me know what you think!

Copy link
Contributor

@bcotrim bcotrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finding and fixing the issue @katinthehatsite

Nit: If we move sendIpcEventToRenderer inside the handleSiteEvent function, we could structure it with early returns for the "don't send" cases and a single send at the end. In my opinion, it makes the function intention easier to read and understand.

@katinthehatsite
Copy link
Contributor Author

Nit: If we move sendIpcEventToRenderer inside the handleSiteEvent function, we could structure it with early returns for the "don't send" cases and a single send at the end. In my opinion, it makes the function intention easier to read and understand.

Thanks for the suggestion! I did the refactor in bc0c681

@katinthehatsite katinthehatsite merged commit d158e2b into trunk Jan 20, 2026
9 checks passed
@katinthehatsite katinthehatsite deleted the fix/duplicate-sites-added-to-sidebar branch January 20, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants