CLI: refactor cli from commander to yargs #1232

bcotrim · 2025-04-17T17:43:47Z

Related issues

Fixes STU-355
Fixes STU-379

Proposed Changes

Refactor CLI to use yargs
Adjusted unit tests
Added locale config to CLI

You can check more details about supported locales here

Testing Instructions

Run npm run cli:build
Run npm run start and change your language in Studio
Run node dist/cli/main.js --help and confirm the default yargs strings are translated
Test other cli preview commands to ensure it's working as before

Pre-merge Checklist

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

fredrikekelund

Core functionality works great 👍 I left a couple of small suggestion and one slightly bigger comment about printing a help page by default when no command is provided.

fredrikekelund · 2025-04-22T09:34:39Z

cli/index.ts

+		} );

-	registerPreviewCreateCommand( program );
-	registerPreviewListCommand( previewCommand );
-	registerPreviewDeleteCommand( previewCommand );
-	registerPreviewUpdateCommand( previewCommand );
+	registerPreviewCommands( studioArgv ).strict().help();

-	program.parse( process.argv );
+	await studioArgv.argv;


} ) .demandCommand( 1, __( 'You need to specify a command' ) ) .strict(); registerPreviewCommands( studioArgv );

When I run the CLI script without any arguments, the help page isn't printed right now. Adding a demandCommand call fixes that.

fredrikekelund · 2025-04-22T09:43:24Z

cli/commands/preview/index.ts

Not a requirement, but I'd argue that keeping this logic in cli/index.ts makes it easier to understand the structure of all the commands provided.

fredrikekelund · 2025-04-22T09:45:49Z

cli/commands/preview/create.ts

