Skip to content

Commit

Permalink
feat: add strategies for command discovery (#945)
Browse files Browse the repository at this point in the history
* feat: add import strategy for command discovery

* refactor: clean up implementation

* test: add tests for explicit command strategy

* chore: rename search to pattern

* fix: explicit aliases

* chore: code review

* feat: add new strategy for single command CLIs

* fix: add missing files

* chore: rename single command cli symbol

* refactor: make fetch more clear

* ci: retry external integration tests

* chore: use CommandCache type

Co-authored-by: Shane McLaughlin <shane.mclaughlin@salesforce.com>

* chore(release): 3.19.2-qa.0 [skip ci]

* feat: bundle friendly

* chore(release): 3.19.2-dev.0 [skip ci]

* fix: update perf marker

* chore(release): 3.19.3-dev.0 [skip ci]

* test: add esbuild plugin to interop tests

* test: rename esm-cjs to interop

* test: clean up

* chore(release): 3.19.5-dev.0 [skip ci]

* chore: regen yarn.lock

* chore: code review

* chore: proper logic for missing hook

* fix: only register command ids if they have valid cmd

* chore: code suggestion

* chore(release): 3.20.1-dev.0 [skip ci]

* test: refine esbuild interop test

* ci: skip windows plugin-plugins integration tests

* test: more plugin-test-esbuild tests

* feat: allow pjson to be provided to Config

* ci: esbuild integration test

* chore: clean up

* fix: make dir optional for execute

* chore(release): 3.20.1-dev.1 [skip ci]

---------

Co-authored-by: Shane McLaughlin <shane.mclaughlin@salesforce.com>
Co-authored-by: svc-cli-bot <Svc_cli_bot@salesforce.com>
  • Loading branch information
3 people committed Mar 4, 2024
1 parent bda047d commit eaf5a86
Show file tree
Hide file tree
Showing 37 changed files with 973 additions and 1,230 deletions.
16 changes: 14 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ jobs:
matrix:
os: [ubuntu-latest, windows-latest]
node_version: [lts/*, latest]
test: [esm, cjs, precore, coreV1, coreV2]
test: [esm, cjs, precore, coreV1, coreV2, esbuild]
dev_runtime: [default, bun, tsx]
exclude:
- os: windows-latest
Expand Down Expand Up @@ -160,6 +160,7 @@ jobs:
- oclif/plugin-which
- oclif/plugin-warn-if-update-available
- oclif/plugin-version
- oclif/plugin-test-esbuild
uses: ./.github/workflows/external-test.yml
with:
repo: ${{ matrix.repo }}
Expand All @@ -170,7 +171,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
os: [ubuntu-latest]
uses: ./.github/workflows/external-test.yml
with:
repo: oclif/plugin-plugins
Expand All @@ -189,3 +190,14 @@ jobs:
repo: oclif/plugin-update
os: ${{ matrix.os }}
command: yarn test:integration:sf --retries 3
plugin-esbuild-integration:
needs: linux-unit-tests
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
uses: ./.github/workflows/external-test.yml
with:
repo: oclif/plugin-test-esbuild-single
os: ${{ matrix.os }}
command: yarn test
1,405 changes: 292 additions & 1,113 deletions CHANGELOG.md

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@oclif/core",
"description": "base library for oclif CLIs",
"version": "3.20.0",
"version": "3.20.1-dev.1",
"author": "Salesforce",
"bugs": "https://github.com/oclif/core/issues",
"dependencies": {
Expand Down Expand Up @@ -114,7 +114,6 @@
"access": "public"
},
"scripts": {
"build:dev": "shx rm -rf lib && tsc --sourceMap",
"build": "shx rm -rf lib && tsc",
"commitlint": "commitlint",
"compile": "tsc",
Expand All @@ -129,7 +128,6 @@
"test:integration": "mocha --forbid-only \"test/**/*.integration.ts\" --parallel --timeout 1200000",
"test:interoperability": "cross-env DEBUG=integration:* ts-node test/integration/interop.ts",
"test:perf": "ts-node test/perf/parser.perf.ts",
"test:dev": "nyc mocha \"test/**/*.test.ts\"",
"test": "nyc mocha --forbid-only \"test/**/*.test.ts\""
},
"types": "lib/index.d.ts"
Expand Down
28 changes: 28 additions & 0 deletions src/cache.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import {readFileSync} from 'node:fs'
import {join} from 'node:path'

import {PJSON, Plugin} from './interfaces'

type OclifCoreInfo = {name: string; version: string}

type CacheContents = {
rootPlugin: Plugin
exitCodes: PJSON.Plugin['oclif']['exitCodes']
'@oclif/core': OclifCoreInfo
}

type ValueOf<T> = T[keyof T]
Expand All @@ -12,6 +18,11 @@ type ValueOf<T> = T[keyof T]
*/
export default class Cache extends Map<keyof CacheContents, ValueOf<CacheContents>> {
static instance: Cache
public constructor() {
super()
this.set('@oclif/core', this.getOclifCoreMeta())
}

static getInstance(): Cache {
if (!Cache.instance) {
Cache.instance = new Cache()
Expand All @@ -20,9 +31,26 @@ export default class Cache extends Map<keyof CacheContents, ValueOf<CacheContent
return Cache.instance
}

public get(key: '@oclif/core'): OclifCoreInfo
public get(key: 'rootPlugin'): Plugin | undefined
public get(key: 'exitCodes'): PJSON.Plugin['oclif']['exitCodes'] | undefined
public get(key: keyof CacheContents): ValueOf<CacheContents> | undefined {
return super.get(key)
}

private getOclifCoreMeta(): OclifCoreInfo {
try {
// eslint-disable-next-line node/no-extraneous-require
return {name: '@oclif/core', version: require('@oclif/core/package.json').version}
} catch {
try {
return {
name: '@oclif/core',
version: JSON.parse(readFileSync(join(__dirname, '..', 'package.json'), 'utf8')),
}
} catch {
return {name: '@oclif/core', version: 'unknown'}
}
}
}
}
6 changes: 3 additions & 3 deletions src/cli-ux/config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {PJSON} from '../interfaces/pjson'
import {requireJson} from '../util/fs'
import Cache from '../cache'
import {ActionBase} from './action/base'
import simple from './action/simple'
import spinner from './action/spinner'
Expand Down Expand Up @@ -51,7 +50,8 @@ export class Config {
}

function fetch() {
const major = requireJson<PJSON>(__dirname, '..', '..', 'package.json').version.split('.')[0]
const core = Cache.getInstance().get('@oclif/core')
const major = core?.version.split('.')[0] || 'unknown'
if (globals[major]) return globals[major]
globals[major] = new Config()
return globals[major]
Expand Down
5 changes: 2 additions & 3 deletions src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import chalk from 'chalk'
import {fileURLToPath} from 'node:url'
import {inspect} from 'node:util'

import Cache from './cache'
import {ux} from './cli-ux'
import {Config} from './config'
import * as Errors from './errors'
import {PrettyPrintableError} from './errors'
import {formatCommandDeprecationWarning, formatFlagDeprecationWarning, normalizeArgv} from './help/util'
import {PJSON} from './interfaces'
import {LoadOptions} from './interfaces/config'
import {CommandError} from './interfaces/errors'
import {
Expand All @@ -27,11 +27,10 @@ import {
import {Plugin} from './interfaces/plugin'
import * as Parser from './parser'
import {aggregateFlags} from './util/aggregate-flags'
import {requireJson} from './util/fs'
import {toConfiguredId} from './util/ids'
import {uniq} from './util/util'

const pjson = requireJson<PJSON>(__dirname, '..', 'package.json')
const pjson = Cache.getInstance().get('@oclif/core')

/**
* swallows stdout epipe errors
Expand Down
30 changes: 22 additions & 8 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {Theme} from '../interfaces/theme'
import {loadWithData} from '../module-loader'
import {OCLIF_MARKER_OWNER, Performance} from '../performance'
import {settings} from '../settings'
import {requireJson, safeReadJson} from '../util/fs'
import {safeReadJson} from '../util/fs'
import {getHomeDir, getPlatform} from '../util/os'
import {compact, isProd} from '../util/util'
import PluginLoader from './plugin-loader'
Expand All @@ -27,7 +27,7 @@ import {Debug, collectUsableIds, getCommandIdPermutations} from './util'
// eslint-disable-next-line new-cap
const debug = Debug()

const _pjson = requireJson<PJSON>(__dirname, '..', '..', 'package.json')
const _pjson = Cache.getInstance().get('@oclif/core')
const BASE = `${_pjson.name}@${_pjson.version}`

function channelFromVersion(version: string) {
Expand Down Expand Up @@ -85,6 +85,7 @@ export class Config implements IConfig {
public errlog!: string
public flexibleTaxonomy!: boolean
public home!: string
public isSingleCommandCLI = false
public name!: string
public npmRegistry?: string
public nsisCustomization?: string
Expand Down Expand Up @@ -286,7 +287,7 @@ export class Config implements IConfig {
(settings.performanceEnabled === undefined ? this.options.enablePerf : settings.performanceEnabled) ?? false
const marker = Performance.mark(OCLIF_MARKER_OWNER, 'config.load')
this.pluginLoader = new PluginLoader({plugins: this.options.plugins, root: this.options.root})
this.rootPlugin = await this.pluginLoader.loadRoot()
this.rootPlugin = await this.pluginLoader.loadRoot({pjson: this.options.pjson})

// Cache the root plugin so that we can reference it later when determining if
// we should skip ts-node registration for an ESM plugin.
Expand Down Expand Up @@ -362,6 +363,12 @@ export class Config implements IConfig {
...(s3.templates && s3.templates.vanilla),
},
}
this.isSingleCommandCLI = Boolean(
this.pjson.oclif.default ||
(typeof this.pjson.oclif.commands !== 'string' &&
this.pjson.oclif.commands?.strategy === 'single' &&
this.pjson.oclif.commands?.target),
)

await this.loadPluginsAndCommands()

Expand Down Expand Up @@ -544,15 +551,22 @@ export class Config implements IConfig {
const hooks = p.hooks[event] || []

for (const hook of hooks) {
const marker = Performance.mark(OCLIF_MARKER_OWNER, `config.runHook#${p.name}(${hook})`)
const marker = Performance.mark(OCLIF_MARKER_OWNER, `config.runHook#${p.name}(${hook.target})`)
try {
/* eslint-disable no-await-in-loop */
const {filePath, isESM, module} = await loadWithData(p, await tsPath(p.root, hook, p))
const {filePath, isESM, module} = await loadWithData(p, await tsPath(p.root, hook.target, p))
debug('start', isESM ? '(import)' : '(require)', filePath)
// If no hook is found using the identifier, then we should `search` for the hook but only if the hook identifier is 'default'
// A named identifier (e.g. MY_HOOK) that isn't found indicates that the hook isn't implemented in the plugin.
const hookFn = module[hook.identifier] ?? (hook.identifier === 'default' ? search(module) : undefined)
if (!hookFn) {
debug('No hook found for hook definition:', hook)
continue
}

const result = timeout
? await withTimeout(timeout, search(module).call(context, {...(opts as any), config: this, context}))
: await search(module).call(context, {...(opts as any), config: this, context})
? await withTimeout(timeout, hookFn.call(context, {...(opts as any), config: this, context}))
: await hookFn.call(context, {...(opts as any), config: this, context})
final.successes.push({plugin: p, result})

if (p.name === '@oclif/plugin-legacy' && event === 'init') {
Expand All @@ -578,7 +592,7 @@ export class Config implements IConfig {

marker?.addDetails({
event,
hook,
hook: hook.target,
plugin: p.name,
})
marker?.stop()
Expand Down
4 changes: 2 additions & 2 deletions src/config/plugin-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ export default class PluginLoader {
return {errors: this.errors, plugins: this.plugins}
}

public async loadRoot(): Promise<IPlugin> {
public async loadRoot({pjson}: {pjson?: PJSON.Plugin}): Promise<IPlugin> {
let rootPlugin: IPlugin
if (this.pluginsProvided) {
const plugins = [...this.plugins.values()]
rootPlugin = plugins.find((p) => p.root === this.options.root) ?? plugins[0]
} else {
const marker = Performance.mark(OCLIF_MARKER_OWNER, 'plugin.load#root')
rootPlugin = new Plugin.Plugin({isRoot: true, root: this.options.root})
rootPlugin = new Plugin.Plugin({isRoot: true, pjson, root: this.options.root})
await rootPlugin.load()
marker?.addDetails({
commandCount: rootPlugin.commands.length,
Expand Down

0 comments on commit eaf5a86

Please sign in to comment.