[CLI] partial fixes to cwd option, propose that commands import deps lazily

[jtoar]

This PR 1) partially fixes the CLI's cwd option and 2) gives an idea of some of the tweaks I think we should make to the CLI to increase maintainability (for contributors) and start up time (for users). Note that the diff is mostly the result of of fixing the CLI's cwd option.

In sum, the tweaks I'm suggesting are:

• Take advantage of yargs middleware to remove duplicate code. Basically what was suggested in Add getPaths, getConfig and structure to global context of CLI #2793
• Lazily import as much as possible to increase startup time (benchmarks, of course, are TBD). See https://mael.dev/clipanion/docs/tips#lazy-evaluation.

To make things more concrete, I've reworked three of the commands to show what they'd look like: check, info, and lint. The changes are pretty minimal. I'd like for all the commands in the CLI to look more or less like this.

I'll talk about lint a bit more. The changes I want to bring attention to are in the handler function:

redwood/packages/cli/src/commands/lint.js

Lines 27 to 29 in d4b65fc

	export async function handler(argv) {
	const { default: execa } = await import('execa')
	const { fix, files, redwoodProject } = argv

Note that execa may not actually be the best example because it's used often enough in the CLI that we may be better off just importing it at the top-level, like colors, but it'll do for now.

All the dependencies used by the lint command are now imported inside the handler function. There's also a "new" property on argv, redwoodProject—naming and association with @redwoodjs/internal/structure TBD—that has "everything you need to know". I.e., no more calling getPaths.

Ok, so all the dependencies used by the lint command are now imported inside the handler function... this may not seem like a big deal. Currently, execa is imported outside the handler function:

redwood/packages/cli/src/commands/lint.js

Line 3 in 9bea0a3

import execa from 'execa'

It's not that bad for one command but it doesn't scale nicely. The way it is now, if a user runs yarn rw info, a command that only uses one dependency, env-info, execa gets imported even though it's never actually used for anything. And it's way more than execa—it's every dependency every command imports.

The reason they're all imported is commandDir recursively requires every file in the directory it's given (yargs has to build the whole CLI before it can figure out what to execute). The consequence is that start-up time is slow. Moving the imports into the handler only imports the dependencies if the command is actually invoked.

Again, this isn't a full-on rework of the CLI. If I were to really rework the CLI, I'd probably use clipanion.

Note that the missing improvement here is bundling the CLI. Yargs has a short doc on bundling: https://github.com/yargs/yargs/blob/main/docs/bundling.md. And I've tried to follow it, using ESBuild, but am facing some issues. While I'm hoping to resolve those, note that it may only be beneficial to bundle what's included in the entry-point index.js file since bundling lazily-imported dependencies could give us the same slow start-up time we're seeing now. Think of it like the code-splitting the Router does automatically.

Partially fixing the cwd option

Now about the less-interesting change in this PR. The CLI has a cwd option that should let you change the working directory so that you can invoke the CLI in another project.

There's more than one way to do this, setting the RWJS_CWD env var being one of the others. That one works.

One of the reasons it's broken is getPaths is called in many places before getCwdMiddleware has the chance to set the cwd. For example, in yargsDefaults, getPaths is called as soon as this file is imported, which is before getCwdMiddleware runs:

redwood/packages/cli/src/commands/generate.js

Lines 25 to 38 in f3e034c

	export const yargsDefaults = {
	force: {
	alias: 'f',
	default: false,
	description: 'Overwrite existing files',
	type: 'boolean',
	},
	typescript: {
	alias: 'ts',
	default: getProject().isTypeScriptProject,
	description: 'Generate TypeScript files',
	type: 'boolean',
	},
	}

The solution is making constants that use getPaths functions so that they wait till it's actually set. So now, after building, you should be able to test out some of the commands like so...

~/redwood-framework> node packages/cli/dist/index.js --cwd ~/redwood-project info

  System:
    OS: macOS 12.3.1
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.15.0 - ~/.nvm/versions/node/v16.15.0/bin/node
    Yarn: 3.2.1 - ~/.nvm/versions/node/v16.15.0/bin/yarn
  Databases:
    SQLite: 3.37.0 - /usr/bin/sqlite3
  Browsers:
    Chrome: 101.0.4951.64
    Safari: 15.4

Not all commands work yet for reasons TBD.

[netlify]

✅ Deploy Preview for redwoodjs-docs canceled.

Name	Link
🔨 Latest commit	d4b65fc
🔍 Latest deploy log	https://app.netlify.com/sites/redwoodjs-docs/deploys/6289aaf5e412f60008f35744

[dac09]

This is a really great first step Dom! I wonder before we commit to this, if it's worth trying to bundle the cli?

[jtoar]

@dac09 definitely, I did look into it—did you seem my note about it in the OP? Curious what you think about the code-splitting analogy and how bundling would work with lazily imported dependencies:

Note that the missing improvement here is bundling the CLI. Yargs has a short doc on bundling: https://github.com/yargs/yargs/blob/main/docs/bundling.md. And I've tried to follow it, using ESBuild, but am facing some issues. While I'm hoping to resolve those, note that it may only be beneficial to bundle what's included in the entry-point index.js file since bundling lazily-imported dependencies could give us the same slow start-up time we're seeing now. Think of it like the code-splitting the Router does automatically.

[dac09]

I didn't see no - I think you can just use Babel to just do the bundle though - small command change in yarn build:js.

The reason I think we should bundle dependencies like rwjs/internal is so that we can remove them as dependencies from the prod build, which causes for example docker images to be huge.

This is just a theory - but personally I think being able to not have all these other dependencies in the docker image outweighs the minimal increase in startup time we may get e.g. 5 minutes of build time vs 140ms of startup time. But I think... there maybe an answer inbetween - because the problem seems to be that yargs loads everything in the commandDir directory. Perhaps there's a way to get it to lazily load commandDirs?

Again I don't mean to discourage - just pointing out that these two things might be mutually exclusive, which is annoying.

[jtoar]

Closing this as it's superseded by #5537.