feat: strengthened ModuleLoader & unit tests; now supports mixed ESM …

…/ CJS plugins (#163)

* significantly strengthened ModuleLoader / Oclif can now load mixed CJS / ESM plugins.

* More complete ModuleLoader unit tests.

* Added complete ESM, mixed (CJS w/ ESM), and mixed (ESM w/ CJS) runtime tests.

* test description update

* removed unnecessary yarn.lock files

* removed to-do comments

* removed commented out old code.

src/config/plugin.ts

@@ -156,7 +156,7 @@ export class Plugin implements IPlugin {
     }
     this._debug(`loading IDs from ${this.commandsDir}`)
     const patterns = [
-      '**/*.+(js|ts|tsx)',
+      '**/*.+(js|cjs|mjs|ts|tsx)',
       '!**/*.+(d.ts|test.ts|test.js|spec.ts|spec.js)?(x)',
     ]
     const ids = globby.sync(patterns, {cwd: this.commandsDir})

src/module-loader.ts

@@ -1,5 +1,6 @@
 import * as path from 'path'
 import * as url from 'url'
+import * as fs from 'fs-extra'
 
 import {ModuleLoadError} from './errors'
 import {Config as IConfig} from './interfaces'
@@ -8,6 +9,11 @@ import * as Config from './config'
 
 const getPackageType = require('get-package-type')
 
+/**
+ * Defines file extension resolution when source files do not have an extension.
+ */
+const s_EXTENSIONS: string[] = ['.js', '.mjs', '.cjs']
+
 /**
  * Provides a mechanism to use dynamic import / import() with tsconfig -> module: commonJS as otherwise import() gets
  * transpiled to require().
@@ -38,13 +44,15 @@ export default class ModuleLoader {
    * @returns {Promise<*>} The entire ESM module from dynamic import or CJS module by require.
    */
   static async load(config: IConfig|IPlugin, modulePath: string): Promise<any> {
-    const {isESM, filePath} = ModuleLoader.resolvePath(config, modulePath)
+    let filePath
+    let isESM
     try {
+      ({isESM, filePath} = ModuleLoader.resolvePath(config, modulePath))
       // It is important to await on _importDynamic to catch the error code.
       return isESM ? await _importDynamic(url.pathToFileURL(filePath)) : require(filePath)
     } catch (error) {
       if (error.code === 'MODULE_NOT_FOUND' || error.code === 'ERR_MODULE_NOT_FOUND') {
-        throw new ModuleLoadError(`${isESM ? 'import()' : 'require'} failed to load ${filePath}`)
+        throw new ModuleLoadError(`${isESM ? 'import()' : 'require'} failed to load ${filePath || modulePath}`)
       }
       throw error
     }
@@ -68,13 +76,15 @@ export default class ModuleLoader {
    *                                                                   file path and whether the module is ESM.
    */
   static async loadWithData(config: IConfig|IPlugin, modulePath: string): Promise<{isESM: boolean; module: any; filePath: string}> {
-    const {isESM, filePath} = ModuleLoader.resolvePath(config, modulePath)
+    let filePath
+    let isESM
     try {
+      ({isESM, filePath} = ModuleLoader.resolvePath(config, modulePath))
       const module = isESM ? await _importDynamic(url.pathToFileURL(filePath)) : require(filePath)
       return {isESM, module, filePath}
     } catch (error) {
       if (error.code === 'MODULE_NOT_FOUND' || error.code === 'ERR_MODULE_NOT_FOUND') {
-        throw new ModuleLoadError(`${isESM ? 'import()' : 'require'} failed to load ${filePath}`)
+        throw new ModuleLoadError(`${isESM ? 'import()' : 'require'} failed to load ${filePath || modulePath}`)
       }
       throw error
     }
@@ -106,23 +116,38 @@ export default class ModuleLoader {
 
   /**
    * Resolves a modulePath first by `require.resolve` to allow Node to resolve an actual module. If this fails then
-   * the `modulePath` is resolved from the root of the provided config. `path.resolve` is used for ESM and `tsPath`
-   * for non-ESM paths.
+   * the `modulePath` is resolved from the root of the provided config. `Config.tsPath` is used for initial resolution.
+   * If this file path does not exist then several extensions are tried from `s_EXTENSIONS` in order: '.js', '.mjs',
+   * '.cjs'. After a file path has been selected `isPathModule` is used to determine if the file is an ES Module.
    *
    * @param {IConfig|IPlugin} config - Oclif config or plugin config.
    * @param {string} modulePath - File path to load.
    *
    * @returns {{isESM: boolean, filePath: string}} An object including file path and whether the module is ESM.
    */
   static resolvePath(config: IConfig|IPlugin, modulePath: string): {isESM: boolean; filePath: string} {
-    let isESM = config.pjson.type === 'module'
-    let filePath
+    let isESM: boolean
+    let filePath: string
 
     try {
       filePath = require.resolve(modulePath)
       isESM = ModuleLoader.isPathModule(filePath)
     } catch (error) {
-      filePath = isESM ? path.resolve(path.join(config.root, modulePath)) : Config.tsPath(config.root, modulePath)
+      filePath = Config.tsPath(config.root, modulePath)
+
+      // Try all supported extensions.
+      if (!fs.existsSync(filePath)) {
+        for (const extension of s_EXTENSIONS) {
+          const testPath = `${filePath}${extension}`
+
+          if (fs.existsSync(testPath)) {
+            filePath = testPath
+            break
+          }
+        }
+      }
+
+      isESM = ModuleLoader.isPathModule(filePath)
     }
 
     return {isESM, filePath}

test/config/esm.test.ts

@@ -0,0 +1,52 @@
+import * as path from 'path'
+
+import {Config} from '../../src/config'
+
+import {expect, fancy} from './test'
+
+const root = path.resolve(__dirname, 'fixtures/esm')
+const p = (p: string) => path.join(root, p)
+
+const withConfig = fancy
+.add('config', () => Config.load(root))
+
+describe('esm', () => {
+  withConfig
+  .it('has commandsDir', ({config}) => {
+    expect(config.plugins[0]).to.deep.include({
+      commandsDir: p('src/commands'),
+    })
+  })
+
+  withConfig
+  .stdout()
+  .it('runs esm command and prerun & postrun hooks', async ctx => {
+    await ctx.config.runCommand('foo:bar:baz')
+    expect(ctx.stdout).to.equal('running esm prerun hook\nit works!\nrunning esm postrun hook\n')
+  })
+
+  withConfig
+  .stdout()
+  .it('runs faulty command, only prerun hook triggers', async ctx => {
+    try {
+      await ctx.config.runCommand('foo:bar:fail')
+    } catch {
+      console.log('caught error')
+    }
+    expect(ctx.stdout).to.equal('running esm prerun hook\nit fails!\ncaught error\n')
+  })
+
+  withConfig
+  .stdout()
+  .it('runs esm command, postrun hook captures command result', async ctx => {
+    await ctx.config.runCommand('foo:bar:test-result')
+    expect(ctx.stdout).to.equal('running esm prerun hook\nit works!\nrunning esm postrun hook\nreturned success!\n')
+  })
+
+  withConfig
+  .stdout()
+  .it('runs init hook', async ctx => {
+    await (ctx.config.runHook as any)('init', {id: 'myid', argv: ['foo']})
+    expect(ctx.stdout).to.equal('running esm init hook\n')
+  })
+})

test/config/fixtures/esm/package.json

@@ -0,0 +1,22 @@
+{
+  "name": "esm-plugin",
+  "description": "Module type ESM; All ESM source",
+  "private": true,
+  "type": "module",
+  "files": [],
+  "oclif": {
+    "commands": "./src/commands",
+    "hooks": {
+      "init": "./src/hooks/init",
+      "prerun": [
+        "./src/hooks/prerun"
+      ],
+      "postrun": [
+        "./src/hooks/postrun"
+      ]
+    }
+  },
+  "devDependencies": {
+    "globby": "^8.0.1"
+  }
+}

test/config/fixtures/esm/src/commands/foo/bar/baz.js

@@ -0,0 +1,5 @@
+export class Command {
+  static run() {
+    console.log('it works!')
+  }
+}

test/config/fixtures/esm/src/commands/foo/bar/fail.js

@@ -0,0 +1,6 @@
+export class Command {
+  static run() {
+    console.log('it fails!')
+    throw new Error('random error')
+  }
+}

test/config/fixtures/esm/src/commands/foo/bar/test-result.js

@@ -0,0 +1,6 @@
+export class Command {
+  static run() {
+    console.log('it works!')
+    return 'returned success!'
+  }
+}

test/config/fixtures/esm/src/hooks/init.js

@@ -0,0 +1,3 @@
+export function init() {
+  console.log('running esm init hook')
+}

test/config/fixtures/esm/src/hooks/postrun.js

@@ -0,0 +1,6 @@
+export default function postrun(options) {
+  console.log('running esm postrun hook')
+  if (options.Command.id === 'foo:bar:test-result') {
+    console.log(options.result)
+  }
+}

test/config/fixtures/esm/src/hooks/prerun.js

@@ -0,0 +1,3 @@
+export default function prerun() {
+  console.log('running esm prerun hook')
+}

test/config/fixtures/mixed-cjs-esm/package.json

@@ -0,0 +1,21 @@
+{
+  "name": "mixed-cjs-esm-plugin",
+  "description": "CommonJS module, but mixed w/ ESM by .mjs extension",
+  "private": true,
+  "files": [],
+  "oclif": {
+    "commands": "./src/commands",
+    "hooks": {
+      "init": "./src/hooks/init",
+      "prerun": [
+        "./src/hooks/prerun"
+      ],
+      "postrun": [
+        "./src/hooks/postrun"
+      ]
+    }
+  },
+  "devDependencies": {
+    "globby": "^8.0.1"
+  }
+}

test/config/fixtures/mixed-cjs-esm/src/commands/foo/bar/baz.mjs

@@ -0,0 +1,5 @@
+export class Command {
+  static run() {
+    console.log('it works!')
+  }
+}

test/config/fixtures/mixed-cjs-esm/src/commands/foo/bar/fail.cjs

@@ -0,0 +1,8 @@
+class Command {
+  static run() {
+    console.log('it fails!')
+    throw new Error('random error')
+  }
+}
+
+module.exports = Command

test/config/fixtures/mixed-cjs-esm/src/commands/foo/bar/test-result.js

@@ -0,0 +1,8 @@
+class Command {
+  static run() {
+    console.log('it works!')
+    return 'returned success!'
+  }
+}
+
+module.exports = Command

test/config/fixtures/mixed-cjs-esm/src/hooks/init.js

@@ -0,0 +1,5 @@
+function init() {
+  console.log('running mixed-cjs-esm init hook')
+}
+
+module.exports = init

test/config/fixtures/mixed-cjs-esm/src/hooks/postrun.mjs

@@ -0,0 +1,6 @@
+export default function postrun(options) {
+  console.log('running mixed-cjs-esm postrun hook')
+  if (options.Command.id === 'foo:bar:test-result') {
+    console.log(options.result)
+  }
+}

test/config/fixtures/mixed-cjs-esm/src/hooks/prerun.cjs

@@ -0,0 +1,5 @@
+function prerun() {
+  console.log('running mixed-cjs-esm prerun hook')
+}
+
+module.exports = prerun

test/config/fixtures/mixed-esm-cjs/package.json

@@ -0,0 +1,22 @@
+{
+  "name": "mixed-esm-cjs-plugin",
+  "description": "Module type ESM, but mixed w/ CommonJS by .cjs extension",
+  "private": true,
+  "type": "module",
+  "files": [],
+  "oclif": {
+    "commands": "./src/commands",
+    "hooks": {
+      "init": "./src/hooks/init",
+      "prerun": [
+        "./src/hooks/prerun"
+      ],
+      "postrun": [
+        "./src/hooks/postrun"
+      ]
+    }
+  },
+  "devDependencies": {
+    "globby": "^8.0.1"
+  }
+}

test/config/fixtures/mixed-esm-cjs/src/commands/foo/bar/baz.js

@@ -0,0 +1,5 @@
+export class Command {
+  static run() {
+    console.log('it works!')
+  }
+}

test/config/fixtures/mixed-esm-cjs/src/commands/foo/bar/fail.cjs

@@ -0,0 +1,8 @@
+class Command {
+  static run() {
+    console.log('it fails!')
+    throw new Error('random error')
+  }
+}
+
+module.exports = Command

test/config/fixtures/mixed-esm-cjs/src/commands/foo/bar/test-result.js

@@ -0,0 +1,7 @@
+export default class Command {
+  static run() {
+    console.log('it works!')
+    return 'returned success!'
+  }
+}
+

test/config/fixtures/mixed-esm-cjs/src/hooks/init.js

@@ -0,0 +1,3 @@
+export default function init() {
+  console.log('running mixed-esm-cjs init hook')
+}

test/config/fixtures/mixed-esm-cjs/src/hooks/postrun.js

@@ -0,0 +1,6 @@
+export default function postrun(options) {
+  console.log('running mixed-esm-cjs postrun hook')
+  if (options.Command.id === 'foo:bar:test-result') {
+    console.log(options.result)
+  }
+}

test/config/fixtures/mixed-esm-cjs/src/hooks/prerun.cjs

@@ -0,0 +1,5 @@
+function prerun() {
+  console.log('running mixed-esm-cjs prerun hook')
+}
+
+module.exports = prerun

test/config/mixed-cjs-esm.test.ts

@@ -0,0 +1,52 @@
+import * as path from 'path'
+
+import {Config} from '../../src/config'
+
+import {expect, fancy} from './test'
+
+const root = path.resolve(__dirname, 'fixtures/mixed-cjs-esm')
+const p = (p: string) => path.join(root, p)
+
+const withConfig = fancy
+.add('config', () => Config.load(root))
+
+describe('mixed-cjs-esm', () => {
+  withConfig
+  .it('has commandsDir', ({config}) => {
+    expect(config.plugins[0]).to.deep.include({
+      commandsDir: p('src/commands'),
+    })
+  })
+
+  withConfig
+  .stdout()
+  .it('runs mixed-cjs-esm command and prerun & postrun hooks', async ctx => {
+    await ctx.config.runCommand('foo:bar:baz')
+    expect(ctx.stdout).to.equal('running mixed-cjs-esm prerun hook\nit works!\nrunning mixed-cjs-esm postrun hook\n')
+  })
+
+  withConfig
+  .stdout()
+  .it('runs faulty command, only prerun hook triggers', async ctx => {
+    try {
+      await ctx.config.runCommand('foo:bar:fail')
+    } catch {
+      console.log('caught error')
+    }
+    expect(ctx.stdout).to.equal('running mixed-cjs-esm prerun hook\nit fails!\ncaught error\n')
+  })
+
+  withConfig
+  .stdout()
+  .it('runs mixed-cjs-esm command, postrun hook captures command result', async ctx => {
+    await ctx.config.runCommand('foo:bar:test-result')
+    expect(ctx.stdout).to.equal('running mixed-cjs-esm prerun hook\nit works!\nrunning mixed-cjs-esm postrun hook\nreturned success!\n')
+  })
+
+  withConfig
+  .stdout()
+  .it('runs init hook', async ctx => {
+    await (ctx.config.runHook as any)('init', {id: 'myid', argv: ['foo']})
+    expect(ctx.stdout).to.equal('running mixed-cjs-esm init hook\n')
+  })
+})