Skip to content
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

Integrate ESM loading of commands & hooks #160

Merged
merged 6 commits into from
May 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"cli-ux": "^5.1.0",
"debug": "^4.1.1",
"fs-extra": "^9.0.1",
"get-package-type": "^0.1.0",
"globby": "^11.0.1",
"indent-string": "^4.0.0",
"is-wsl": "^2.1.1",
Expand All @@ -30,6 +31,7 @@
"@oclif/plugin-plugins": "^1.7.7",
"@oclif/test": "^1.2.2",
"@types/chai": "^4.1.7",
"@types/chai-as-promised": "^7.1.3",
"@types/clean-stack": "^2.1.1",
"@types/fs-extra": "^9.0.1",
"@types/indent-string": "^4.0.1",
Expand All @@ -45,6 +47,7 @@
"@types/strip-ansi": "^5.2.1",
"@types/wrap-ansi": "^3.0.0",
"chai": "^4.2.0",
"chai-as-promised": "^7.1.1",
"commitlint": "^12.1.1",
"eslint": "^7.3.1",
"eslint-config-oclif": "^3.1.0",
Expand Down
42 changes: 25 additions & 17 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import {Hook} from '../interfaces/hooks'
import {PJSON} from '../interfaces/pjson'
import * as Plugin from './plugin'
import {Topic} from '../interfaces/topic'
import {tsPath} from './ts-node'
import {compact, flatMap, loadJSON, uniq} from './util'
import {isProd} from '../util'
import ModuleLoader from '../module-loader'

