refactor: use yargs for the cli

[james-elicx]

Changes

• Replaces our args parsing with yargs (out of the box commands, optionals, help menus, etc.)
• Removes output arg as it was never used.
• Moves the setup part of the CLI to a reusable function called in each command, and commands now just accept args as their param.
• Moves wrangler config retrieval to the setup of the CLI.
• Reads the compiled OpenNext config in non-build commands.
• The flags --env and --config become proper first class citizens.

Replaces #790

Commands Tested

• build
• preview
• deploy
• upload
• populateCache

[changeset-bot]

🦋 Changeset detected

Latest commit: d02a59e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@opennextjs/cloudflare	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@opennextjs/cloudflare@791

commit: 50dd17e

[vicb]

Looks great, thanks for the PR.

Does it still work as expected when there is a --?

I'll take one more look tomorrow

[james-elicx]

Does it still work as expected when there is a --?

Unknown args and args after -- will be added to the array of args to pass to wrangler.

If --env or --config are supplied in the --, they won't be picked up by OpenNext but will be picked up by the Wrangler CLI. Because I'm changing them to be first class citizens, we're no longer doing the searching through argv for them at any position, so it wouldn't extract them if they're after --.

[vicb]

If --env or --config are supplied in the --, they won't be picked up by OpenNext but will be picked up by the Wrangler CLI. Because I'm changing them to be first class citizens, we're no longer doing the searching through argv for them at any position, so it wouldn't extract them if they're after --.

Is it what we want?
IIUC opennextjs-cloudflare deploy -- --env=... would populate i.e. populate the cache and run wrangler deploy for different envs?

[vicb]

Looks good.
I think more comments might help maintaining this code in the future

[james-elicx]

If --env or --config are supplied in the --, they won't be picked up by OpenNext but will be picked up by the Wrangler CLI. Because I'm changing them to be first class citizens, we're no longer doing the searching through argv for them at any position, so it wouldn't extract them if they're after --.

Is it what we want? IIUC opennextjs-cloudflare deploy -- --env=... would populate i.e. populate the cache and run wrangler deploy for different envs?

Yep, that's true, although people were complaining when we wanted people to use -- instead of args being first-class citizens...

What would you think of us throwing an error when people pass --env or --config after the --? I really wanted to make the flag a first-class citizen in the CLI because we use it like one for things like cache population. It's not normal for CLIs to parse flags after the -- and use them internally, which is why I was trying to get away from doing that.

[vicb]

Can we filter out -- here?

[james-elicx]

Anything after -- are considered non-option arguments and put into args._ by yargs

[vicb]

I took a deeper look and have some comments:

First I really like the changes, I fell like the CLI is much nice now, thanks for working on this.

I do have a few comments:

1. --

We use to recommend opennextjs-cloudflare deploy -- --env=prod

IIUC this command will now (maybe before also?) use a different env for populating the cache than for deploying.

2. yargs

I see

Would be nice to ba able to solve this

3. Open Next config / setupCLI

IIUC setupCLI is here to factor some code. It can retrieve or compile the config.

I wouldn't mind more duplication in each of the command to make the code easier to follow/extend

i.e. each command would:

printHeader();

compileConfig() or retieveCompiledConfig();

readWranglerConfig();

Each of this function could be helpers (i.e. printHeader(); could print the helper and show the windows warning, ...)

Maybe one thing we could do then is to move compileConfig() back to build.ts because it really belongs there.

Thinking forward if we add new commands, we might have to add options to setupCLI() and it might makes code less readable.

[james-elicx]

I took a deeper look and have some comments:

First I really like the changes, I fell like the CLI is much nice now, thanks for working on this.

I do have a few comments:

1. --

We use to recommend opennextjs-cloudflare deploy -- --env=prod

IIUC this command will now (maybe before also?) use a different env for populating the cache than for deploying.

So would you like to preserve the old behaviour, or throw an error if it's used after the --?

2. yargs

I see

Would be nice to ba able to solve this

I don't get this - that looks like the dependencies aren't installed.

3. Open Next config / setupCLI

IIUC setupCLI is here to factor some code. It can retrieve or compile the config.

I wouldn't mind more duplication in each of the command to make the code easier to follow/extend

i.e. each command would:

printHeader();

compileConfig() or retieveCompiledConfig();

readWranglerConfig();

Each of this function could be helpers (i.e. printHeader(); could print the helper and show the windows warning, ...)

Maybe one thing we could do then is to move compileConfig() back to build.ts because it really belongs there.

Thinking forward if we add new commands, we might have to add options to setupCLI() and it might makes code less readable.

Sure, I'll change it.

[vicb]

Thank for this PR!

Great code and nice to have some help on the CLI now!

One nit about an error message when the config is not found.