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

feat: various fixes #824

Merged
merged 8 commits into from
Oct 13, 2023
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
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"clean-stack": "^3.0.1",
"cli-progress": "^3.12.0",
"debug": "^4.3.4",
"ejs": "^3.1.8",
"ejs": "^3.1.9",
"get-package-type": "^0.1.0",
"globby": "^11.1.0",
"hyperlinker": "^1.0.0",
Expand All @@ -39,11 +39,11 @@
"@oclif/test": "^3.0.1",
"@types/ansi-styles": "^3.2.1",
"@types/benchmark": "^2.1.2",
"@types/chai": "^4.3.4",
"@types/chai": "^4.3.8",
"@types/chai-as-promised": "^7.1.5",
"@types/clean-stack": "^2.1.1",
"@types/cli-progress": "^3.11.0",
"@types/ejs": "^3.1.2",
"@types/ejs": "^3.1.3",
"@types/indent-string": "^4.0.1",
"@types/js-yaml": "^3.12.7",
"@types/mocha": "^10.0.2",
Expand All @@ -55,7 +55,7 @@
"@types/wordwrap": "^1.0.1",
"@types/wrap-ansi": "^3.0.0",
"benchmark": "^2.1.4",
"chai": "^4.3.7",
"chai": "^4.3.10",
"chai-as-promised": "^7.1.1",
"commitlint": "^17.7.2",
"cross-env": "^7.0.3",
Expand Down
2 changes: 1 addition & 1 deletion src/cli-ux/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ interface IPromptConfig {
function normal(options: IPromptConfig, retries = 100): Promise<string> {
if (retries < 0) throw new Error('no input')
return new Promise((resolve, reject) => {
let timer: NodeJS.Timer
let timer: NodeJS.Timeout
if (options.timeout) {
timer = setTimeout(() => {
process.stdin.pause()
Expand Down
26 changes: 26 additions & 0 deletions src/config/cache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import {Plugin} from '../interfaces'

type CacheContents = {
rootPlugin: Plugin
}

type ValueOf<T> = T[keyof T]

/**
* A simple cache for storing values that need to be accessed globally.
*/
export default class Cache extends Map<keyof CacheContents, ValueOf<CacheContents>> {
static instance: Cache
static getInstance(): Cache {
if (!Cache.instance) {
Cache.instance = new Cache()
}

return Cache.instance
}

public get(key: 'rootPlugin'): Plugin | undefined
public get(key: keyof CacheContents): ValueOf<CacheContents> | undefined {
return super.get(key)
}
}
28 changes: 17 additions & 11 deletions src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {settings} from '../settings'
import {requireJson} from '../util/fs'
import {getHomeDir, getPlatform} from '../util/os'
import {compact, isProd} from '../util/util'
import Cache from './cache'
import PluginLoader from './plugin-loader'
import {Debug, collectUsableIds, getCommandIdPermutations} from './util'

Expand Down Expand Up @@ -104,13 +105,14 @@ export class Config implements IConfig {

private _commands = new Map<string, Command.Loadable>()

private static _rootPlugin: IPlugin

private _topics = new Map<string, Topic>()

private commandPermutations = new Permutations()

private pluginLoader!: PluginLoader

private rootPlugin!: IPlugin

private topicPermutations = new Permutations()

constructor(public options: Options) {}
Expand Down Expand Up @@ -149,7 +151,7 @@ export class Config implements IConfig {
}

static get rootPlugin(): IPlugin | undefined {
return Config._rootPlugin
return this.rootPlugin
}

public get commandIDs(): string[] {
Expand Down Expand Up @@ -281,18 +283,22 @@ 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})
Config._rootPlugin = await this.pluginLoader.loadRoot()
this.rootPlugin = await this.pluginLoader.loadRoot()

// Cache the root plugin so that we can reference it later when determining if
// we should skip ts-node registration for an ESM plugin.
Cache.getInstance().set('rootPlugin', this.rootPlugin)

this.root = Config._rootPlugin.root
this.pjson = Config._rootPlugin.pjson
this.root = this.rootPlugin.root
this.pjson = this.rootPlugin.pjson

this.plugins.set(Config._rootPlugin.name, Config._rootPlugin)
this.root = Config._rootPlugin.root
this.pjson = Config._rootPlugin.pjson
this.plugins.set(this.rootPlugin.name, this.rootPlugin)
this.root = this.rootPlugin.root
this.pjson = this.rootPlugin.pjson
this.name = this.pjson.name
this.version = this.options.version || this.pjson.version || '0.0.0'
this.channel = this.options.channel || channelFromVersion(this.version)
this.valid = Config._rootPlugin.valid
this.valid = this.rootPlugin.valid

this.arch = arch() === 'ia32' ? 'x86' : (arch() as any)
this.platform = WSL ? 'wsl' : getPlatform()
Expand Down Expand Up @@ -364,7 +370,7 @@ export class Config implements IConfig {
dataDir: this.dataDir,
devPlugins: this.options.devPlugins,
force: opts?.force ?? false,
rootPlugin: Config._rootPlugin,
rootPlugin: this.rootPlugin,
userPlugins: this.options.userPlugins,
})

Expand Down
5 changes: 4 additions & 1 deletion src/config/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ export class Plugin implements IPlugin {

const marker = Performance.mark(OCLIF_MARKER_OWNER, `plugin.commandIDs#${this.name}`, {plugin: this.name})
this._debug(`loading IDs from ${this.commandsDir}`)
const patterns = ['**/*.+(js|cjs|mjs|ts|tsx)', '!**/*.+(d.ts|test.ts|test.js|spec.ts|spec.js)?(x)']
const patterns = [
'**/*.+(js|cjs|mjs|ts|tsx|mts|cts)',
'!**/*.+(d.ts|test.ts|test.js|spec.ts|spec.js|d.mts|d.cts)?(x)',
]
const ids = sync(patterns, {cwd: this.commandsDir}).map((file) => {
const p = parse(file)
const topics = p.dir.split('/')
Expand Down
51 changes: 36 additions & 15 deletions src/config/ts-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,14 @@ import {Plugin, TSConfig} from '../interfaces'
import {settings} from '../settings'
import {readJsonSync} from '../util/fs'
import {isProd} from '../util/util'
import Cache from './cache'
import {Debug} from './util'

// eslint-disable-next-line new-cap
const debug = Debug('ts-node')

export const TS_CONFIGS: Record<string, TSConfig> = {}
const REGISTERED = new Set<string>()
/**
* Cache the root plugin so that we can reference it later when determining if
* we should skip ts-node registration for an ESM plugin.
*/
let ROOT_PLUGIN: Plugin | undefined

function loadTSConfig(root: string): TSConfig | undefined {
if (TS_CONFIGS[root]) return TS_CONFIGS[root]
Expand Down Expand Up @@ -111,7 +107,7 @@ function registerTSNode(root: string): TSConfig | undefined {

tsNode.register(conf)
REGISTERED.add(root)

debug('%O', tsconfig)
return tsconfig
}

Expand All @@ -127,8 +123,12 @@ function registerTSNode(root: string): TSConfig | undefined {
* We still register ts-node for ESM plugins when NODE_ENV is "test" or "development" and root plugin is also ESM
* since that allows plugins to be auto-transpiled when developing locally using `bin/dev.js`.
*/
function cannotTranspileEsm(root: string, plugin: Plugin | undefined, isProduction: boolean): boolean {
return (isProduction || ROOT_PLUGIN?.moduleType === 'commonjs') && plugin?.moduleType === 'module'
function cannotTranspileEsm(
rootPlugin: Plugin | undefined,
plugin: Plugin | undefined,
isProduction: boolean,
): boolean {
return (isProduction || rootPlugin?.moduleType === 'commonjs') && plugin?.moduleType === 'module'
}

/**
Expand All @@ -154,9 +154,19 @@ function cannotUseTsNode(root: string, plugin: Plugin | undefined, isProduction:
function determinePath(root: string, orig: string): string {
const tsconfig = registerTSNode(root)
if (!tsconfig) return orig
const {outDir, rootDir, rootDirs} = tsconfig.compilerOptions
const rootDirPath = rootDir || (rootDirs || [])[0]
if (!rootDirPath || !outDir) return orig
debug(`determining path for ${orig}`)
const {baseUrl, outDir, rootDir, rootDirs} = tsconfig.compilerOptions
const rootDirPath = rootDir ?? (rootDirs ?? [])[0] ?? baseUrl
if (!rootDirPath) {
debug(`no rootDir, rootDirs, or baseUrl specified in tsconfig.json. Returning default path ${orig}`)
return orig
}

if (!outDir) {
debug(`no outDir specified in tsconfig.json. Returning default path ${orig}`)
return orig
}

// rewrite path from ./lib/foo to ./src/foo
const lib = join(root, outDir) // ./lib
const src = join(root, rootDirPath) // ./src
Expand All @@ -168,7 +178,18 @@ function determinePath(root: string, orig: string): string {
// For hooks, it might point to a module, not a file. Something like "./hooks/myhook"
// That file doesn't exist, and the real file is "./hooks/myhook.ts"
// In that case we attempt to resolve to the filename. If it fails it will revert back to the lib path
if (existsSync(out) || existsSync(out + '.ts')) return out

debug(`lib dir: ${lib}`)
debug(`src dir: ${src}`)
debug(`src commands dir: ${out}`)
if (existsSync(out) || existsSync(out + '.ts')) {
debug(`Found source file for ${orig} at ${out}`)
return out
}

debug(`No source file found. Returning default path ${orig}`)
if (!isProd()) memoizedWarn(`Could not find source for ${orig} based on tsconfig. Defaulting to compiled source.`)

return orig
}

Expand All @@ -180,7 +201,7 @@ function determinePath(root: string, orig: string): string {
export function tsPath(root: string, orig: string, plugin: Plugin): string
export function tsPath(root: string, orig: string | undefined, plugin?: Plugin): string | undefined
export function tsPath(root: string, orig: string | undefined, plugin?: Plugin): string | undefined {
if (plugin?.isRoot) ROOT_PLUGIN = plugin
const rootPlugin = Cache.getInstance().get('rootPlugin')

if (!orig) return orig
orig = orig.startsWith(root) ? orig : join(root, orig)
Expand All @@ -194,9 +215,9 @@ export function tsPath(root: string, orig: string | undefined, plugin?: Plugin):

const isProduction = isProd()

if (cannotTranspileEsm(root, plugin, isProduction)) {
if (cannotTranspileEsm(rootPlugin, plugin, isProduction)) {
debug(
`Skipping ts-node registration for ${root} because it's an ESM module (NODE_ENV: ${process.env.NODE_ENV}, root plugin module type: ${ROOT_PLUGIN?.moduleType})))`,
`Skipping ts-node registration for ${root} because it's an ESM module (NODE_ENV: ${process.env.NODE_ENV}, root plugin module type: ${rootPlugin?.moduleType})))`,
)
if (plugin?.type === 'link')
memoizedWarn(
Expand Down
1 change: 0 additions & 1 deletion src/flags.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable valid-jsdoc */
import {URL} from 'node:url'

import {CLIError} from './errors'
Expand Down
1 change: 1 addition & 0 deletions src/interfaces/ts-config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export interface TSConfig {
compilerOptions: {
baseUrl?: string
emitDecoratorMetadata?: boolean
esModuleInterop?: boolean
experimentalDecorators?: boolean
Expand Down
2 changes: 1 addition & 1 deletion src/module-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const getPackageType = require('get-package-type')
* Defines file extension resolution when source files do not have an extension.
*/
// eslint-disable-next-line camelcase
const s_EXTENSIONS: string[] = ['.ts', '.js', '.mjs', '.cjs']
const s_EXTENSIONS: string[] = ['.ts', '.js', '.mjs', '.cjs', '.mts', '.cts']

const isPlugin = (config: IConfig | IPlugin): config is IPlugin => (<IPlugin>config).type !== undefined

Expand Down
9 changes: 4 additions & 5 deletions src/util/cache-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,10 @@ export async function cacheCommand(
// @ts-expect-error because v2 commands have base flags stored in _baseFlags
const uncachedBaseFlags = cmd.baseFlags ?? cmd._baseFlags

const flags = await cacheFlags(
aggregateFlags(uncachedFlags, uncachedBaseFlags, cmd.enableJsonFlag),
respectNoCacheDefault,
)
const args = await cacheArgs(ensureArgObject(cmd.args), respectNoCacheDefault)
const [flags, args] = await Promise.all([
await cacheFlags(aggregateFlags(uncachedFlags, uncachedBaseFlags, cmd.enableJsonFlag), respectNoCacheDefault),
await cacheArgs(ensureArgObject(cmd.args), respectNoCacheDefault),
])

const stdProperties = {
aliases: cmd.aliases || [],
Expand Down
3 changes: 1 addition & 2 deletions test/command/main.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import {expect} from 'chai'
import {resolve} from 'node:path'
import {SinonSandbox, SinonStub, createSandbox} from 'sinon'
import stripAnsi from 'strip-ansi'

import {Interfaces, stdout} from '../../src/index'
import {run} from '../../src/main'
import {requireJson} from '../../src/util/fs'

import stripAnsi = require('strip-ansi')

const pjson = requireJson<Interfaces.PJSON>(__dirname, '..', '..', 'package.json')
const version = `@oclif/core/${pjson.version} ${process.platform}-${process.arch} node-${process.version}`

Expand Down
1 change: 0 additions & 1 deletion test/integration/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ export class Executor {
}
}

// eslint-disable-next-line valid-jsdoc
/**
* Setup for integration tests.
*
Expand Down
Loading
Loading