Skip to content

Add site name to start site dialog error#2577

Merged
sejas merged 8 commits intotrunkfrom
stu-1287-studio-add-site-name-to-start-alert-error
Feb 12, 2026
Merged

Add site name to start site dialog error#2577
sejas merged 8 commits intotrunkfrom
stu-1287-studio-add-site-name-to-start-alert-error

Conversation

@sejas
Copy link
Copy Markdown
Member

@sejas sejas commented Feb 11, 2026

Related issues

Proposed Changes

  • Adds the site name to error dialog title to all 5 errors when a site fails to start, helping users identify which site failed (especially useful during "Start all" or autostart sites on launch)
  • Optimizes auto start sites to run it only on mounting the provider.
  • Adds comprehensive unit tests for all error branches, with a shared setupStartServerError helper to reduce boilerplate
  • Changes startSite parameter to receive the whole site to make easier to get the site name.

Testing Instructions

Automated

  • npm test -- src/hooks/tests/use-site-details.test.tsx

Manual

  • Create a site
  • Add some code to make it fail at start, for example add these constants to wp-config.php
define( 'WP_ALLOW_MULTISITE', true );
define( 'MULTISITE', true );
  • Start that specific site or start all the sites
  • Observe that the alert appears, and confirm the site name is included in the title.
Screenshot 2026-02-11 at 20 02 28

Pre-merge Checklist

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

When a site fails to start, the error dialog now shows which site
failed (e.g., "Failed to start 'My Site'") instead of the generic
"Failed to start the site server". This is especially useful when
starting multiple sites at once via "Start all" or auto-start.
@sejas sejas self-assigned this Feb 11, 2026
@sejas sejas changed the title Stu 1287 studio add site name to start alert error Add site name to start site dialog error Feb 11, 2026
@sejas sejas marked this pull request as ready for review February 12, 2026 10:13
@sejas sejas requested a review from a team February 12, 2026 10:13
@wpmobilebot
Copy link
Copy Markdown
Collaborator

wpmobilebot commented Feb 12, 2026

📊 Performance Test Results

Comparing 3f34c0e vs trunk

site-editor

Metric trunk 3f34c0e Diff Change
load 2694.00 ms 2694.00 ms 0.00 ms ⚪ 0.0%

site-startup

Metric trunk 3f34c0e Diff Change
siteCreation 6074.00 ms 6098.00 ms +24.00 ms ⚪ 0.0%
siteStartup 3930.00 ms 3926.00 ms -4.00 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

Copy link
Copy Markdown
Contributor

@nightnei nightnei left a comment

Choose a reason for hiding this comment

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

Works well and LGTM, only one tiny thing - take a look please at the comment. Anyway approved, as it's not very critical 👍

Screenshot 2026-02-12 at 11 54 26

Comment thread src/hooks/use-site-details.tsx Outdated
Comment on lines +172 to +175
const sitesRef = useRef( sites );
useEffect( () => {
sitesRef.current = sites;
}, [ sites ] );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need this trick with useRef?
Theotretically it should work well if we include sites into useCallback dependencies of startServer function, unless I am missing something.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed on ad7d197
And then optimized array dependencies on 3f34c0e

@nightnei
Copy link
Copy Markdown
Contributor

@sejas I see the changes, so you can ignore my previous comment, reviewing again 🙂

@sejas
Copy link
Copy Markdown
Member Author

sejas commented Feb 12, 2026

@nightnei , great feedback! Consider last changes addressing your suggestion :D.
The useRef was used to avoid an infinite loop producedby the auto start logic. In my last changes I've optimized the auto start useEffect to run only on mount, and I also optimized the startSite logic to grab the site name from the parameter. 👍

Copy link
Copy Markdown
Contributor

@nightnei nightnei left a comment

Choose a reason for hiding this comment

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

@sejas It's unreally awesome commit - having full site will definitely be usefull and simplify the codebase in future changes, the same as in this case 🔥
Looks great and works as expected 👍

@sejas sejas merged commit bcc69e4 into trunk Feb 12, 2026
12 checks passed
@sejas sejas deleted the stu-1287-studio-add-site-name-to-start-alert-error branch February 12, 2026 12:20
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