Skip to content

Commit

Permalink
Merge pull request #824 from oclif/mdonnalley/various-fixes
Browse files Browse the repository at this point in the history
feat: various fixes
  • Loading branch information
WillieRuemmele authored Oct 13, 2023
2 parents 093398a + 5803cee commit 2d1590d
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 73 deletions.
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: 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
Loading

0 comments on commit 2d1590d

Please sign in to comment.