Skip to content

CLI: refactor cli from commander to yargs #1232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 23, 2025
Merged

CLI: refactor cli from commander to yargs #1232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
de4e879
refactor cli from commander to yargs
bcotrim Apr 17, 2025
5b2613f
fix unit tests
bcotrim Apr 18, 2025
cb1c419
add locale to yargs
bcotrim Apr 18, 2025
d769c02
add comment in to cli webpack build
bcotrim Apr 18, 2025
c98407a
address pr comments
bcotrim Apr 22, 2025
92845a2
fix command execution
bcotrim Apr 22, 2025
4fda597
update command init and create name
bcotrim Apr 23, 2025
f65b077
fix uni test
bcotrim Apr 23, 2025
6cac39c
add translation to preview command
bcotrim Apr 23, 2025
6611443
rename go to preview create and fix argv consistency
bcotrim Apr 23, 2025
d07cd9b
add helper for preview
bcotrim Apr 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions cli/commands/preview/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ import { createArchive, cleanup } from 'cli/lib/archive';
import { saveSnapshotToAppdata } from 'cli/lib/snapshots';
import { validateSiteFolder, validateSiteSize } from 'cli/lib/validation';
import { Logger, LoggerError } from 'cli/logger';
import { RegisterCommand, OutputFormat } from 'cli/types';
import { OutputFormat, StudioArgv } from 'cli/types';

