Repository: oclif/core
Pull request #165 (2021-05-27 03:10:44)

Help subsystem async conversion + test updates.
@amphro 
The help subsystem has been updated for asynchronous usage. This matches the rest of Oclif v2 runtime such that all control paths are now async for running commands, loading a help class, and displaying help. 

Why: To support ESM loading of custom help classes facilitated through ModuleLoader this is an async operation. 

Ancillary Benefit: Custom help classes can now run async operations for complex CLI implementations such as collecting runtime help information from various plugins installed. 

The updates include:
- main.ts now invokes loadHelpClass / and showHelp async.
- getHelpClass renamed to loadHelpClass matching `ModuleLoader.load` naming.
- usage of ModuleLoader to load custom help class; can now load Node modules for custom help classes.
- Help / HelpBase refactored for async.
- existing tests updated / no new tests needed.

**Note**: this is a breaking change with the following implications.  
- Any TS based Oclif projects with custom help classes likely need to update method signatures to add async. 
- Other main Oclif plugins need to be updated for async access of the help subsystem for instance @oclif/dev-cli and its README command will need to be updated. 


Patch:
diff --git a/src/help/index.ts b/src/help/index.ts
index aa861c5..768d68d 100644
--- a/src/help/index.ts
+++ b/src/help/index.ts
@@ -11,7 +11,7 @@ import {stdtermwidth} from '../screen'
 import {compact, sortBy, uniqBy} from '../util'
 import {standardizeIDFromArgv, template} from './util'
 
