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

Added ESM loading support - tests pass - MacOS / Linux / Windows works #144

Closed
wants to merge 1 commit into from
Closed
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",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't love shipping with 0.x releases. I took a look in the repo and I can't tell if there are plans to have a 1.x release. They might be waiting for a node equivalent?

"globby": "^11.0.1",
"indent-string": "^4.0.0",
"is-wsl": "^2.1.1",
Expand All @@ -29,6 +30,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 @@ -44,6 +46,7 @@
"@types/strip-ansi": "^5.2.1",
"@types/wrap-ansi": "^3.0.0",
"chai": "^4.2.0",
"chai-as-promised": "^7.1.1",
"conventional-changelog-cli": "^2.0.21",
"eslint": "^7.3.1",
"eslint-config-oclif": "^3.1.0",
Expand Down
45 changes: 28 additions & 17 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ 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 ModuleLoader from '../module-loader'

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

home!: string

isESM = false

platform!: PlatformTypes

shell!: string
Expand Down Expand Up @@ -110,6 +112,7 @@ export class Config implements IConfig {
this.root = plugin.root
this.pjson = plugin.pjson
this.name = this.pjson.name
this.isESM = plugin.isESM
this.version = this.options.version || this.pjson.version || '0.0.0'
this.channel = this.options.channel || channelFromVersion(this.version)
this.valid = plugin.valid
Expand Down Expand Up @@ -201,7 +204,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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have a @oclif/core version of the docs yet, but we will have to update the reference to hook execution order when we do. https://github.com/oclif/oclif.github.io/blob/docs/docs/hooks.md

const debug = require('debug')([this.bin, p.name, 'hooks', event].join(':'))
const context: Hook.Context = {
config: this,
Expand All @@ -219,26 +229,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.loadGetData(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 @@ -249,7 +260,7 @@ export class Config implements IConfig {
await this.runHook('command_not_found', {id})
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: 15 additions & 11 deletions src/config/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {PJSON} from '../interfaces/pjson'
import {Topic} from '../interfaces/topic'
import {tsPath} from './ts-node'
import {compact, exists, flatMap, loadJSON, mapValues} from './util'
import ModuleLoader from '../module-loader'

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

Expand Down Expand Up @@ -73,6 +74,8 @@ export class Plugin implements IPlugin {

pjson!: PJSON.Plugin

isESM = false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't we completely hide this in ModuleLoader and just call ModuleLoader.load() here instead? The getPackageType call in the ModuleLoader code should resolve the correct pjson.type. To potentially reduce file io, you could also pass in pjson.type to the ModuleLoader.load method maybe?


type!: string

root!: string
Expand Down Expand Up @@ -110,6 +113,7 @@ export class Plugin implements IPlugin {
this._debug('reading %s plugin %s', this.type, root)
this.pjson = await loadJSON(path.join(root, 'package.json')) as any
this.name = this.pjson.name
this.isESM = this.pjson.type === 'module'
const pjsonPath = path.join(root, 'package.json')
if (!this.name) throw new Error(`no name in ${pjsonPath}`)
if (process.env.NODE_DEV === 'development' && !this.pjson.files) this.warn(`files attribute must be specified in ${pjsonPath}`)
Expand All @@ -126,7 +130,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 @@ -168,23 +172,23 @@ 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)
this._debug(this.isESM ? '(import)' : '(require)', p)
let m
try {
m = require(p)
m = this.isESM ? await ModuleLoader.importDynamic(p) : require(p)
} catch (error) {
if (!opts.must && error.code === 'MODULE_NOT_FOUND') return
throw error
Expand All @@ -195,7 +199,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 @@ -227,15 +231,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
11 changes: 3 additions & 8 deletions src/help/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 getHelpClass(config: IConfig): Promise<HelpBaseDerived> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we change this to fetchHelpClass now that it is async?

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}`)
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: 4 additions & 0 deletions src/interfaces/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ export interface Config {
* example: /home/myuser
*/
home: string;
/**
* type from package.json is set to module
*/
isESM: boolean;
/**
* process.platform
*/
Expand Down
8 changes: 6 additions & 2 deletions src/interfaces/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ export interface Plugin {
* parsed with read-pkg
*/
pjson: PJSON.Plugin | PJSON.CLI;
/**
* type from package.json is set to module
*/
isESM: boolean;
/**
* used to tell the user how the plugin was installed
* examples: core, link, user, dev
Expand All @@ -66,7 +70,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>;
}
4 changes: 2 additions & 2 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 getHelpClass(config)
const help = new Help(config)
help.showHelp(argv)
await help.showHelp(argv)
return
}

Expand Down
Loading