async function runCommand( siteFolder: string, outputFormat?: OutputFormat ): Promise< void > {
export async function runCommand(
siteFolder: string,
outputFormat?: OutputFormat
): Promise< void > {
const archivePath = path.join(
os.tmpdir(),
`${ path.basename( siteFolder ) }-${ Date.now() }.zip`
Expand Down Expand Up @@ -60,14 +63,21 @@ async function runCommand( siteFolder: string, outputFormat?: OutputFormat ): Pr
}
}

export const registerCommand: RegisterCommand = ( parentCommand, rootCommand = parentCommand ) => {
parentCommand
.command( 'go [folder]' )
.description(
__( 'Create a preview site from the specified folder (defaults to current directory)' )
)
.action( async ( siteFolder: string = process.cwd() ) => {
const outputFormat = rootCommand.opts().outputFormat;
await runCommand( siteFolder, outputFormat );
} );
export const registerCommand = ( yargs: StudioArgv ) => {
return yargs.command( {
command: 'create [folder]',
describe: __(
'Create a preview site from the specified folder (defaults to current directory)'
),
builder: ( yargs ) => {
return yargs.positional( 'folder', {
type: 'string',
default: process.cwd(),
description: __( 'The folder to create a preview from' ),
} );
},
handler: async ( argv ) => {
await runCommand( argv.folder, argv.outputFormat );
},
} );
};
30 changes: 19 additions & 11 deletions cli/commands/preview/delete.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { __ } from '@wordpress/i18n';
import { PreviewCommandLoggerAction as LoggerAction } from 'common/logger-actions';
import { Argv } from 'yargs';
import { deleteSnapshot } from 'cli/lib/api';
import { getAuthToken } from 'cli/lib/appdata';
import { deleteSnapshotFromAppdata, getSnapshotsFromAppdata } from 'cli/lib/snapshots';
import { normalizeHostname } from 'cli/lib/utils';
import { Logger, LoggerError } from 'cli/logger';
import { RegisterCommand, OutputFormat } from 'cli/types';
import { GlobalOptions, OutputFormat } from 'cli/types';

async function runCommand( host: string, outputFormat?: OutputFormat ): Promise< void > {
export async function runCommand( host: string, outputFormat?: OutputFormat ): Promise< void > {
const logger = new Logger< LoggerAction >( outputFormat );

try {
Expand Down Expand Up @@ -39,13 +40,20 @@ async function runCommand( host: string, outputFormat?: OutputFormat ): Promise<
}
}

export const registerCommand: RegisterCommand = ( parentCommand, rootCommand = parentCommand ) => {
parentCommand
.command( 'delete <host>' )
.description( __( 'Delete a preview site' ) )
.action( async ( host: string ) => {
const outputFormat = rootCommand.opts().outputFormat;
const normalizedHost = normalizeHostname( host );
await runCommand( normalizedHost, outputFormat );
} );
export const registerCommand = ( yargs: Argv< GlobalOptions > ) => {
return yargs.command( {
command: 'delete <host>',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
command: 'delete <host>',
command: 'preview delete <host>',

I think it would be better if all commands were printed on the top-level help page. Having to explicitly explore the preview command to understand which commands are available will be cumbersome, especially for the first release (when there are so few commands in total).

This change would also require some changes in src/index.ts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't make it work with "2 names commands". From my testing all the commands would register as "preview", so only the last one to be registered would be available.
I still moved to the logic to the main index, grouped under the preview command.
I explored options for displaying sub-commands in the main help output but couldn't find a clean way to do this without manually writing the help text.
I think that's worse than having the different helps for the sub-commands.
What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ugh, pulling my hair over this issue 😭

I couldn't figure out a solution either, so let's stick with this approach for now.

describe: __( 'Delete a preview site' ),
builder: ( yargs: Argv< GlobalOptions > ) => {
return yargs.positional( 'host', {
type: 'string',
description: __( 'The hostname of the preview site to delete' ),
demandOption: true,
} );
},
handler: async ( argv ) => {
const normalizedHost = normalizeHostname( argv.host );
await runCommand( normalizedHost, argv.outputFormat );
},
} );
};
32 changes: 20 additions & 12 deletions cli/commands/preview/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ import { getSnapshotCliTable } from 'cli/lib/output';
import { getSnapshotsFromAppdata } from 'cli/lib/snapshots';
import { validateSiteFolder } from 'cli/lib/validation';
import { Logger, LoggerError } from 'cli/logger';
import { RegisterCommand, OutputFormat } from 'cli/types';
import { OutputFormat, StudioArgv } from 'cli/types';

async function runCommand( siteFolder: string, outputFormat?: OutputFormat ): Promise< void > {
export async function runCommand(
siteFolder: string,
outputFormat?: OutputFormat
): Promise< void > {
const logger = new Logger< LoggerAction >( outputFormat );

try {
Expand Down Expand Up @@ -40,14 +43,19 @@ async function runCommand( siteFolder: string, outputFormat?: OutputFormat ): Pr
}
}

export const registerCommand: RegisterCommand = ( parentCommand, rootCommand = parentCommand ) => {
parentCommand
.command( 'list [folder]' )
.description(
__( 'List preview sites for the specified folder (defaults to current directory)' )
)
.action( async ( siteFolder: string = process.cwd() ) => {
const outputFormat = rootCommand.opts().outputFormat;
await runCommand( siteFolder, outputFormat );
} );
export const registerCommand = ( yargs: StudioArgv ) => {
return yargs.command( {
command: 'list [folder]',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
command: 'list [folder]',
command: 'list [folder=cwd]',

describe: __( 'List preview sites for the specified folder (defaults to current directory)' ),
builder: ( yargs ) => {
return yargs.positional( 'folder', {
type: 'string',
default: process.cwd(),
description: __( 'The folder to list previews for' ),
} );
},
handler: async ( argv ) => {
await runCommand( argv.folder, argv.outputFormat );
},
} );
};
57 changes: 18 additions & 39 deletions cli/commands/preview/tests/create.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import os from 'os';
import path from 'path';
import { Command } from 'commander';
import { uploadArchive, waitForSiteReady } from 'cli/lib/api';
import { getAuthToken } from 'cli/lib/appdata';
import { createArchive, cleanup } from 'cli/lib/archive';
Expand Down Expand Up @@ -30,7 +29,6 @@ describe( 'Preview Create Command', () => {
file: jest.fn(),
finalize: jest.fn(),
};
let program: Command;
let mockLogger: {
reportStart: jest.Mock;
reportSuccess: jest.Mock;
Expand All @@ -43,7 +41,6 @@ describe( 'Preview Create Command', () => {
jest.spyOn( path, 'basename' ).mockReturnValue( mockBasename );
jest.spyOn( process, 'cwd' ).mockReturnValue( mockFolder );

program = new Command( 'studio' );
mockLogger = {
reportStart: jest.fn(),
reportSuccess: jest.fn(),
Expand All @@ -69,10 +66,8 @@ describe( 'Preview Create Command', () => {
} );

it( 'should complete the preview creation process successfully', async () => {
const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( validateSiteFolder ).toHaveBeenCalledWith( mockFolder );
expect( mockLogger.reportStart.mock.calls[ 0 ] ).toEqual( [ 'validate', 'Validating...' ] );
Expand Down Expand Up @@ -118,10 +113,8 @@ describe( 'Preview Create Command', () => {
} );

it( 'should use current directory when no folder is specified', async () => {
const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go' ] );
const { runCommand } = await import( '../create' );
await runCommand( process.cwd() );

expect( validateSiteFolder ).toHaveBeenCalledWith( process.cwd() );
} );
Expand All @@ -132,10 +125,8 @@ describe( 'Preview Create Command', () => {
throw new LoggerError( errorMessage );
} );

const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( mockLogger.reportError ).toHaveBeenCalled();
expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) );
Expand All @@ -149,10 +140,8 @@ describe( 'Preview Create Command', () => {
throw new LoggerError( errorMessage );
} );

const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( mockLogger.reportError ).toHaveBeenCalled();
expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) );
Expand All @@ -165,10 +154,8 @@ describe( 'Preview Create Command', () => {
throw new LoggerError( errorMessage );
} );

const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( mockLogger.reportError ).toHaveBeenCalled();
expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) );
Expand All @@ -181,10 +168,8 @@ describe( 'Preview Create Command', () => {
throw new LoggerError( errorMessage );
} );

const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( mockLogger.reportError ).toHaveBeenCalled();
expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) );
Expand All @@ -197,10 +182,8 @@ describe( 'Preview Create Command', () => {
throw new LoggerError( errorMessage );
} );

const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( mockLogger.reportError ).toHaveBeenCalled();
expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) );
Expand All @@ -213,10 +196,8 @@ describe( 'Preview Create Command', () => {
throw new LoggerError( errorMessage );
} );

const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( mockLogger.reportError ).toHaveBeenCalled();
expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) );
Expand All @@ -227,10 +208,8 @@ describe( 'Preview Create Command', () => {
throw new LoggerError( 'Upload failed' );
} );

const { registerCommand } = await import( '../create' );
registerCommand( program );

await program.parseAsync( [ 'node', 'studio', 'go', mockFolder ] );
const { runCommand } = await import( '../create' );
await runCommand( mockFolder );

expect( cleanup ).toHaveBeenCalledWith( mockArchivePath );
} );
Expand Down
Loading
Loading