-export {standardizeIDFromArgv, getHelpClass} from './util'
+export {standardizeIDFromArgv, loadHelpClass} from './util'
 
 const wrap = require('wrap-ansi')
 const {
@@ -42,14 +42,14 @@ export abstract class HelpBase {
    * Show help, used in multi-command CLIs
    * @param args passed into your command, useful for determining which type of help to display
    */
-  public abstract showHelp(argv: string[]): void;
+  public abstract async showHelp(argv: string[]): Promise<void>;
 
   /**
    * Show help for an individual command
    * @param command
    * @param topics
    */
-  public abstract showCommandHelp(command: Interfaces.Command, topics: Interfaces.Topic[]): void;
+  public abstract async showCommandHelp(command: Interfaces.Command, topics: Interfaces.Topic[]): Promise<void>;
 }
 
 export class Help extends HelpBase {
@@ -93,34 +93,34 @@ export class Help extends HelpBase {
     this.render = template(this)
   }
 
-  public showHelp(argv: string[]) {
+  public async showHelp(argv: string[]) {
     if (this.config.topicSeparator !== ':') argv = standardizeIDFromArgv(argv, this.config)
     const subject = getHelpSubject(argv)
     if (!subject) {
       if (this.config.pjson.oclif.default) {
         const rootCmd = this.config.findCommand(this.config.pjson.oclif.default)
-        if (rootCmd) this.showCommandHelp(rootCmd)
+        if (rootCmd) await this.showCommandHelp(rootCmd)
       }
-      this.showRootHelp()
+      await this.showRootHelp()
       return
     }
 
     const command = this.config.findCommand(subject)
     if (command) {
-      this.showCommandHelp(command)
+      await this.showCommandHelp(command)
       return
     }
 
     const topic = this.config.findTopic(subject)
     if (topic)  {
-      this.showTopicHelp(topic)
+      await this.showTopicHelp(topic)
       return
     }
 
     error(`command ${subject} not found`)
   }
 
-  public showCommandHelp(command: Interfaces.Command) {
+  public async showCommandHelp(command: Interfaces.Command) {
     const name = command.id
     const depth = name.split(':').length
 
@@ -143,7 +143,7 @@ export class Help extends HelpBase {
     }
   }
 
-  protected showRootHelp() {
+  protected async showRootHelp() {
     let rootTopics = this.sortedTopics
     let rootCommands = this.sortedCommands
 
@@ -167,7 +167,7 @@ export class Help extends HelpBase {
     }
   }
 
-  protected showTopicHelp(topic: Interfaces.Topic) {
+  protected async showTopicHelp(topic: Interfaces.Topic) {
     const name = topic.name
     const depth = name.split(':').length
 
diff --git a/src/help/util.ts b/src/help/util.ts
index 35fe99b..d5454f9 100644
--- a/src/help/util.ts
+++ b/src/help/util.ts
@@ -2,28 +2,23 @@ import lodashTemplate = require('lodash.template')
 
 import {Config as IConfig, HelpOptions} from '../interfaces'
 import {Help, HelpBase} from '.'
-import * as Config from '../config'
+import ModuleLoader from '../module-loader'
 
 interface HelpBaseDerived {
   new(config: IConfig, opts?: Partial<HelpOptions>): HelpBase;
 }
 
-function extractExport(config: IConfig, classPath: string): HelpBaseDerived {
-  const helpClassPath = Config.tsPath(config.root, classPath)
-  return require(helpClassPath) as HelpBaseDerived
-}
-
 function extractClass(exported: any): HelpBaseDerived {
   return exported && exported.default ? exported.default : exported
 }
 
-export function getHelpClass(config: IConfig): HelpBaseDerived {
+export async function loadHelpClass(config: IConfig): Promise<HelpBaseDerived> {
   const pjson = config.pjson
   const configuredClass = pjson && pjson.oclif && pjson.oclif.helpClass
 
   if (configuredClass) {
     try {
-      const exported = extractExport(config, configuredClass)
+      const exported = await ModuleLoader.load(config, configuredClass) as HelpBaseDerived
       return extractClass(exported) as HelpBaseDerived
     } catch (error) {
       throw new Error(`Unable to load configured help class "${configuredClass}", failed with message:\n${error.message}`)
diff --git a/src/index.ts b/src/index.ts
index 806afe7..70ff35e 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -7,7 +7,7 @@ import {Config, Plugin, tsPath, toCached} from './config'
 import * as Interfaces from './interfaces'
 import * as Errors from './errors'
 import * as Flags from './flags'
-import {HelpBase, Help, getHelpClass} from './help'
+import {HelpBase, Help, loadHelpClass} from './help'
 import {toStandardizedId, toConfiguredId} from './help/util'
 import * as Parser from './parser'
 import {Hook} from './interfaces/hooks'
@@ -18,7 +18,7 @@ export {
   Config,
   Errors,
   Flags,
-  getHelpClass,
+  loadHelpClass,
   Help,
   HelpBase,
   Hook,
diff --git a/src/main.ts b/src/main.ts
index aa7325d..b106a56 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -2,7 +2,7 @@ import {format, inspect} from 'util'
 
 import * as Interfaces from './interfaces'
 import {Config} from './config'
-import {getHelpClass, standardizeIDFromArgv} from './help'
+import {loadHelpClass, standardizeIDFromArgv} from './help'
 
 const log = (message = '', ...args: any[]) => {
   // tslint:disable-next-line strict-type-predicates
@@ -45,9 +45,9 @@ export async function run(argv = process.argv.slice(2), options?: Interfaces.Loa
       if (arg === '--help') return false
       return true
     })
-    const Help = getHelpClass(config)
+    const Help = await loadHelpClass(config)
     const help = new Help(config)
-    help.showHelp(argv)
+    await help.showHelp(argv)
     return
   }
 


Test patch:
diff --git a/src/help/_test-help-class.ts b/src/help/_test-help-class.ts
index c7c7339..00ad605 100644
--- a/src/help/_test-help-class.ts
+++ b/src/help/_test-help-class.ts
@@ -1,4 +1,4 @@
-// `getHelpClass` tests require an oclif project for testing so
+// `loadHelpClass` tests require an oclif project for testing so
 // it is re-using the setup here to be able to do a lookup for
 // this sample help class file in tests, although it is not needed
 // for ../help itself.
@@ -6,11 +6,11 @@
 import {HelpBase} from '.'
 
 export default class extends HelpBase  {
-  showHelp() {
+  async showHelp() {
     console.log('help')
   }
 
-  showCommandHelp() {
+  async showCommandHelp() {
     console.log('command help')
   }
 
diff --git a/test/command/helpers/test-help-in-src/src/test-help-plugin.ts b/test/command/helpers/test-help-in-src/src/test-help-plugin.ts
index d136d74..c807f34 100644
--- a/test/command/helpers/test-help-in-src/src/test-help-plugin.ts
+++ b/test/command/helpers/test-help-in-src/src/test-help-plugin.ts
@@ -10,11 +10,11 @@ export default class extends HelpBase {
     config.showHelpSpy = this.showHelp
   }
 
-  showCommandHelp = spy(() => {
+  showCommandHelp = spy(async () => {
     console.log('hello from test-help-plugin #showCommandHelp')
   })
 
-  showHelp = spy(() => {
+  showHelp = spy(async () => {
     console.log('hello showHelp')
   })
 
diff --git a/test/help/show-help.test.ts b/test/help/show-help.test.ts
index 011ce6c..757f298 100644
--- a/test/help/show-help.test.ts
+++ b/test/help/show-help.test.ts
@@ -13,11 +13,11 @@ g.oclif.columns = 80
 class TestHelp extends Help {
   public config: any;
 
-  public showRootHelp() {
+  public async showRootHelp() {
     return super.showRootHelp()
   }
 
-  public showTopicHelp(topic: Interfaces.Topic) {
+  public async showTopicHelp(topic: Interfaces.Topic) {
     return super.showTopicHelp(topic)
   }
 }
@@ -26,9 +26,9 @@ const test = base
 .register('setupHelp', () => ({
   async run(ctx: { help: TestHelp; stubs: { [k: string]: SinonStub }}) {
     ctx.stubs = {
-      showRootHelp: stub(TestHelp.prototype, 'showRootHelp').returns(),
-      showTopicHelp: stub(TestHelp.prototype, 'showTopicHelp').returns(),
-      showCommandHelp: stub(TestHelp.prototype, 'showCommandHelp').returns(),
+      showRootHelp: stub(TestHelp.prototype, 'showRootHelp').resolves(),
+      showTopicHelp: stub(TestHelp.prototype, 'showTopicHelp').resolves(),
+      showCommandHelp: stub(TestHelp.prototype, 'showCommandHelp').resolves(),
     }
 
     // use devPlugins: true to bring in plugins-plugin with topic commands for testing
@@ -54,7 +54,7 @@ describe('showHelp for root', () => {
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -63,7 +63,7 @@ describe('showHelp for root', () => {
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp([])
+    await help.showHelp([])
   })
   .it('shows a command and topic when the index has siblings', ({stdout, config}) => {
     expect(stdout.trim()).to.equal(`base library for oclif CLIs
@@ -84,7 +84,7 @@ COMMANDS
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -93,7 +93,7 @@ COMMANDS
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp([])
+    await help.showHelp([])
   })
   .it('shows a command only when the topic only contains an index', ({stdout, config}) => {
     expect(stdout.trim()).to.equal(`base library for oclif CLIs
@@ -113,7 +113,7 @@ describe('showHelp for a topic', () => {
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -122,7 +122,7 @@ describe('showHelp for a topic', () => {
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp(['apps'])
+    await help.showHelp(['apps'])
   })
   .it('shows topic help with commands', ({stdout}) => {
     expect(stdout.trim()).to.equal(`This topic is for the apps topic
@@ -138,7 +138,7 @@ COMMANDS
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -147,7 +147,7 @@ COMMANDS
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp(['apps'])
+    await help.showHelp(['apps'])
   })
   .it('shows topic help with topic and commands', ({stdout}) => {
     expect(stdout.trim()).to.equal(`This topic is for the apps topic
@@ -166,7 +166,7 @@ COMMANDS
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -175,7 +175,7 @@ COMMANDS
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp(['apps'])
+    await help.showHelp(['apps'])
   })
   .it('shows topic help with topic and commands and topic command', ({stdout}) => {
     expect(stdout.trim()).to.equal(`This topic is for the apps topic
@@ -195,7 +195,7 @@ COMMANDS
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -204,7 +204,7 @@ COMMANDS
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp(['apps'])
+    await help.showHelp(['apps'])
   })
   .it('ignores other topics and commands', ({stdout}) => {
     expect(stdout.trim()).to.equal(`This topic is for the apps topic
@@ -225,7 +225,7 @@ describe('showHelp for a command', () => {
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -234,7 +234,7 @@ describe('showHelp for a command', () => {
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp(['apps:create'])
+    await help.showHelp(['apps:create'])
   })
   .it('shows help for a leaf (or childless) command', ({stdout}) => {
     expect(stdout.trim()).to.equal(`Create an app
@@ -249,7 +249,7 @@ DESCRIPTION
   test
   .loadConfig()
   .stdout()
-  .do(ctx => {
+  .do(async ctx => {
     const config = ctx.config;
 
     (config as any).plugins = [{
@@ -258,7 +258,7 @@ DESCRIPTION
     }]
 
     const help = new TestHelp(config as any)
-    help.showHelp(['apps'])
+    await help.showHelp(['apps'])
   })
   .it('shows help for a command that has children topics and commands', ({stdout}) => {
     expect(stdout.trim()).to.equal(`List all apps (app index command)
@@ -281,8 +281,8 @@ describe('showHelp routing', () => {
   describe('shows root help', () => {
     test
     .setupHelp()
-    .it('shows root help when no subject is provided', ({help, stubs}) => {
-      help.showHelp([])
+    .it('shows root help when no subject is provided', async ({help, stubs}) => {
+      await help.showHelp([])
       expect(stubs.showRootHelp.called).to.be.true
 
       expect(stubs.showCommandHelp.called).to.be.false
@@ -291,8 +291,8 @@ describe('showHelp routing', () => {
 
     test
     .setupHelp()
-    .it('shows root help when help is the only arg', ({help, stubs}) => {
-      help.showHelp(['help'])
+    .it('shows root help when help is the only arg', async ({help, stubs}) => {
+      await help.showHelp(['help'])
       expect(stubs.showRootHelp.called).to.be.true
 
       expect(stubs.showCommandHelp.called).to.be.false
@@ -304,8 +304,8 @@ describe('showHelp routing', () => {
     test
     .setupHelp()
     .makeTopicsWithoutCommand()
-    .it('shows the topic help when a topic has no matching command', ({help, stubs}) => {
-      help.showHelp(['plugins'])
+    .it('shows the topic help when a topic has no matching command', async ({help, stubs}) => {
+      await help.showHelp(['plugins'])
       expect(stubs.showTopicHelp.called).to.be.true
 
       expect(stubs.showRootHelp.called).to.be.false
@@ -315,8 +315,8 @@ describe('showHelp routing', () => {
     test
     .setupHelp()
     .makeTopicsWithoutCommand()
-    .it('shows the topic help when a topic has no matching command and is preceded by help', ({help, stubs}) => {
-      help.showHelp(['help', 'plugins'])
+    .it('shows the topic help when a topic has no matching command and is preceded by help', async ({help, stubs}) => {
+      await help.showHelp(['help', 'plugins'])
       expect(stubs.showTopicHelp.called).to.be.true
 
       expect(stubs.showRootHelp.called).to.be.false
@@ -327,8 +327,8 @@ describe('showHelp routing', () => {
   describe('shows command help', () => {
     test
     .setupHelp()
-    .it('calls showCommandHelp when a topic that is also a command is called', ({help, stubs}) => {
-      help.showHelp(['plugins'])
+    .it('calls showCommandHelp when a topic that is also a command is called', async ({help, stubs}) => {
+      await help.showHelp(['plugins'])
       expect(stubs.showCommandHelp.called).to.be.true
 
       expect(stubs.showRootHelp.called).to.be.false
@@ -337,8 +337,8 @@ describe('showHelp routing', () => {
 
     test
     .setupHelp()
-    .it('calls showCommandHelp when a command is called', ({help, stubs}) => {
-      help.showHelp(['plugins:install'])
+    .it('calls showCommandHelp when a command is called', async ({help, stubs}) => {
+      await help.showHelp(['plugins:install'])
       expect(stubs.showCommandHelp.called).to.be.true
 
       expect(stubs.showRootHelp.called).to.be.false
@@ -347,8 +347,8 @@ describe('showHelp routing', () => {
 
     test
     .setupHelp()
-    .it('calls showCommandHelp when a command is preceded by the help arg', ({help, stubs}) => {
-      help.showHelp(['help', 'plugins:install'])
+    .it('calls showCommandHelp when a command is preceded by the help arg', async ({help, stubs}) => {
+      await help.showHelp(['help', 'plugins:install'])
       expect(stubs.showCommandHelp.called).to.be.true
 
       expect(stubs.showRootHelp.called).to.be.false
@@ -359,8 +359,8 @@ describe('showHelp routing', () => {
   describe('errors', () => {
     test
     .setupHelp()
-    .it('shows an error when there is a subject but it does not match a topic or command', ({help}) => {
-      expect(() => help.showHelp(['meow'])).to.throw('command meow not found')
+    .it('shows an error when there is a subject but it does not match a topic or command', async ({help}) => {
+      await expect(help.showHelp(['meow'])).to.be.rejectedWith('command meow not found')
     })
   })
 })
diff --git a/test/help/util.test.ts b/test/help/util.test.ts
index 542902c..0454d96 100644
--- a/test/help/util.test.ts
+++ b/test/help/util.test.ts
@@ -2,7 +2,7 @@
 import {resolve} from 'path'
 import {Config, Interfaces} from '../../src'
 import {expect, test} from '@oclif/test'
-import {getHelpClass, standardizeIDFromArgv} from '../../src/help'
+import {loadHelpClass, standardizeIDFromArgv} from '../../src/help'
 import configuredHelpClass from  '../../src/help/_test-help-class'
 
 describe('util', () => {
@@ -12,12 +12,12 @@ describe('util', () => {
     config = await Config.load()
   })
 
-  describe('#getHelpClass', () => {
+  describe('#loadHelpClass', () => {
     test
-    .it('defaults to the class exported', () => {
+    .it('defaults to the class exported', async () => {
       delete config.pjson.oclif.helpClass
 
-      const helpClass = getHelpClass(config)
+      const helpClass = await loadHelpClass(config)
       expect(helpClass).not.be.undefined
       expect(helpClass.prototype.showHelp)
       expect(helpClass.prototype.showCommandHelp)
@@ -25,19 +25,19 @@ describe('util', () => {
     })
 
     test
-    .it('loads help class defined in pjson.oclif.helpClass', () => {
+    .it('loads help class defined in pjson.oclif.helpClass', async () => {
       config.pjson.oclif.helpClass = '../src/help/_test-help-class'
       config.root = resolve(__dirname, '..')
 
       expect(configuredHelpClass).to.not.be.undefined
-      expect(getHelpClass(config)).to.deep.equal(configuredHelpClass)
+      expect(await loadHelpClass(config)).to.deep.equal(configuredHelpClass)
     })
 
     describe('error cases', () => {
       test
-      .it('throws an error when failing to load the help class defined in pjson.oclif.helpClass', () => {
+      .it('throws an error when failing to load the help class defined in pjson.oclif.helpClass', async () => {
         config.pjson.oclif.helpClass = './lib/does-not-exist-help-class'
-        expect(() => getHelpClass(config)).to.throw('Unable to load configured help class "./lib/does-not-exist-help-class", failed with message:')
+        await expect(loadHelpClass(config)).to.be.rejectedWith('Unable to load configured help class "./lib/does-not-exist-help-class", failed with message:')
       })
     })
   })