-		} );
+export const registerCommand = ( yargs: StudioArgv ) => {
+	return yargs.command( {
+		command: 'go [folder]',


Suggested change

command: 'go [folder]',

command: 'go [folder=cwd]',

While we're at it, I think this would add clarity for users.

For some reason this breaks the command. (it always uses the cwd value)

fredrikekelund · 2025-04-22T09:46:07Z

cli/commands/preview/list.ts

-		} );
+export const registerCommand = ( yargs: StudioArgv ) => {
+	return yargs.command( {
+		command: 'list [folder]',


Suggested change

command: 'list [folder]',

command: 'list [folder=cwd]',

fredrikekelund · 2025-04-22T09:46:21Z

cli/commands/preview/update.ts

-		} );
+export const registerCommand = ( yargs: Argv< GlobalOptions > ) => {
+	return yargs.command( {
+		command: 'update [folder]',


Suggested change

command: 'update [folder]',

command: 'update [folder=cwd]',

fredrikekelund · 2025-04-22T09:51:13Z

cli/commands/preview/update.ts

+					description: __( 'The folder to update the preview from' ),
+				} )
+				.option( 'host', {
+					type: 'string',


Suggested change

type: 'string',

alias: 'h',

type: 'string',

I'm not sure what I think the best approach is here, but I wanted to raise the question. -h is what we had before, but I guess you could argue that it could be confused with --help. WDYT, @bcotrim?

I added the alias for H.
I think it makes it less likely to be confused with --help
What do you think?

Good call 👍

fredrikekelund

Changes are looking good 👍 I added a few more comments, with one substantial potential change that I thought of just now. Let's also add yargs (and the types package) as a top-level dependency in package.json.

fredrikekelund · 2025-04-23T06:44:50Z

cli/index.ts

-				} )
-				.hideHelp()
-		);
+		.middleware( async () =>


Suggested change

.middleware( async () =>

.middleware( () =>

Subjective nit, but since we aren't using await inside the function, I think it's clearer which promise is relevant if we remove the async keyword. Alternatively, we could add an await in the function body.

fredrikekelund · 2025-04-23T06:49:05Z

cli/commands/preview/update.ts

+					description: __( 'The folder to update the preview from' ),
+				} )
+				.option( 'host', {
+					type: 'string',


Good call 👍

fredrikekelund · 2025-04-23T06:52:23Z

cli/commands/preview/delete.ts

-		} );
+export const registerCommand = ( yargs: Argv< GlobalOptions > ) => {
+	return yargs.command( {
+		command: 'delete <host>',


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.

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?

Ugh, pulling my hair over this issue 😭

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

fredrikekelund · 2025-04-23T09:20:37Z

cli/index.ts

-	const previewCommand = studioCommand
-		.command( 'preview' )
-		.description( __( 'Manage preview sites' ) );
+	registerCreateCommand( studioArgv );


IMO, we might as well address https://linear.app/a8c/issue/STU-379 in this PR, too, and rename this command to preview create.

fredrikekelund

Looking good and works as expected 👍

fredrikekelund · 2025-04-23T12:40:09Z

package.json

Still missing the yargs dependency here 🙂

We already had yargs in there: https://github.com/Automattic/studio/blob/trunk/package.json#L159

fredrikekelund · 2025-04-23T12:40:45Z

cli/index.ts

-	registerPreviewListCommand( previewCommand, studioCommand );
-	registerPreviewDeleteCommand( previewCommand, studioCommand );
-	registerPreviewUpdateCommand( previewCommand, studioCommand );
+		.command( 'preview', 'Preview commands', ( previewYargs ) => {


Suggested change

.command( 'preview', 'Preview commands', ( previewYargs ) => {

.command( 'preview', __( 'Manage preview sites' ), ( previewYargs ) => {

fredrikekelund · 2025-04-23T12:41:15Z

cli/index.ts

+		.command( 'preview', 'Preview commands', ( previewYargs ) => {
+			registerCreateCommand( previewYargs );
+			registerListCommand( previewYargs );
+			registerDeleteCommand( previewYargs );
+			registerUpdateCommand( previewYargs );
+		} );


I also tried hard getting these commands to display in the default help output, but couldn't figure out a solution…

fredrikekelund · 2025-04-23T12:48:19Z

cli/index.ts

+			registerCreateCommand( previewYargs );
+			registerListCommand( previewYargs );
+			registerDeleteCommand( previewYargs );
+			registerUpdateCommand( previewYargs );


Suggested change

registerUpdateCommand( previewYargs );

registerUpdateCommand( previewYargs );

previewYargs.demandCommand( 1, __( 'You must provide a valid command' ) );

Forgot to add this suggestion. Without this, the help page doesn't display when passing just the preview command

fredrikekelund · 2025-04-23T12:48:51Z

cli/index.ts

+			registerListCommand( previewYargs );
+			registerDeleteCommand( previewYargs );
+			registerUpdateCommand( previewYargs );
+		} );


Suggested change

} );

} )

.demandCommand( 1, __( 'You must provide a valid command' ) )

.strict();

demandCommand needed to display the help page by default and strict seems like good practice.

bcotrim requested a review from a team April 18, 2025 17:32

bcotrim self-assigned this Apr 18, 2025

bcotrim marked this pull request as ready for review April 18, 2025 17:37

fredrikekelund reviewed Apr 22, 2025

View reviewed changes

bcotrim force-pushed the stu-355-switch-commander-for-yargs branch from 0fb1724 to 236d026 Compare April 22, 2025 19:58

bcotrim requested a review from fredrikekelund April 22, 2025 20:06

fredrikekelund reviewed Apr 23, 2025

View reviewed changes

bcotrim added 7 commits April 23, 2025 12:43

refactor cli from commander to yargs

de4e879

fix unit tests

5b2613f

add locale to yargs

cb1c419

add comment in to cli webpack build

d769c02

address pr comments

c98407a

fix command execution

92845a2

update command init and create name

4fda597

bcotrim force-pushed the stu-355-switch-commander-for-yargs branch from 7a38a92 to 4fda597 Compare April 23, 2025 11:44

fix uni test

f65b077

bcotrim requested a review from fredrikekelund April 23, 2025 11:48

fredrikekelund approved these changes Apr 23, 2025

View reviewed changes

fredrikekelund reviewed Apr 23, 2025

View reviewed changes

bcotrim added 3 commits April 23, 2025 13:49

add translation to preview command

6cac39c

rename go to preview create and fix argv consistency

6611443

add helper for preview

d07cd9b

bcotrim merged commit 5f1276a into trunk Apr 23, 2025
8 checks passed

bcotrim deleted the stu-355-switch-commander-for-yargs branch April 23, 2025 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI: refactor cli from commander to yargs #1232

CLI: refactor cli from commander to yargs #1232

bcotrim commented Apr 17, 2025 •

edited

Loading

fredrikekelund left a comment

fredrikekelund Apr 22, 2025

fredrikekelund Apr 22, 2025

fredrikekelund Apr 22, 2025

bcotrim Apr 22, 2025

fredrikekelund Apr 22, 2025

fredrikekelund Apr 22, 2025

fredrikekelund Apr 22, 2025

bcotrim Apr 22, 2025

fredrikekelund Apr 23, 2025

fredrikekelund left a comment

fredrikekelund Apr 23, 2025

fredrikekelund Apr 23, 2025

fredrikekelund Apr 23, 2025

bcotrim Apr 23, 2025

fredrikekelund Apr 23, 2025

fredrikekelund Apr 23, 2025

fredrikekelund left a comment

fredrikekelund Apr 23, 2025

bcotrim Apr 23, 2025

fredrikekelund Apr 23, 2025

fredrikekelund Apr 23, 2025

fredrikekelund Apr 23, 2025

fredrikekelund Apr 23, 2025

	.command( 'preview', 'Preview commands', ( previewYargs ) => {
	.command( 'preview', __( 'Manage preview sites' ), ( previewYargs ) => {

-		} );
+		} )
+		.demandCommand( 1, __( 'You must provide a valid command' ) )
+		.strict();

CLI: refactor cli from commander to yargs #1232

CLI: refactor cli from commander to yargs #1232

Conversation

bcotrim commented Apr 17, 2025 • edited Loading

Related issues

Proposed Changes

Testing Instructions

Pre-merge Checklist

fredrikekelund left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikekelund left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredrikekelund left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcotrim commented Apr 17, 2025 •

edited

Loading