Skip to content

Commit 131072d

Browse files
committed
Address review feedback on studio uninstall command
1 parent d074394 commit 131072d

3 files changed

Lines changed: 19 additions & 12 deletions

File tree

‎apps/cli/commands/tests/uninstall.test.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import fs from 'node:fs';
22
import os from 'node:os';
33
import path from 'node:path';
4+
import trash from 'trash';
45
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
56
import { getCliInstallKind } from 'cli/lib/update-notifier';
67
import { runCommand } from '../uninstall';
78

9+
vi.mock( 'trash' );
10+
811
vi.mock( 'cli/lib/update-notifier', async ( importOriginal ) => ( {
912
...( await importOriginal< typeof import('cli/lib/update-notifier') >() ),
1013
getCliInstallKind: vi.fn(),
@@ -57,6 +60,7 @@ describe( 'CLI: studio uninstall', () => {
5760
} );
5861
setPlatform( originalPlatform );
5962
delete process.env.DEV_CONFIG_DIR;
63+
process.exitCode = 0;
6064
fs.rmSync( root, { recursive: true, force: true } );
6165
vi.restoreAllMocks();
6266
} );
@@ -69,13 +73,13 @@ describe( 'CLI: studio uninstall', () => {
6973
expect( fs.existsSync( configDir ) ).toBe( true );
7074
} );
7175

72-
it( 'removes user config too when --purge is passed non-interactively', async () => {
76+
it( 'trashes user config when --purge is passed non-interactively', async () => {
7377
Object.defineProperty( process.stdin, 'isTTY', { value: false, configurable: true } );
7478

7579
await runCommand( true );
7680

7781
expect( fs.existsSync( path.join( installDir, 'cli' ) ) ).toBe( false );
78-
expect( fs.existsSync( configDir ) ).toBe( false );
82+
expect( trash ).toHaveBeenCalledWith( configDir );
7983
} );
8084

8185
it( 'does nothing destructive for non-standalone installs', async () => {
@@ -84,6 +88,7 @@ describe( 'CLI: studio uninstall', () => {
8488
await runCommand( true );
8589

8690
expect( fs.existsSync( path.join( installDir, 'bin' ) ) ).toBe( true );
87-
expect( fs.existsSync( configDir ) ).toBe( true );
91+
expect( trash ).not.toHaveBeenCalled();
92+
expect( process.exitCode ).toBe( 1 );
8893
} );
8994
} );

‎apps/cli/commands/uninstall.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import path from 'node:path';
55
import { confirm } from '@inquirer/prompts';
66
import { getConfigDirectory } from '@studio/common/lib/well-known-paths';
77
import { __, sprintf } from '@wordpress/i18n';
8+
import trash from 'trash';
89
import { Mode, runCommand as stopSites } from 'cli/commands/site/stop';
910
import { getCliInstallKind } from 'cli/lib/update-notifier';
1011
import { StudioArgv } from 'cli/types';
@@ -73,6 +74,7 @@ export async function runCommand( purge: boolean ): Promise< void > {
7374
console.log(
7475
__( 'This Studio CLI was installed via npm. Remove it with: npm rm -g wp-studio' )
7576
);
77+
process.exitCode = 1;
7678
return;
7779
}
7880
if ( installKind !== 'standalone' ) {
@@ -81,6 +83,7 @@ export async function runCommand( purge: boolean ): Promise< void > {
8183
'This Studio CLI is bundled with the Studio desktop app. Uninstall the app to remove it.'
8284
)
8385
);
86+
process.exitCode = 1;
8487
return;
8588
}
8689

@@ -100,7 +103,7 @@ export async function runCommand( purge: boolean ): Promise< void > {
100103
removeConfig = await confirm( {
101104
message: sprintf(
102105
/* translators: %s is the config directory path */
103-
__( 'Permanently delete your Studio config and site list in %s?' ),
106+
__( 'Delete your Studio config in %s?' ),
104107
configDir
105108
),
106109
default: false,
@@ -125,7 +128,9 @@ export async function runCommand( purge: boolean ): Promise< void > {
125128
}
126129

127130
if ( removeConfig ) {
128-
fs.rmSync( configDir, { recursive: true, force: true } );
131+
// Trash rather than hard-delete — the user's sites/config are recoverable if they
132+
// change their mind.
133+
await trash( configDir );
129134
removed.push( configDir );
130135
}
131136

@@ -137,7 +142,7 @@ export async function runCommand( purge: boolean ): Promise< void > {
137142
console.log(
138143
sprintf(
139144
/* translators: %s is the config directory path */
140-
__( '\nYour sites and config were kept in %s. Re-run with --purge to remove them.' ),
145+
__( '\nYour Studio config and sites are still in %s.' ),
141146
configDir
142147
)
143148
);
@@ -159,7 +164,7 @@ export const registerCommand = ( yargs: StudioArgv ) => {
159164
return yargs.option( 'purge', {
160165
type: 'boolean',
161166
alias: 'all',
162-
describe: __( 'Also delete your Studio config and site list (~/.studio)' ),
167+
describe: __( 'Also delete your Studio config (~/.studio)' ),
163168
default: false,
164169
} );
165170
},

‎apps/cli/lib/update-notifier.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,8 @@ async function notifyStandalone( currentVersion: string ): Promise< void > {
194194

195195
let latestVersion: string | null = null;
196196

197-
// Ignore a cached check from a different channel than the running CLI. The cache is
198-
// keyed only by config dir (~/.studio), which is shared across installs — reinstalling
199-
// a different channel (e.g. nightly → production) onto it would otherwise surface a
200-
// stale, cross-channel banner (a `-dev` build offered to a production install) until the
201-
// TTL elapses. On a mismatch we refetch and let the server decide.
197+
// ~/.studio is shared across installs, so a cache left by a different channel (e.g. a
198+
// prior nightly) must not be trusted for this one — refetch on a channel mismatch.
202199
if (
203200
cached &&
204201
now - cached.lastChecked < UPDATE_CHECK_INTERVAL_MS &&

0 commit comments

Comments
 (0)