oclif / oclif

CLI for generating, building, and releasing oclif CLIs. Built by Salesforce.
https://oclif.io
MIT License
9.02k stars 318 forks source link

Add commands dynamically #110

Closed milesj closed 2 years ago

milesj commented 6 years ago

Do you want to request a feature or report a bug?

Feature.

I really like Oclif, but ran into a blocker when trying to port an existing CLI app. In my current app, I generate commands dynamically through a for loop, like so: https://github.com/milesj/beemo/blob/master/packages/cli/src/CLI.ts#L44

With Oclif, it looks like each command needs a file, which isn't possible for my app. Is there a way to have a shared command that can be used for all these dynamic entries?

What is the current behavior?

Not supported.

What is the expected behavior?

Supports dynamic commands.

jdx commented 6 years ago

This is not very compatible with the current architecture, I'm afraid. The CLI's manifest must be built into a json file that it uses to load all the commands. It can't read every command as that quickly gets slow even with CLIs that only have a few commands. The manifest is essentially a cache to keep the startup time as low as possible.

What you could do, however, is include a plugin that had an init hook that manually loaded all the commands in. We do something similar with our legacy plugin we use internally to migrate some older CLI code: https://github.com/oclif/plugin-legacy/blob/master/src/hooks/init.ts

This probably isn't that bad of a use-case though, so I think this could be improved or at least documented.

In theory, if you just added commands there on a plugin they should show up. Definitely be careful with the init times though as anything you do in an init will be overhead for every call.

DullReferenceException commented 6 years ago

I've actually got this working, but through using some internal API calls, which is not ideal. In the CLI project, there's an init hook installed, which does something like the following:

const { Command } = require('@oclif/config');

function init({ config }) {
  // These are command constructors. They must have an `id` property as well.
  const commands = getDynamicCommands(); 

  config.plugins.push({
    name: 'Fake plugin',
    hooks: {}, // Not optional
    topics: [], // Not optional
    commands: commands.map(cmd => ({
      ...Command.toCached(cmd),
      load: () => cmd
    }))
  });
}

Having a built-in way to inject commands would be great. In the meantime, I'd love to know if there's any alternative way to do this that doesn't rely on that toCached method, since I'm not sure if that's officially supported for external callers.

jdx commented 6 years ago

This might be good for the realm of a plugin. I could see many different ways that we could make commands dynamic.

We could probably add better primitives for doing it as well. I'd be open to a PR for that. Maybe a .addCommand() function or something?

borekb commented 6 years ago

Just going to note that WP-CLI (CLI framework & set of commands for WordPress) registers commands by add_command and it can take quite a few things: docs. Maybe there's some inspiration to be drawn from there.

jdx commented 6 years ago

I created an example of how this can be done today: https://github.com/jdxcode/oclif-example-dynamic-commands/blob/master/src/hooks/init/addcommand.ts

stephenthedev commented 5 years ago

@jdxcode Your example works like a charm. Any idea how to make these into subcommands? IE:

  1. Dynamic commands are crawl, walk and run
  2. calling them would look like my-cli mycommand crawl my-cli mycommand walk``my-cli mycommand run
stephenthedev commented 5 years ago

@jdxcode Nevermind I figured it out:

  1. Prefix each command with the topic
  2. Make sure to return a topic in the plugin
class DynamicPlugin extends Config.Plugin {
  get hooks() { return {} }
  get topics() {
    return [{name: 'api'}]
  }
  get commandIDs() {
    return apiCommands.map((command: Command) => command.id) // where each id starts with 'api:'
  }

  get commands(): Config.Command.Plugin[] {
    return apiCommands
  }
}
jdx commented 5 years ago

I'm just noting this has come up a few times now and I think it's a good idea to support this in a more well defined, easier way. It's a very hard problem to handle due to performance implications—but we're trying to think of potential solutions.

kgalli commented 4 years ago

@jdxcode I'm also interested in this feature. Is there already a solution next to what you already mentioned?

Any support needed?

srlowe commented 4 years ago

Also would be very interested to know if there were any developments in this direction.

milesj commented 4 years ago

I started building my own CLI as I needed different requirements. But I'll leave this open so others can see it.

ozum commented 4 years ago

Maybe this plugin may help: https://github.com/oclif/plugin-plugins

RasPhilCo commented 4 years ago

Jeff's hook solution seems to be enough for now. This isn't currently in the backlog but I'll get this in mind for future planning.

