fix: improve plugin loading performance (#856)

* fix: improve plugin loading performance

* fix: removes empty files attribute warning (#857)

* chore(release): 3.10.5 [skip ci]

* fix: removes warning about empty files

---------

Co-authored-by: svc-cli-bot <svc_cli_bot@salesforce.com>

---------

Co-authored-by: Roman <491247+moltar@users.noreply.github.com>
Co-authored-by: svc-cli-bot <svc_cli_bot@salesforce.com>

src/config/config.ts

@@ -19,6 +19,7 @@ import {getHomeDir, getPlatform} from '../util/os'
 import {compact, isProd} from '../util/util'
 import Cache from './cache'
 import PluginLoader from './plugin-loader'
+import {tsPath} from './ts-node'
 import {Debug, collectUsableIds, getCommandIdPermutations} from './util'
 
 // eslint-disable-next-line new-cap
@@ -511,7 +512,7 @@ export class Config implements IConfig {
         const marker = Performance.mark(OCLIF_MARKER_OWNER, `config.runHook#${p.name}(${hook})`)
         try {
           /* eslint-disable no-await-in-loop */
-          const {filePath, isESM, module} = await loadWithData(p, hook)
+          const {filePath, isESM, module} = await loadWithData(p, await tsPath(p.root, hook, p))
           debug('start', isESM ? '(import)' : '(require)', filePath)
 
           const result = timeout

src/config/plugin.ts

@@ -13,7 +13,7 @@ import {OCLIF_MARKER_OWNER, Performance} from '../performance'
 import {cacheCommand} from '../util/cache-command'
 import {findRoot} from '../util/find-root'
 import {readJson, requireJson} from '../util/fs'
-import {castArray, compact, isProd} from '../util/util'
+import {castArray, compact} from '../util/util'
 import {tsPath} from './ts-node'
 import {Debug, getCommandIdPermutations} from './util'
 
@@ -65,6 +65,7 @@ export class Plugin implements IPlugin {
 
   commandIDs: string[] = []
 
+  // This will be initialized in the _manifest() method, which gets called in the load() method.
   commands!: Command.Loadable[]
 
   commandsDir: string | undefined
@@ -121,14 +122,15 @@ export class Plugin implements IPlugin {
     })
 
     const fetch = async () => {
-      if (!this.commandsDir) return
+      const commandsDir = await this.getCommandsDir()
+      if (!commandsDir) return
       let module
       let isESM: boolean | undefined
       let filePath: string | undefined
       try {
         ;({filePath, isESM, module} = cachedCommandCanBeUsed(this.manifest, id)
           ? await loadWithDataFromManifest(this.manifest.commands[id], this.root)
-          : await loadWithData(this, join(this.commandsDir ?? this.pjson.oclif.commands, ...id.split(':'))))
+          : await loadWithData(this, join(commandsDir ?? this.pjson.oclif.commands, ...id.split(':'))))
         this._debug(isESM ? '(import)' : '(require)', filePath)
       } catch (error: any) {
         if (!opts.must && error.code === 'MODULE_NOT_FOUND') return
@@ -170,7 +172,6 @@ export class Plugin implements IPlugin {
     this.alias = this.options.name ?? this.pjson.name
     const pjsonPath = join(root, 'package.json')
     if (!this.name) throw new CLIError(`no name in ${pjsonPath}`)
-    if (!isProd() && !this.pjson.files) this.warn(`files attribute must be specified in ${pjsonPath}`)
     // eslint-disable-next-line new-cap
     this._debug = Debug(this.name)
     this.version = this.pjson.version
@@ -180,17 +181,7 @@ export class Plugin implements IPlugin {
       this.pjson.oclif = this.pjson['cli-engine'] || {}
     }
 
-    this.commandsDir = await this.getCommandsDir()
-    this.commandIDs = await this.getCommandIDs()
-
-    this.hooks = Object.fromEntries(
-      await Promise.all(
-        Object.entries(this.pjson.oclif.hooks ?? {}).map(async ([k, v]) => [
-          k,
-          await Promise.all(castArray(v).map(async (i) => tsPath(this.root, i, this))),
-        ]),
-      ),
-    )
+    this.hooks = Object.fromEntries(Object.entries(this.pjson.oclif.hooks ?? {}).map(([k, v]) => [k, castArray(v)]))
 
     this.manifest = await this._manifest()
     this.commands = Object.entries(this.manifest.commands)
@@ -236,10 +227,12 @@ export class Plugin implements IPlugin {
       if (manifest) {
         marker?.addDetails({commandCount: Object.keys(manifest.commands).length, fromCache: true})
         marker?.stop()
+        this.commandIDs = Object.keys(manifest.commands)
         return manifest
       }
     }
 
+    this.commandIDs = await this.getCommandIDs()
     const manifest = {
       commands: (
         await Promise.all(
@@ -291,11 +284,12 @@ export class Plugin implements IPlugin {
   }
 
   private async getCommandIDs(): Promise<string[]> {
-    if (!this.commandsDir) return []
+    const commandsDir = await this.getCommandsDir()
+    if (!commandsDir) return []
 
     const marker = Performance.mark(OCLIF_MARKER_OWNER, `plugin.getCommandIDs#${this.name}`, {plugin: this.name})
-    this._debug(`loading IDs from ${this.commandsDir}`)
-    const files = await globby(GLOB_PATTERNS, {cwd: this.commandsDir})
+    this._debug(`loading IDs from ${commandsDir}`)
+    const files = await globby(GLOB_PATTERNS, {cwd: commandsDir})
     const ids = processCommandIds(files)
     this._debug('found commands', ids)
     marker?.addDetails({count: ids.length})
@@ -304,7 +298,8 @@ export class Plugin implements IPlugin {
   }
 
   private async getCommandsDir(): Promise<string | undefined> {
-    return tsPath(this.root, this.pjson.oclif.commands, this)
+    this.commandsDir = await tsPath(this.root, this.pjson.oclif.commands, this)
+    return this.commandsDir
   }
 
   private warn(err: CLIError | Error | string, scope?: string): void {

src/util/find-root.ts

@@ -28,11 +28,13 @@ function* up(from: string) {
 async function findPluginRoot(root: string, name?: string) {
   // If we know the plugin name then we just need to traverse the file
   // system until we find the directory that matches the plugin name.
+  debug.extend(name ?? 'root-plugin')(`Finding root starting at ${root}`)
   if (name) {
-    debug.extend(name)(`Finding root starting at ${root}`)
     for (const next of up(root)) {
-      debug.extend(name)(`Checking ${next}`)
-      if (next.endsWith(basename(name))) return next
+      if (next.endsWith(basename(name))) {
+        debug.extend(name)('Found root based on plugin name!')
+        return next
+      }
     }
   }
 
@@ -49,8 +51,11 @@ async function findPluginRoot(root: string, name?: string) {
 
     try {
       const cur = join(next, 'package.json')
-      debug.extend('root-plugin')(`Checking ${cur}`)
-      if (await safeReadJson<PJSON>(cur)) return dirname(cur)
+      debug.extend(name ?? 'root-plugin')(`Checking ${cur}`)
+      if (await safeReadJson<PJSON>(cur)) {
+        debug.extend(name ?? 'root-plugin')('Found root by traversing up from starting point!')
+        return dirname(cur)
+      }
     } catch {}
   }
 }