// eslint-disable-next-line new-cap
const debug = Debug()
Expand Down Expand Up @@ -202,7 +202,14 @@ export class Config implements IConfig {

async runHook<T>(event: string, opts: T) {
debug('start %s hook', event)
const promises = this.plugins.map(p => {

const search = (m: any): Hook<T> => {
if (typeof m === 'function') return m
if (m.default && typeof m.default === 'function') return m.default
return Object.values(m).find((m: any) => typeof m === 'function') as Hook<T>
}

for (const p of this.plugins) {
const debug = require('debug')([this.bin, p.name, 'hooks', event].join(':'))
const context: Hook.Context = {
config: this,
Expand All @@ -220,26 +227,27 @@ export class Config implements IConfig {
warn(message)
},
}
return Promise.all((p.hooks[event] || [])
.map(async hook => {

const hooks = p.hooks[event] || []

for (const hook of hooks) {
try {
const f = tsPath(p.root, hook)
debug('start', f)
const search = (m: any): Hook<T> => {
if (typeof m === 'function') return m
if (m.default && typeof m.default === 'function') return m.default
return Object.values(m).find((m: any) => typeof m === 'function') as Hook<T>
}

await search(require(f)).call(context, {...opts as any, config: this})
/* eslint-disable no-await-in-loop */
const {isESM, module, filePath} = await ModuleLoader.loadWithData(p, hook)

debug('start', isESM ? '(import)' : '(require)', filePath)

await search(module).call(context, {...opts as any, config: this})
/* eslint-enable no-await-in-loop */

debug('done')
} catch (error) {
if (error && error.oclif && error.oclif.exit !== undefined) throw error
this.warn(error, `runHook ${event}`)
}
}))
})
await Promise.all(promises)
}
}

debug('%s hook done', event)
}

Expand All @@ -250,7 +258,7 @@ export class Config implements IConfig {
await this.runHook('command_not_found', {id, argv})
throw new CLIError(`command ${id} not found`)
}
const command = c.load()
const command = await c.load()
await this.runHook('prerun', {Command: command, argv})
const result = await command.run(argv, this)
await this.runHook('postrun', {Command: command, result: result, argv})
Expand Down
26 changes: 14 additions & 12 deletions src/config/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {Topic} from '../interfaces/topic'
import {tsPath} from './ts-node'
import {compact, exists, flatMap, loadJSON, mapValues} from './util'
import {isProd} from '../util'
import ModuleLoader from '../module-loader'

const _pjson = require('../../package.json')

Expand Down Expand Up @@ -127,7 +128,7 @@ export class Plugin implements IPlugin {

this.manifest = await this._manifest(Boolean(this.options.ignoreManifest), Boolean(this.options.errorOnManifestCreate))
this.commands = Object.entries(this.manifest.commands)
.map(([id, c]) => ({...c, load: () => this.findCommand(id, {must: true})}))
.map(([id, c]) => ({...c, load: async () => this.findCommand(id, {must: true})}))
this.commands.sort((a, b) => {
if (a.id < b.id) return -1
if (a.id > b.id) return 1
Expand Down Expand Up @@ -169,23 +170,24 @@ export class Plugin implements IPlugin {
return ids
}

findCommand(id: string, opts: {must: true}): Command.Class
async findCommand(id: string, opts: {must: true}): Promise<Command.Class>

findCommand(id: string, opts?: {must: boolean}): Command.Class | undefined
async findCommand(id: string, opts?: {must: boolean}): Promise<Command.Class | undefined>

findCommand(id: string, opts: {must?: boolean} = {}): Command.Class | undefined {
const fetch = () => {
async findCommand(id: string, opts: {must?: boolean} = {}): Promise<Command.Class | undefined> {
const fetch = async () => {
if (!this.commandsDir) return
const search = (cmd: any) => {
if (typeof cmd.run === 'function') return cmd
if (cmd.default && cmd.default.run) return cmd.default
return Object.values(cmd).find((cmd: any) => typeof cmd.run === 'function')
}
const p = require.resolve(path.join(this.commandsDir, ...id.split(':')))
this._debug('require', p)
let m
try {
m = require(p)
const p = path.join(this.pjson.oclif.commands as string, ...id.split(':'))
const {isESM, module, filePath} = await ModuleLoader.loadWithData(this, p)
this._debug(isESM ? '(import)' : '(require)', filePath)
m = module
} catch (error) {
if (!opts.must && error.code === 'MODULE_NOT_FOUND') return
throw error
Expand All @@ -196,7 +198,7 @@ export class Plugin implements IPlugin {
cmd.plugin = this
return cmd
}
const cmd = fetch()
const cmd = await fetch()
if (!cmd && opts.must) error(`command ${id} not found`)
return cmd
}
Expand Down Expand Up @@ -228,15 +230,15 @@ export class Plugin implements IPlugin {
return {
version: this.version,
// eslint-disable-next-line array-callback-return
commands: this.commandIDs.map(id => {
commands: (await Promise.all(this.commandIDs.map(async id => {
try {
return [id, toCached(this.findCommand(id, {must: true}), this)]
return [id, toCached(await this.findCommand(id, {must: true}), this)]
} catch (error) {
const scope = 'toCached'
if (Boolean(errorOnManifestCreate) === false) this.warn(error, scope)
else throw this.addErrorScope(error, scope)
}
})
})))
.filter((f): f is [string, Command] => Boolean(f))
.reduce((commands, [id, c]) => {
commands[id] = c
Expand Down
13 changes: 13 additions & 0 deletions src/errors/errors/module-load.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import {CLIError} from './cli'
import {OclifError} from '../../interfaces'

export class ModuleLoadError extends CLIError implements OclifError {
oclif!: { exit: number }

code = 'MODULE_NOT_FOUND'

constructor(message: string) {
super(`[MODULE_NOT_FOUND] ${message}`, {exit: 1})
this.name = 'ModuleLoadError'
}
}
1 change: 1 addition & 0 deletions src/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

export {handle} from './handle'
export {ExitError} from './errors/exit'
export {ModuleLoadError} from './errors/module-load'
export {CLIError} from './errors/cli'
export {Logger} from './logger'
export {config} from './config'
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,6 @@ export namespace Command {
}

export interface Plugin extends Command {
load(): Class;
load(): Promise<Class>;
}
}
4 changes: 2 additions & 2 deletions src/interfaces/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export interface Plugin {
readonly commandIDs: string[];
readonly topics: Topic[];

findCommand(id: string, opts: { must: true }): Command.Class;
findCommand(id: string, opts?: { must: boolean }): Command.Class | undefined;
findCommand(id: string, opts: { must: true }): Promise<Command.Class>;
findCommand(id: string, opts?: { must: boolean }): Promise<Command.Class> | undefined;
load(): Promise<void>;
}
130 changes: 130 additions & 0 deletions src/module-loader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import * as path from 'path'
import * as url from 'url'

import {ModuleLoadError} from './errors'
import {Config as IConfig} from './interfaces'
import {Plugin as IPlugin} from './interfaces'
import * as Config from './config'

const getPackageType = require('get-package-type')

/**
* Provides a mechanism to use dynamic import / import() with tsconfig -> module: commonJS as otherwise import() gets
* transpiled to require().
*/
const _importDynamic = new Function('modulePath', 'return import(modulePath)') // eslint-disable-line no-new-func

/**
* Provides a static class with several utility methods to work with Oclif config / plugin to load ESM or CJS Node
* modules and source files.
*
* @author Michael Leahy <support@typhonjs.io> (https://github.com/typhonrt)
*/
export default class ModuleLoader {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought your original PR had tests? Is this worth unit testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were no added tests. I'm certainly willing to submit tests as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the bare minimum, can you add a new fixture for a esm and a main test for it here.

/**
* Loads and returns a module.
*
* Uses `getPackageType` to determine if `type` is set to 'module. If so loads '.js' files as ESM otherwise uses
* a bare require to load as CJS. Also loads '.mjs' files as ESM.
*
* Uses dynamic import to load ESM source or require for CommonJS.
*
* A unique error, ModuleLoadError, combines both CJS and ESM loader module not found errors into a single error that
* provides a consistent stack trace and info.
*
* @param {IConfig|IPlugin} config - Oclif config or plugin config.
* @param {string} modulePath - NPM module name or file path to load.
*
* @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)
try {
// 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 error
}
}

/**
* Loads a module and returns an object with the module and data about the module.
*
* Uses `getPackageType` to determine if `type` is set to `module`. If so loads '.js' files as ESM otherwise uses
* a bare require to load as CJS. Also loads '.mjs' files as ESM.
*
* Uses dynamic import to load ESM source or require for CommonJS.
*
* A unique error, ModuleLoadError, combines both CJS and ESM loader module not found errors into a single error that
* provides a consistent stack trace and info.
*
* @param {IConfig|IPlugin} config - Oclif config or plugin config.
* @param {string} modulePath - NPM module name or file path to load.
*
* @returns {Promise<{isESM: boolean, module: *, filePath: string}>} An object with the loaded module & data including
* 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)
try {
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 error
}
}

/**
* For `.js` files uses `getPackageType` to determine if `type` is set to `module` in associated `package.json`. If
* the `modulePath` provided ends in `.mjs` it is assumed to be ESM.
*
* @param {string} filePath - File path to test.
*
* @returns {boolean} The modulePath is an ES Module.
* @see https://www.npmjs.com/package/get-package-type
*/
static isPathModule(filePath: string): boolean {
const extension = path.extname(filePath).toLowerCase()

switch (extension) {
case '.js':
return getPackageType.sync(filePath) === 'module'

case '.mjs':
return true

default:
return false
}
}

/**
* 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.
*
* @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

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)
}

return {isESM, filePath}
}
}
4 changes: 4 additions & 0 deletions test/helpers/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,9 @@ const path = require('path')
process.env.TS_NODE_PROJECT = path.resolve('test/tsconfig.json')
process.env.NODE_ENV = 'development'

const chai = require('chai')
const chaiAsPromised = require('chai-as-promised')
chai.use(chaiAsPromised);

global.oclif = global.oclif || {}
global.oclif.columns = 80
1 change: 1 addition & 0 deletions test/module-loader/fixtures/cjs/errors/bad_reference.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bad_reference
3 changes: 3 additions & 0 deletions test/module-loader/fixtures/cjs/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "commonjs"
}
2 changes: 2 additions & 0 deletions test/module-loader/fixtures/cjs/success.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module.exports = ['SUCCESS'];
module.exports.namedExport = 'SUCCESS_NAMED';
1 change: 1 addition & 0 deletions test/module-loader/fixtures/esm/errors/bad_reference.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bad_reference
3 changes: 3 additions & 0 deletions test/module-loader/fixtures/esm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
3 changes: 3 additions & 0 deletions test/module-loader/fixtures/esm/success.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default 'SUCCESS'

export const namedExport = 'SUCCESS_NAMED'
1 change: 1 addition & 0 deletions test/module-loader/fixtures/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
2 changes: 2 additions & 0 deletions test/module-loader/fixtures/success.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module.exports = ['SUCCESS_CJS'];
module.exports.namedExport = 'SUCCESS_NAMED_CJS';
3 changes: 3 additions & 0 deletions test/module-loader/fixtures/success.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default 'SUCCESS_MJS';

export const namedExport = 'SUCCESS_NAMED_MJS';
Loading