alexkli commented 4 years ago

FWIW, there is a opts.config.loadPlugins(path, pluginType, pluginOpts) function on the opts argument passed to init hooks, which can be used to load a plugin by just knowing the path:

await config.loadPlugins(path, "user", [{
    type: "user", // plugin type, user seems best, I have also seen "core" and "link" for npm-linked plugins
    name: "@my/plugin", // npm name
    root: "/some/path/node_modules/@my/plugin" // path to plugin code
}]);

I am using it in an init hook here. This is part of a build tool that you'd run on nodejs projects, and this init logic here replaces the current plugin with one from a devDependency in the project to use the correct version of it.

justinedelson commented 3 years ago

@jdxcode was playing with your sample code. It doesn't seem like it results in the dynamic commands being added to the README, i.e. if I take https://github.com/jdxcode/oclif-example-dynamic-commands and run oclif-dev readme (via node_modules/.bin) the resulting README.md doesn't have the dynamically generated command. Is that expected?

0xdevalias commented 2 years ago

It doesn't seem like it results in the dynamic commands being added to the README, i.e. if I take jdxcode/oclif-example-dynamic-commands and run oclif-dev readme (via node_modules/.bin) the resulting README.md doesn't have the dynamically generated command. Is that expected?

@justinedelson tl;dr: By adding static pluginType = "core" the oclif-dev:readme command won't filter out the dynamic command, so will try and generate the docs as desired. We then also need to add static args = [] since Readme.commandUsage doesn't check whether args is defined before trying to access it.

The final diff of additions ends up looking like this:

diff --git a/src/hooks/init/addcommand.ts b/src/hooks/init/addcommand.ts
index bbdfd6c..1dee109 100644
--- a/src/hooks/init/addcommand.ts
+++ b/src/hooks/init/addcommand.ts
@@ -14,6 +14,9 @@ class DynamicPlugin extends Config.Plugin {
   get commands(): Config.Command.Plugin[] {
     const cmd = class extends Command {
       static id = 'mydynamiccommand'
+      static pluginType = "core"
+      static description = "This is a very useful dynamic command description"
+      static args = []
       static load() { return cmd }
       async run() {
         ux.log('running mydynamiccommand')

DeepDive

The readme command is defined in src/commands/readme.ts, and loads the config with:

Config.load({root: cwd, devPlugins: false, userPlugins: false})

It also appears to run the init hook with the following:

await (config as Config).runHook('init', {id: 'readme', argv: this.argv})

If we instrument src/hooks/init/addcommand.ts with some basic console.logs:

Diff ```diff diff --git a/src/hooks/init/addcommand.ts b/src/hooks/init/addcommand.ts index bbdfd6c..2212b52 100644 --- a/src/hooks/init/addcommand.ts +++ b/src/hooks/init/addcommand.ts @@ -8,10 +8,12 @@ class DynamicPlugin extends Config.Plugin { return [] } get commandIDs() { + console.log('DynamicPlugin::get commandIDs') return ['mydynamiccommand'] } get commands(): Config.Command.Plugin[] { + console.log('DynamicPlugin::get commands') const cmd = class extends Command { static id = 'mydynamiccommand' static load() { return cmd } @@ -24,7 +26,10 @@ class DynamicPlugin extends Config.Plugin { } const hook: Config.Hook<'init'> = async function () { + console.log('DynamicPlugin::hook') this.config.plugins.push(new DynamicPlugin(this.config)) + + console.log(this.config.commands) } export default hook ``` ```diff diff --git a/README.md b/README.md index 31fd167..38a1079 100644 --- a/README.md +++ b/README.md @@ -9,3 +9,9 @@ Usage + + +# Usage + +# Commands + ```

Then running oclif-dev readme resulted in the following output, showing that at the very least, the hook is being called:

Details ``` ⇒ ./node_modules/.bin/oclif-dev readme DynamicPlugin::hook DynamicPlugin::get commands [ { id: 'hello', description: 'describe the command here', usage: undefined, pluginName: 'oclif-example-dynamic-commands', pluginType: 'core', hidden: undefined, aliases: [], examples: [ '$ oclif-example-dynamic-commands hello\n' + 'hello world from ./src/hello.ts!\n' ], flags: { help: [Object], name: [Object], force: [Object] }, args: [ [Object] ], load: [Function: load] }, { id: 'help', description: 'display help for <%= config.bin %>', pluginName: '@oclif/plugin-help', pluginType: 'core', aliases: [], flags: { all: [Object] }, args: [ [Object] ], load: [Function: load] }, [class _a extends Command] { id: 'mydynamiccommand' } ] replacing in README.md replacing in README.md ```

It looks like the 'standard' plugins are being shown as objects here, whereas the dynamic plugin is being displayed as [class _a extends Command] { id: 'mydynamiccommand' }

However running in DEBUG mode showed that oclif-dev:readme only found 2 commands after filtering:

⇒  DEBUG=* ./node_modules/.bin/oclif-dev readme
..snip..
  @oclif/config init hook done +27ms
DynamicPlugin::get commands
  oclif-dev:readme commands: 2 +406ms
replacing <!-- usage --> in README.md
  oclif-dev:readme rendering command hello +2ms
  oclif-dev:readme rendering command help +13ms
replacing <!-- commands --> in README.md

Looking at the filtering code, it filters out anything that doesn't have a pluginType of core, which the dynamic command doesn't appear to have:

let commands = config.commands
    .filter(c => !c.hidden && c.pluginType === 'core')
    .map(c => c.id === '.' ? {...c, id: ''} : c)

    this.debug('commands:', commands.map(c => c.id).length)

If we look at the types for Command in oclif/core, it defines pluginType.

In oclif/core's Plugin's load, we see that pluginType is set to this.type, which is defined as:

this.commands = Object
    .entries(this.manifest.commands)
    .map(([id, c]) => ({...c, pluginAlias: this.alias, pluginType: this.type, load: async () => this.findCommand(id, {must: true})}))
    .sort((a, b) => a.id.localeCompare(b.id))
this.type = this.options.type || 'core'

Looking back at the definition of Config.Command.Plugin, we can see that it includes the type field:

/**
   * used to tell the user how the plugin was installed
   * examples: core, link, user, dev
   */
  type: string;

Based on the above deep dive, I tried adding static pluginType = "core" to the dynamic command, which seemed to make the command get detected in oclif-dev:readme, but resulted in a new error:

⇒  ./node_modules/.bin/oclif-dev readme
replacing <!-- usage --> in README.md
TypeError: Cannot read properties of undefined (reading 'filter')
    at defaultUsage (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/dev-cli/lib/commands/readme.js:209:30)
    at Readme.commandUsage (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/dev-cli/lib/commands/readme.js:213:38)
    at ~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/dev-cli/lib/commands/readme.js:108:34
    at Array.map (<anonymous>)
    at Readme.commands (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/dev-cli/lib/commands/readme.js:107:25)
    at Readme.run (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/dev-cli/lib/commands/readme.js:38:123)
    at Readme._run (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/command/lib/command.js:29:20)
    at Config.runCommand (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/config/lib/config.js:151:9)
    at Main.run (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/command/lib/main.js:21:9)
    at Main._run (~/dev/tmp/oclif-example-dynamic-commands/node_modules/@oclif/command/lib/command.js:29:20)

Looking at the code for Readme.commandUsage, we can see that it tries to access command.args without checking if it's defined first:

const defaultUsage = () => {
  return compact([
    id,
    command.args.filter(a => !a.hidden).map(a => arg(a)).join(' '),
  ]).join(' ')
}

After I defined static args = [] in the dynamic command and ran ./node_modules/.bin/oclif-dev readme it finally seemed to generate the README docs:

## `oclif-example-dynamic-commands mydynamiccommand`

\```
USAGE
  $ oclif-example-dynamic-commands mydynamiccommand
\```

We can also add static description = "foo" to make the README docs a bit more useful again.

0xdevalias commented 2 years ago

@mdonnalley It would be nice to include some context/description on why the issue is being closed (if it was completed, then when/where it was completed; if it's 'not planned' then more of an explanation of why it's not going to happen). This current process of 'close a huge pile of issues' with no context is generally considered pretty 'hostile'/'disrespectful' (for lack of a better term) to open source/collaboration/etc.

DanHulton commented 1 year ago

Anyone looking to do dynamic commands in oclif 2, it works a bit differently now, and the examples above do not work. However, I think I have a working example posted in the comments of issue #479 here: https://github.com/oclif/core/issues/479#issuecomment-1522774928

DanHulton commented 1 year ago

Follow-up to my last comment, I packaged up the code for running dynamic commands into a plugin: oclif-dynamic-commands on npm.