-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
feat(cli): Experimental support for plugins #8316
Conversation
looping in @orta for visibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff @Josh-Walker-GM! I ran some preliminary benchmarks and I can't really argue with them. Right now the Redwood commands are virtually the same and the plugin example you provided for testing (thank you 🙏) is much faster, which bodes well.
I just did a preliminary review; most of my comments are nitpicks, but the theme of it is "can we make the code a little more readable"? I'm sure some of it is that it's just a new concept I have to get familiar with. (I'll do another round today, I want to understand the for loop on line 105 a bit more. But figured I'd let you wallow in the nits for now. Sorry. I'll add that none of my comments are blockers)
Last nit 😅 we should probably go with [experimental.cli]
because, well 1) it's experimental 2) we had confusion around that fact already, and 3) I bet if a user saw this table in the TOML right now they'd just assume it's stable.
And re next steps:
We should start splitting up the existing CLI package into smaller packages based on...
This is what I'm most excited for!
packages/cli/src/plugin.js
Outdated
// If we didn't load any commands for this namespace we can skip registering it | ||
// unless we're loading all namespaces for the help output | ||
if (namespacedCommands.length === 0 && !showingRootHelp) { | ||
continue | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Josh-Walker-GM could you help me understand when this condition would be met?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now this occurs when we've filtered out all possible commands. This might happen for example if we could tell from the cache that no plugins could possibly satisfy the command you entered.
This raises the point that the help message is not great in that scenario I give above. We didn't load the plugins into memory because we know none can satisfy the user but as a result we don't get a very good help message. For example if I run yarn rw @joshgmw testing
- which no @joshgmw
plugin provides then I get the following as the last line of the output:
Unknown arguments: @joshgmw, testing
and the default yargs message only contains any other plugins that are registered. Right now that includes all the built-in CLI commands but in the future there will be none or only a few basic ones.
Maybe we should in that case go back and load them all into memory and yargs will then produce the appropriate help message. This would take a little refactor to loop round again and load all the plugins. In the meantime I have added a console warning pointing the user to run yarn rw @namespace --help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll refactor this later tonight when I get some more time so that it just loads all the plugins and provides the standard yargs help.
Made CLI TOML options nested under experimental. Log errors for cache IO related tasks. Add checks to ensure package and version are listed in the toml objects. Print a warning if we did't load any plugins when running a command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! At this point most of the comments I was about to write don't feel helpful for the stage we're at. Thanks for taking my feedback in stride—let's keep going
We should only load the one plugin that is known to match the command via the cache. Load all plugins if we need yargs to show the help.
* Initial plugin loader * Update project-config test snapshot * Refactor to attempt to improve readability. Made CLI TOML options nested under experimental. Log errors for cache IO related tasks. Add checks to ensure package and version are listed in the toml objects. Print a warning if we did't load any plugins when running a command. * Another refactor and a slight update to the logic. We should only load the one plugin that is known to match the command via the cache. Load all plugins if we need yargs to show the help. * chore: correct spelling --------- Co-authored-by: Dominic Saadi <[email protected]>
…te-default * 'main' of github.com:redwoodjs/redwood: chore(deps): update dependency dependency-cruiser to v13.0.2 (redwoodjs#8381) fix(coherence): fix health check path (redwoodjs#8380) fix(deps): update dependency graphql-scalars to v1.22.0 (redwoodjs#8356) fix(deps): update dependency eslint to v8.41.0 (redwoodjs#8376) chore(deps): update dependency glob to v10.2.6 (redwoodjs#8379) fix(deps): update dependency @tremor/react to v2.7.0 (redwoodjs#8361) chore(deps): update dependency @playwright/test to v1.34.0 (redwoodjs#8377) fix(deps): update dependency style-loader to v3.3.3 (redwoodjs#8369) fix(deps): update dependency vite to v4.3.8 (redwoodjs#8371) fix(deps): update dependency css-loader to v6.7.4 (redwoodjs#8365) fix(deps): update dependency mini-css-extract-plugin to v2.7.6 (redwoodjs#8366) Fallback to auth-provider header case (redwoodjs#8368) feat(cli): Experimental support for plugins (redwoodjs#8316) fix(deps): update dependency @clerk/clerk-sdk-node to v4.9.2 (redwoodjs#8364) chore(deps): update dependency octokit to v2.0.16 (redwoodjs#8370) chore(deps): update dependency vite to v4.3.8 (redwoodjs#8355) chore(deps): update dependency @clerk/types to v3.38.1 (redwoodjs#8363)
Problem
We want Redwood's CLI to be more pluggable. It should start to support plugins so we as a framework can write smaller individual CLI plugin packages which should be more performant and maintainable. We also need to support third party plugins so our partners and community can develop their own plugins for the redwood CLI.
Solution
We support arbitrary npm packages that export a field
commands
which is a list of yargs command objects. These plugin packages are defined in theredwood.toml
file with the following structure (the example is a valid third party package if you wish to test):Packages must have a
package
and aversion
and can optionally contain anenabled
flag which defaults to true. Thepackage
is the npm package name andversion
is the npm package version, semver or tag.autoInstall
is a flag which controls whether the CLI should attempt to automatically install any plugins which are defined in the TOML but which are not currently installed as a dev dependency. It currently defaults tofalse
.Plugin packages must be published under a scope such that they obtain an
@author/
prefix to the package name. This is because plugin commands are "namespaced" under the npm scope they were published under. Such that attempting to call commands from the example third party plugin above is achieved by:This namespacing is nessecary to avoid global command name collisions. Without namespacing two plugins which have an
setup
command would conflict and yargs would only dispatch to the firstsetup
which it loaded. With namespacing the task of preventing collisions belongs to plugin authors and they only need to avoid conflicts with any other plugins published under their scope.RedwoodJS has reserved the
''
namespace so that all plugins published under@redwoodjs/
don't need a namespace prefix, retaining their usual form ofyarn rw [command]
Optimisations
Although the goal is to support plugins we must be careful to ensure the CLI is performant - both with disk usage and process memory. A few simple optimisations have been added here:
Namespace-based plugin culling
This means any plugin which are not from the namespace included in the invoked command are not loaded into memory. E.g. we only load plugins from
@joshgmw/
when we runyarn rw @joshgmw [command]
as we know@jtoar
plugins will not be needed.Command-based plugin culling
It would still be non-performant to load all plugins within the same namespace.
@redwoodjs/
will likely publish multiple plugins and we do not wish to load all of them into memory to support a command which will only be performed by one of those packages.To avoid this we currently cache the top level commands a plugin provides and use this cache to opportunistically skip loading them if we are sure they cannot provide the command we wish to call. This caching currently requires the plugin to have been loaded into memory at least once before so the cache entry is present.
Example: Assume
@joshgmw/rw-1
provides generator commands underyarn rw @joshgmw generate [type]
and that@joshgmw/rw-2
provides setup commands underyarn rw @joshgmw setup [type]
. Now assume a user runsyarn rw @joshgmw setup coolThing
. By sampling the first command word,setup
, and using the cache entries we can eliminate the need to load@joshgmw/rw-1
because we know it cannot satisfy a setup command.Those optimisations should ensure only the dependencies of the packages which could provide the command are loaded into memory. Smaller CLI plugin packages which contain the minimal dependencies will therefore reduce memory usage.
To reduce initial disk size we can make use of lazy-installing plugin packages. Plugins are installed as dev dependencies and plugins are known to the CLI via the TOML. This means it's possible for the CLI to know about the existence of commands which have not yet been fully installed. By selectively including only a subset of command packages as dev dependencies at the time we install a users project we can reduce the initial disk usage. The CLI plugin packages will be installed when needed.
Next steps (in following PRs)
i. Usage volume - niche commands can be in a package that isn't installed in a users project at install time
ii. Scope/theme - commands with similar actions should be grouped into the same package
iii. Need - Commands that are immediately essential at dev time, like
dev
,build
orserve
should be grouped into a package@redwoodjs/
cli plugins when they change and add that to the CRWA template. This would be an initial stopgap solution to an online cache entry.cli.plugins
TOML entries. Commands likeadd
,remove
, andupdate
which assist in maintaining the plugin entries.Note: The instrumentation of the CLI using OpenTelemetry is likely to start alongside the process of introducing this plugin based architecture.