Skip to content

Fix removing sites when site folder is already removed#2705

Merged
nightnei merged 3 commits intotrunkfrom
fixRemovingSitesWhenTheSiteFolderIsAlreadyRemoved
Mar 6, 2026
Merged

Fix removing sites when site folder is already removed#2705
nightnei merged 3 commits intotrunkfrom
fixRemovingSitesWhenTheSiteFolderIsAlreadyRemoved

Conversation

@nightnei
Copy link
Copy Markdown
Contributor

@nightnei nightnei commented Mar 4, 2026

Related issues

How AI was used in this PR

AI wrote code; I thoroughly reviewed it and did a few more iterations to polish the solution. I considered another solution (p1772624822355439/1772022345.939889-slack-C06DRMD6VPZ), but ultimately decided to go with this mutual proposition from AI and Fredrik (p1772626864886569/1772022345.939889-slack-C06DRMD6VPZ).

Proposed Changes

The issue - if user removed site folder manually, then studio site delete fails and the site remains in the studio site list.
This PR fixes delete command in thsi case.

Testing Instructions

  1. npm run cli:build
  2. Create a site - node apps/cli/dist/cli/main.js site create --path=~/Studio/qqq
  3. Remove site folder - rm -rf ~/Studio/qqq
  4. Remove site via cli - node apps/cli/dist/cli/main.js site delete --path=~/Studio/qqq --files
  5. Previously we got error. Now - Site files already removed is printed and no errors and node apps/cli/dist/cli/main.js site list shows that the site was removed from Studio

Pre-merge Checklist

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

@nightnei nightnei requested review from a team and fredrikekelund March 4, 2026 12:52
@nightnei nightnei self-assigned this Mar 4, 2026
@wpmobilebot
Copy link
Copy Markdown
Collaborator

wpmobilebot commented Mar 4, 2026

📊 Performance Test Results

Comparing eab1d80 vs trunk

site-editor

Metric trunk eab1d80 Diff Change
load 1488.00 ms 1758.00 ms +270.00 ms 🔴 18.1%

site-startup

Metric trunk eab1d80 Diff Change
siteCreation 6090.00 ms 6102.00 ms +12.00 ms ⚪ 0.0%
siteStartup 3942.00 ms 3930.00 ms -12.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

@epeicher epeicher 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 fixing this @nightnei! I have tested it, and I can see that the site is removed from the list when the folder have been removed. Changes also LGTM! :shipit:

Before After
Image Image

Copy link
Copy Markdown
Contributor

@fredrikekelund fredrikekelund left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for tackling this, @nightnei 👍

Comment thread tools/common/lib/fs-utils.ts Outdated
Comment on lines +67 to +68
// Falls back to string comparison when either path doesn't exist (e.g., deleted directories),
// using case-insensitive comparison on macOS/Windows.
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.

Suggested change
// Falls back to string comparison when either path doesn't exist (e.g., deleted directories),
// using case-insensitive comparison on macOS/Windows.
// Falls back to string comparison when either path doesn't exist (e.g., deleted directories).

The second line looks like a leftover

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Forgot to update 🤦‍♂️ Thanks 👍

@nightnei nightnei merged commit 8fc138d into trunk Mar 6, 2026
11 checks passed
@nightnei nightnei deleted the fixRemovingSitesWhenTheSiteFolderIsAlreadyRemoved branch March 6, 2026 13:24
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.

4 participants