From 4411daf053c8f679ff7c7ca48f044e4a0a0be875 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Fri, 27 Oct 2023 09:54:53 -0600 Subject: [PATCH 01/18] feat: add swc option if possible --- src/config/ts-node.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index a9b36ea2c..594f6bbd4 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -47,7 +47,7 @@ function loadTSConfig(root: string): TSConfig | undefined { } } -function registerTSNode(root: string): TSConfig | undefined { +function registerTSNode(root: string, plugin?: Plugin): TSConfig | undefined { const tsconfig = loadTSConfig(root) if (!tsconfig) return if (REGISTERED.has(root)) return tsconfig @@ -80,6 +80,11 @@ function registerTSNode(root: string): TSConfig | undefined { rootDirs.push(join(root, 'src')) } + const swc = plugin + ? Boolean(plugin.pjson.devDependencies?.['@swc/core']) + : existsSync(join(root, 'node_modules', '@swc')) + if (swc) debug(`@swc/core dependency found for ${root}. Using swc for transpilation.`) + const conf: TSNode.RegisterOptions = { compilerOptions: { emitDecoratorMetadata: tsconfig.compilerOptions.emitDecoratorMetadata ?? false, @@ -101,12 +106,14 @@ function registerTSNode(root: string): TSConfig | undefined { scope: true, scopeDir: root, skipProject: true, + swc, transpileOnly: true, } tsNode.register(conf) REGISTERED.add(root) - debug('%O', tsconfig) + debug('tsconfig: %O', tsconfig) + debug('tsconfig registration options: %O', conf) return tsconfig } @@ -150,8 +157,8 @@ function cannotUseTsNode(root: string, plugin: Plugin | undefined, isProduction: /** * Determine the path to the source file from the compiled ./lib files */ -function determinePath(root: string, orig: string): string { - const tsconfig = registerTSNode(root) +function determinePath(root: string, orig: string, plugin?: Plugin): string { + const tsconfig = registerTSNode(root, plugin) if (!tsconfig) return orig debug(`determining path for ${orig}`) const {baseUrl, outDir, rootDir, rootDirs} = tsconfig.compilerOptions @@ -245,7 +252,7 @@ export function tsPath(root: string, orig: string | undefined, plugin?: Plugin): } try { - return determinePath(root, orig) + return determinePath(root, orig, plugin) } catch (error: any) { debug(error) return orig From 25757658852937d3c3f3d3c634e8512424e7c0f2 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Fri, 27 Oct 2023 09:55:40 -0600 Subject: [PATCH 02/18] chore: better debugs --- src/config/ts-node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index 594f6bbd4..aff8d8956 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -113,7 +113,7 @@ function registerTSNode(root: string, plugin?: Plugin): TSConfig | undefined { tsNode.register(conf) REGISTERED.add(root) debug('tsconfig: %O', tsconfig) - debug('tsconfig registration options: %O', conf) + debug('ts-node options: %O', conf) return tsconfig } From 09f4b91a4df2e21632160ce2e3eb8246aa85bc9b Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Fri, 27 Oct 2023 11:48:02 -0600 Subject: [PATCH 03/18] feat: use tsconfck for resolving extended tsconfigs --- package.json | 1 + src/config/plugin.ts | 78 +++++++++++++++++------ src/config/ts-node.ts | 124 +++++++++++++++++++++++++++--------- src/interfaces/ts-config.ts | 1 + src/module-loader.ts | 9 +-- test/config/ts-node.test.ts | 81 +++++++++++++++++++++-- yarn.lock | 5 ++ 7 files changed, 238 insertions(+), 61 deletions(-) diff --git a/package.json b/package.json index f1840b0d2..a9946ab2d 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "strip-ansi": "^6.0.1", "supports-color": "^8.1.1", "supports-hyperlinks": "^2.2.0", + "tsconfck": "^3.0.0", "widest-line": "^3.1.0", "wordwrap": "^1.0.0", "wrap-ansi": "^7.0.0" diff --git a/src/config/plugin.ts b/src/config/plugin.ts index fbf7d9009..945d4a45d 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -1,4 +1,4 @@ -import {sync} from 'globby' +import globby, {sync} from 'globby' import {join, parse, relative, sep} from 'node:path' import {inspect} from 'node:util' @@ -13,8 +13,8 @@ import {OCLIF_MARKER_OWNER, Performance} from '../performance' import {cacheCommand} from '../util/cache-command' import {findRoot} from '../util/find-root' import {readJson, requireJson} from '../util/fs' -import {castArray, compact, isProd, mapValues} from '../util/util' -import {tsPath} from './ts-node' +import {castArray, compact, isProd} from '../util/util' +import {tsPath, tsPathSync} from './ts-node' import {Debug, getCommandIdPermutations} from './util' const _pjson = requireJson(__dirname, '..', '..', 'package.json') @@ -41,6 +41,21 @@ const search = (cmd: any) => { return Object.values(cmd).find((cmd: any) => typeof cmd.run === 'function') } +const GLOB_PATTERNS = [ + '**/*.+(js|cjs|mjs|ts|tsx|mts|cts)', + '!**/*.+(d.ts|test.ts|test.js|spec.ts|spec.js|d.mts|d.cts)?(x)', +] + +function processCommandIds(files: string[]): string[] { + return files.map((file) => { + const p = parse(file) + const topics = p.dir.split('/') + const command = p.name !== 'index' && p.name + const id = [...topics, command].filter(Boolean).join(':') + return id === '' ? '.' : id + }) +} + export class Plugin implements IPlugin { alias!: string @@ -89,32 +104,28 @@ export class Plugin implements IPlugin { constructor(public options: PluginOptions) {} + /** + * @deprecated use getCommandIDs instead. Underlying implementation cannot read extended tsconfig.json files. + */ public get commandIDs(): string[] { if (!this.commandsDir) return [] 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|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('/') - const command = p.name !== 'index' && p.name - const id = [...topics, command].filter(Boolean).join(':') - return id === '' ? '.' : id - }) + const ids = processCommandIds(sync(GLOB_PATTERNS, {cwd: this.commandsDir})) this._debug('found commands', ids) marker?.addDetails({count: ids.length}) marker?.stop() return ids } + /** + * @deprecated use getCommandsDir instead. Underlying implementation cannot read extended tsconfig.json files. + */ public get commandsDir(): string | undefined { if (this._commandsDir) return this._commandsDir - this._commandsDir = tsPath(this.root, this.pjson.oclif.commands, this) + this._commandsDir = tsPathSync(this.root, this.pjson.oclif.commands, this) return this._commandsDir } @@ -133,14 +144,15 @@ export class Plugin implements IPlugin { }) const fetch = async () => { - if (!this.commandsDir) return + const commandsDir = await this.getCommandsDir() + if (!commandsDir) return let module let isESM: boolean | undefined let filePath: string | undefined try { ;({filePath, isESM, module} = cachedCommandCanBeUsed(this.manifest, id) ? await loadWithDataFromManifest(this.manifest.commands[id], this.root) - : await loadWithData(this, join(this.commandsDir ?? this.pjson.oclif.commands, ...id.split(':')))) + : await loadWithData(this, join(commandsDir ?? this.pjson.oclif.commands, ...id.split(':')))) this._debug(isESM ? '(import)' : '(require)', filePath) } catch (error: any) { if (!opts.must && error.code === 'MODULE_NOT_FOUND') return @@ -162,8 +174,29 @@ export class Plugin implements IPlugin { return cmd } + public async getCommandIDs(): Promise { + const commandsDir = await this.getCommandsDir() + if (!commandsDir) return [] + + const marker = Performance.mark(OCLIF_MARKER_OWNER, `plugin.getCommandIDs#${this.name}`, {plugin: this.name}) + this._debug(`loading IDs from ${commandsDir}`) + const files = await globby(GLOB_PATTERNS, {cwd: commandsDir}) + const ids = processCommandIds(files) + this._debug('found commands', ids) + marker?.addDetails({count: ids.length}) + marker?.stop() + return ids + } + + public async getCommandsDir(): Promise { + if (this._commandsDir) return this._commandsDir + + this._commandsDir = await tsPath(this.root, this.pjson.oclif.commands, this) + return this._commandsDir + } + public async load(): Promise { - this.type = this.options.type || 'core' + this.type = this.options.type ?? 'core' this.tag = this.options.tag this.isRoot = this.options.isRoot ?? false if (this.options.parent) this.parent = this.options.parent as Plugin @@ -192,7 +225,11 @@ export class Plugin implements IPlugin { this.pjson.oclif = this.pjson['cli-engine'] || {} } - this.hooks = mapValues(this.pjson.oclif.hooks ?? {}, (i) => castArray(i).map((i) => tsPath(this.root, i, this))) + this.hooks = {} + for (const [k, v] of Object.entries(this.pjson.oclif.hooks ?? {})) { + // eslint-disable-next-line no-await-in-loop + this.hooks[k] = await Promise.all(castArray(v).map(async (i) => tsPath(this.root, i, this))) + } this.manifest = await this._manifest() this.commands = Object.entries(this.manifest.commands) @@ -242,10 +279,11 @@ export class Plugin implements IPlugin { } } + const commandIDs = await this.getCommandIDs() const manifest = { commands: ( await Promise.all( - this.commandIDs.map(async (id) => { + commandIDs.map(async (id) => { try { const cached = await cacheCommand(await this.findCommand(id, {must: true}), this, respectNoCacheDefault) if (this.flexibleTaxonomy) { diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index aff8d8956..63baf8479 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -4,7 +4,7 @@ import * as TSNode from 'ts-node' import {memoizedWarn} from '../errors' import {Plugin, TSConfig} from '../interfaces' import {settings} from '../settings' -import {existsSync, readJsonSync} from '../util/fs' +import {existsSync, readJsonSync, safeReadJson} from '../util/fs' import {isProd} from '../util/util' import Cache from './cache' import {Debug} from './util' @@ -15,7 +15,10 @@ const debug = Debug('ts-node') export const TS_CONFIGS: Record = {} const REGISTERED = new Set() -function loadTSConfig(root: string): TSConfig | undefined { +/** + * @deprecated use loadTSConfig instead since it's able to merge extended tsconfig.json files + */ +function loadTSConfigSync(root: string): TSConfig | undefined { if (TS_CONFIGS[root]) return TS_CONFIGS[root] const tsconfigPath = join(root, 'tsconfig.json') let typescript: typeof import('typescript') | undefined @@ -37,8 +40,7 @@ function loadTSConfig(root: string): TSConfig | undefined { const tsconfig = typescript.parseConfigFileTextToJson(tsconfigPath, readJsonSync(tsconfigPath, false)).config if (!tsconfig || !tsconfig.compilerOptions) { throw new Error( - `Could not read and parse tsconfig.json at ${tsconfigPath}, or it ` + - 'did not contain a "compilerOptions" section.', + `Could not read and parse tsconfig.json at ${tsconfigPath}, or it did not contain a "compilerOptions" section.`, ) } @@ -47,10 +49,35 @@ function loadTSConfig(root: string): TSConfig | undefined { } } -function registerTSNode(root: string, plugin?: Plugin): TSConfig | undefined { - const tsconfig = loadTSConfig(root) - if (!tsconfig) return - if (REGISTERED.has(root)) return tsconfig +async function loadTSConfig(root: string): Promise { + if (TS_CONFIGS[root]) return TS_CONFIGS[root] + const tsconfigPath = join(root, 'tsconfig.json') + const tsconfig = await safeReadJson(tsconfigPath) + if (!tsconfig || Object.keys(tsconfig.compilerOptions).length === 0) return + + try { + const {parse} = await import('tsconfck') + const result = await parse(tsconfigPath, {}) + let tsNodeOpts = {} + for (const extended of (result.extended ?? []).reverse()) { + if (extended.tsconfig['ts-node']) { + tsNodeOpts = {...tsNodeOpts, ...extended.tsconfig['ts-node']} + } + } + + const final = {...result.tsconfig, 'ts-node': tsNodeOpts} + + TS_CONFIGS[root] = final + return final + } catch (error) { + console.log(error) + debug(`Could not parse tsconfig.json. Skipping ts-node registration for ${root}.`) + memoizedWarn(`Could not parse tsconfig.json for ${root}. Falling back to compiled source.`) + } +} + +function registerTSNode(root: string, tsconfig: TSConfig): void { + if (REGISTERED.has(root)) return debug('registering ts-node at', root) const tsNodePath = require.resolve('ts-node', {paths: [root, __dirname]}) debug('ts-node path:', tsNodePath) @@ -80,11 +107,6 @@ function registerTSNode(root: string, plugin?: Plugin): TSConfig | undefined { rootDirs.push(join(root, 'src')) } - const swc = plugin - ? Boolean(plugin.pjson.devDependencies?.['@swc/core']) - : existsSync(join(root, 'node_modules', '@swc')) - if (swc) debug(`@swc/core dependency found for ${root}. Using swc for transpilation.`) - const conf: TSNode.RegisterOptions = { compilerOptions: { emitDecoratorMetadata: tsconfig.compilerOptions.emitDecoratorMetadata ?? false, @@ -106,7 +128,7 @@ function registerTSNode(root: string, plugin?: Plugin): TSConfig | undefined { scope: true, scopeDir: root, skipProject: true, - swc, + swc: tsconfig['ts-node']?.swc ?? false, transpileOnly: true, } @@ -114,7 +136,6 @@ function registerTSNode(root: string, plugin?: Plugin): TSConfig | undefined { REGISTERED.add(root) debug('tsconfig: %O', tsconfig) debug('ts-node options: %O', conf) - return tsconfig } /** @@ -157,9 +178,7 @@ function cannotUseTsNode(root: string, plugin: Plugin | undefined, isProduction: /** * Determine the path to the source file from the compiled ./lib files */ -function determinePath(root: string, orig: string, plugin?: Plugin): string { - const tsconfig = registerTSNode(root, plugin) - if (!tsconfig) return orig +function determinePath(root: string, orig: string, tsconfig: TSConfig): string { debug(`determining path for ${orig}`) const {baseUrl, outDir, rootDir, rootDirs} = tsconfig.compilerOptions const rootDirPath = rootDir ?? (rootDirs ?? [])[0] ?? baseUrl @@ -199,19 +218,9 @@ function determinePath(root: string, orig: string, plugin?: Plugin): string { return orig } -/** - * Convert a path from the compiled ./lib files to the ./src typescript source - * this is for developing typescript plugins/CLIs - * if there is a tsconfig and the original sources exist, it attempts to require ts-node - */ -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 { +function shouldUseOriginal(root: string, orig: string, plugin?: Plugin): string | false { const rootPlugin = plugin?.options.isRoot ? plugin : Cache.getInstance().get('rootPlugin') - if (!orig) return orig - orig = orig.startsWith(root) ? orig : join(root, orig) - // NOTE: The order of these checks matter! if (settings.tsnodeEnabled === false) { @@ -251,8 +260,63 @@ export function tsPath(root: string, orig: string | undefined, plugin?: Plugin): return orig } + return false +} + +/** + * @deprecated use tsPath instead since it's able to merge extended tsconfig.json files + */ +export function tsPathSync(root: string, orig: string, plugin: Plugin): string +/** + * @deprecated use tsPath instead since it's able to merge extended tsconfig.json files + */ +export function tsPathSync(root: string, orig: string | undefined, plugin?: Plugin): string | undefined +/** + * Convert a path from the compiled ./lib files to the ./src typescript source + * this is for developing typescript plugins/CLIs + * if there is a tsconfig and the original sources exist, it attempts to require ts-node + * + * @deprecated use tsPath instead since it's able to merge extended tsconfig.json files + */ +export function tsPathSync(root: string, orig: string | undefined, plugin?: Plugin): string | undefined { + if (!orig) return orig + orig = orig.startsWith(root) ? orig : join(root, orig) + + const originalPath = shouldUseOriginal(root, orig, plugin) + if (originalPath) return originalPath + try { - return determinePath(root, orig, plugin) + const tsconfig = loadTSConfigSync(root) + if (!tsconfig) return orig + + registerTSNode(root, tsconfig) + return determinePath(root, orig, tsconfig) + } catch (error: any) { + debug(error) + return orig + } +} + +/** + * Convert a path from the compiled ./lib files to the ./src typescript source + * this is for developing typescript plugins/CLIs + * if there is a tsconfig and the original sources exist, it attempts to require ts-node + */ +export async function tsPath(root: string, orig: string, plugin: Plugin): Promise +export async function tsPath(root: string, orig: string | undefined, plugin?: Plugin): Promise +export async function tsPath(root: string, orig: string | undefined, plugin?: Plugin): Promise { + if (!orig) return orig + orig = orig.startsWith(root) ? orig : join(root, orig) + + const originalPath = shouldUseOriginal(root, orig, plugin) + if (originalPath) return originalPath + + try { + const tsconfig = await loadTSConfig(root) + if (!tsconfig) return orig + + registerTSNode(root, tsconfig) + return determinePath(root, orig, tsconfig) } catch (error: any) { debug(error) return orig diff --git a/src/interfaces/ts-config.ts b/src/interfaces/ts-config.ts index 44aea6320..9d8b1ff22 100644 --- a/src/interfaces/ts-config.ts +++ b/src/interfaces/ts-config.ts @@ -17,5 +17,6 @@ export interface TSConfig { esm?: boolean experimentalSpecifierResolution?: 'explicit' | 'node' scope?: boolean + swc?: boolean } } diff --git a/src/module-loader.ts b/src/module-loader.ts index 588a4d915..15a2981d5 100644 --- a/src/module-loader.ts +++ b/src/module-loader.ts @@ -38,7 +38,7 @@ export async function load(config: IConfig | IPlugin, modulePath: strin let filePath: string | undefined let isESM: boolean | undefined try { - ;({filePath, isESM} = resolvePath(config, modulePath)) + ;({filePath, isESM} = await resolvePath(config, modulePath)) return (isESM ? await import(pathToFileURL(filePath).href) : require(filePath)) as T } catch (error: any) { if (error.code === 'MODULE_NOT_FOUND' || error.code === 'ERR_MODULE_NOT_FOUND') { @@ -73,7 +73,7 @@ export async function loadWithData( let filePath: string | undefined let isESM: boolean | undefined try { - ;({filePath, isESM} = resolvePath(config, modulePath)) + ;({filePath, isESM} = await resolvePath(config, modulePath)) const module = isESM ? await import(pathToFileURL(filePath).href) : require(filePath) return {filePath, isESM, module} } catch (error: any) { @@ -172,7 +172,7 @@ export function isPathModule(filePath: string): boolean { * * @returns {{isESM: boolean, filePath: string}} An object including file path and whether the module is ESM. */ -function resolvePath(config: IConfig | IPlugin, modulePath: string): {filePath: string; isESM: boolean} { +async function resolvePath(config: IConfig | IPlugin, modulePath: string): Promise<{filePath: string; isESM: boolean}> { let isESM: boolean let filePath: string | undefined @@ -181,7 +181,8 @@ function resolvePath(config: IConfig | IPlugin, modulePath: string): {filePath: isESM = isPathModule(filePath) } catch { filePath = - (isPlugin(config) ? tsPath(config.root, modulePath, config) : tsPath(config.root, modulePath)) ?? modulePath + (isPlugin(config) ? await tsPath(config.root, modulePath, config) : await tsPath(config.root, modulePath)) ?? + modulePath let fileExists = false let isDirectory = false diff --git a/test/config/ts-node.test.ts b/test/config/ts-node.test.ts index 2ab8e44a2..d1d946478 100644 --- a/test/config/ts-node.test.ts +++ b/test/config/ts-node.test.ts @@ -22,7 +22,7 @@ const DEFAULT_TS_CONFIG: Interfaces.TSConfig = { }, } -describe('tsPath', () => { +describe('tsPathSync', () => { let sandbox: SinonSandbox beforeEach(() => { @@ -44,25 +44,25 @@ describe('tsPath', () => { it('should resolve a .js file to ts src', () => { sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - const result = configTsNode.tsPath(root, jsCompiled) + const result = configTsNode.tsPathSync(root, jsCompiled) expect(result).to.equal(join(root, tsModule)) }) it('should resolve a module file to ts src', () => { sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - const result = configTsNode.tsPath(root, jsCompiledModule) + const result = configTsNode.tsPathSync(root, jsCompiledModule) expect(result).to.equal(join(root, tsModule)) }) it('should resolve a .ts file', () => { sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - const result = configTsNode.tsPath(root, tsSource) + const result = configTsNode.tsPathSync(root, tsSource) expect(result).to.equal(join(root, tsSource)) }) it('should resolve .js with no rootDir or outDir', () => { sandbox.stub(util, 'readJsonSync').returns({compilerOptions: {}}) - const result = configTsNode.tsPath(root, jsCompiled) + const result = configTsNode.tsPathSync(root, jsCompiled) expect(result).to.equal(join(root, jsCompiled)) }) @@ -72,7 +72,7 @@ describe('tsPath', () => { const originalNodeEnv = process.env.NODE_ENV delete process.env.NODE_ENV - const result = configTsNode.tsPath(root, jsCompiled) + const result = configTsNode.tsPathSync(root, jsCompiled) expect(result).to.equal(join(root, tsModule)) process.env.NODE_ENV = originalNodeEnv @@ -82,7 +82,74 @@ describe('tsPath', () => { it('should resolve to js if disabled', () => { sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) settings.tsnodeEnabled = false - const result = configTsNode.tsPath(root, jsCompiled) + const result = configTsNode.tsPathSync(root, jsCompiled) + expect(result).to.equal(join(root, jsCompiled)) + + delete settings.tsnodeEnabled + }) +}) + +describe('tsPath', () => { + let sandbox: SinonSandbox + + beforeEach(() => { + sandbox = createSandbox() + // sandbox.stub(util, 'existsSync').returns(true) + sandbox.stub(tsNode, 'register') + }) + + afterEach(() => { + sandbox.restore() + // Clear caches so that unit tests don't affect each other + // @ts-expect-error because TS_CONFIGS is not exported + // eslint-disable-next-line import/namespace + configTsNode.TS_CONFIGS = {} + // @ts-expect-error because REGISTERED is not exported + // eslint-disable-next-line import/namespace + configTsNode.REGISTERED = new Set() + }) + + it('should resolve a .js file to ts src', async () => { + sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + const result = await configTsNode.tsPath(root, jsCompiled) + expect(result).to.equal(join(root, tsModule)) + }) + + it('should resolve a module file to ts src', async () => { + sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + const result = await configTsNode.tsPath(root, jsCompiledModule) + expect(result).to.equal(join(root, tsModule)) + }) + + it('should resolve a .ts file', async () => { + sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + const result = await configTsNode.tsPath(root, tsSource) + expect(result).to.equal(join(root, tsSource)) + }) + + it('should resolve .js with no rootDir or outDir', async () => { + sandbox.stub(util, 'safeReadJson').resolves({compilerOptions: {}}) + const result = await configTsNode.tsPath(root, jsCompiled) + expect(result).to.equal(join(root, jsCompiled)) + }) + + it('should resolve to .ts file if enabled and prod', async () => { + sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + settings.tsnodeEnabled = true + const originalNodeEnv = process.env.NODE_ENV + delete process.env.NODE_ENV + + const result = await configTsNode.tsPath(root, jsCompiled) + expect(result).to.equal(join(root, tsModule)) + + process.env.NODE_ENV = originalNodeEnv + delete settings.tsnodeEnabled + }) + + it('should resolve to js if disabled', async () => { + sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + settings.tsnodeEnabled = false + const result = await configTsNode.tsPath(root, jsCompiled) expect(result).to.equal(join(root, jsCompiled)) delete settings.tsnodeEnabled diff --git a/yarn.lock b/yarn.lock index 00e2162de..347a5e844 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6689,6 +6689,11 @@ ts-node@^10.8.1, ts-node@^10.9.1: v8-compile-cache-lib "^3.0.1" yn "3.1.1" +tsconfck@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/tsconfck/-/tsconfck-3.0.0.tgz#b469f1ced12973bbec3209a55ed8de3bb04223c9" + integrity sha512-w3wnsIrJNi7avf4Zb0VjOoodoO0woEqGgZGQm+LHH9przdUI+XDKsWAXwxHA1DaRTjeuZNcregSzr7RaA8zG9A== + tsconfig-paths@^3.10.1, tsconfig-paths@^3.14.2: version "3.14.2" resolved "https://registry.yarnpkg.com/tsconfig-paths/-/tsconfig-paths-3.14.2.tgz#6e32f1f79412decd261f92d633a9dc1cfa99f088" From 1350dcaab9d8920c31918673ba8b3550a94a3411 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Fri, 27 Oct 2023 14:00:46 -0600 Subject: [PATCH 04/18] test: plugins e2e --- test/integration/plugins.e2e.ts | 1 + test/integration/util.ts | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/test/integration/plugins.e2e.ts b/test/integration/plugins.e2e.ts index 6bf15f8a9..0a88c8c1e 100644 --- a/test/integration/plugins.e2e.ts +++ b/test/integration/plugins.e2e.ts @@ -20,6 +20,7 @@ describe('oclif plugins', () => { '@oclif/plugin-version', '@oclif/plugin-which', ], + yarnInstallArgs: ['--no-lockfile'], }) }) diff --git a/test/integration/util.ts b/test/integration/util.ts index fc2c3066e..1ae0a80f0 100644 --- a/test/integration/util.ts +++ b/test/integration/util.ts @@ -24,6 +24,7 @@ export type SetupOptions = { plugins?: string[] subDir?: string noLinkCore?: boolean + yarnInstallArgs?: string[] } export type ExecutorOptions = { @@ -186,9 +187,12 @@ export async function setup(testFile: string, options: SetupOptions): Promise Date: Mon, 30 Oct 2023 08:31:23 -0600 Subject: [PATCH 05/18] feat: improve implementation --- src/config/plugin.ts | 43 +++--------- src/config/ts-node.ts | 133 +++++++++--------------------------- src/interfaces/plugin.ts | 1 + src/interfaces/ts-config.ts | 1 + 4 files changed, 43 insertions(+), 135 deletions(-) diff --git a/src/config/plugin.ts b/src/config/plugin.ts index 945d4a45d..2e660c53c 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -1,4 +1,4 @@ -import globby, {sync} from 'globby' +import globby from 'globby' import {join, parse, relative, sep} from 'node:path' import {inspect} from 'node:util' @@ -14,7 +14,7 @@ import {cacheCommand} from '../util/cache-command' import {findRoot} from '../util/find-root' import {readJson, requireJson} from '../util/fs' import {castArray, compact, isProd} from '../util/util' -import {tsPath, tsPathSync} from './ts-node' +import {tsPath} from './ts-node' import {Debug, getCommandIdPermutations} from './util' const _pjson = requireJson(__dirname, '..', '..', 'package.json') @@ -63,8 +63,12 @@ export class Plugin implements IPlugin { children: Plugin[] = [] + commandIDs!: string[] + commands!: Command.Loadable[] + commandsDir!: string | undefined + hasManifest = false hooks!: {[k: string]: string[]} @@ -95,8 +99,6 @@ export class Plugin implements IPlugin { _base = `${_pjson.name}@${_pjson.version}` - private _commandsDir!: string | undefined - // eslint-disable-next-line new-cap protected _debug = Debug() @@ -104,31 +106,6 @@ export class Plugin implements IPlugin { constructor(public options: PluginOptions) {} - /** - * @deprecated use getCommandIDs instead. Underlying implementation cannot read extended tsconfig.json files. - */ - public get commandIDs(): string[] { - if (!this.commandsDir) return [] - - const marker = Performance.mark(OCLIF_MARKER_OWNER, `plugin.commandIDs#${this.name}`, {plugin: this.name}) - this._debug(`loading IDs from ${this.commandsDir}`) - const ids = processCommandIds(sync(GLOB_PATTERNS, {cwd: this.commandsDir})) - this._debug('found commands', ids) - marker?.addDetails({count: ids.length}) - marker?.stop() - return ids - } - - /** - * @deprecated use getCommandsDir instead. Underlying implementation cannot read extended tsconfig.json files. - */ - public get commandsDir(): string | undefined { - if (this._commandsDir) return this._commandsDir - - this._commandsDir = tsPathSync(this.root, this.pjson.oclif.commands, this) - return this._commandsDir - } - public get topics(): Topic[] { return topicsToArray(this.pjson.oclif.topics || {}) } @@ -189,10 +166,7 @@ export class Plugin implements IPlugin { } public async getCommandsDir(): Promise { - if (this._commandsDir) return this._commandsDir - - this._commandsDir = await tsPath(this.root, this.pjson.oclif.commands, this) - return this._commandsDir + return tsPath(this.root, this.pjson.oclif.commands, this) } public async load(): Promise { @@ -225,6 +199,9 @@ export class Plugin implements IPlugin { this.pjson.oclif = this.pjson['cli-engine'] || {} } + this.commandsDir = await this.getCommandsDir() + this.commandIDs = await this.getCommandIDs() + this.hooks = {} for (const [k, v] of Object.entries(this.pjson.oclif.hooks ?? {})) { // eslint-disable-next-line no-await-in-loop diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index 63baf8479..07d0b1992 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -4,7 +4,7 @@ import * as TSNode from 'ts-node' import {memoizedWarn} from '../errors' import {Plugin, TSConfig} from '../interfaces' import {settings} from '../settings' -import {existsSync, readJsonSync, safeReadJson} from '../util/fs' +import {existsSync, safeReadJson} from '../util/fs' import {isProd} from '../util/util' import Cache from './cache' import {Debug} from './util' @@ -15,62 +15,32 @@ const debug = Debug('ts-node') export const TS_CONFIGS: Record = {} const REGISTERED = new Set() -/** - * @deprecated use loadTSConfig instead since it's able to merge extended tsconfig.json files - */ -function loadTSConfigSync(root: string): TSConfig | undefined { - if (TS_CONFIGS[root]) return TS_CONFIGS[root] - const tsconfigPath = join(root, 'tsconfig.json') - let typescript: typeof import('typescript') | undefined +async function loadTSConfig(root: string): Promise { try { - typescript = require('typescript') - } catch { - try { - typescript = require(require.resolve('typescript', {paths: [root, __dirname]})) - } catch { - debug(`Could not find typescript dependency. Skipping ts-node registration for ${root}.`) - memoizedWarn( - 'Could not find typescript. Please ensure that typescript is a devDependency. Falling back to compiled source.', - ) - return - } - } - - if (existsSync(tsconfigPath) && typescript) { - const tsconfig = typescript.parseConfigFileTextToJson(tsconfigPath, readJsonSync(tsconfigPath, false)).config - if (!tsconfig || !tsconfig.compilerOptions) { - throw new Error( - `Could not read and parse tsconfig.json at ${tsconfigPath}, or it did not contain a "compilerOptions" section.`, - ) - } + if (TS_CONFIGS[root]) return TS_CONFIGS[root] + const tsconfigPath = join(root, 'tsconfig.json') + const tsconfig = await safeReadJson(tsconfigPath) + if (!tsconfig || Object.keys(tsconfig.compilerOptions).length === 0) return TS_CONFIGS[root] = tsconfig - return tsconfig - } -} -async function loadTSConfig(root: string): Promise { - if (TS_CONFIGS[root]) return TS_CONFIGS[root] - const tsconfigPath = join(root, 'tsconfig.json') - const tsconfig = await safeReadJson(tsconfigPath) - if (!tsconfig || Object.keys(tsconfig.compilerOptions).length === 0) return - - try { - const {parse} = await import('tsconfck') - const result = await parse(tsconfigPath, {}) - let tsNodeOpts = {} - for (const extended of (result.extended ?? []).reverse()) { - if (extended.tsconfig['ts-node']) { - tsNodeOpts = {...tsNodeOpts, ...extended.tsconfig['ts-node']} + if (tsconfig.extends) { + const {parse} = await import('tsconfck') + const result = await parse(tsconfigPath, {}) + let tsNodeOpts = {} + for (const extended of (result.extended ?? []).reverse()) { + if (extended.tsconfig['ts-node']) { + tsNodeOpts = {...tsNodeOpts, ...extended.tsconfig['ts-node']} + } } - } - const final = {...result.tsconfig, 'ts-node': tsNodeOpts} + const final = {...result.tsconfig, 'ts-node': tsNodeOpts} - TS_CONFIGS[root] = final - return final - } catch (error) { - console.log(error) + TS_CONFIGS[root] = final + } + + return TS_CONFIGS[root] + } catch { debug(`Could not parse tsconfig.json. Skipping ts-node registration for ${root}.`) memoizedWarn(`Could not parse tsconfig.json for ${root}. Falling back to compiled source.`) } @@ -218,9 +188,19 @@ function determinePath(root: string, orig: string, tsconfig: TSConfig): string { return orig } -function shouldUseOriginal(root: string, orig: string, plugin?: Plugin): string | false { +/** + * Convert a path from the compiled ./lib files to the ./src typescript source + * this is for developing typescript plugins/CLIs + * if there is a tsconfig and the original sources exist, it attempts to require ts-node + */ +export async function tsPath(root: string, orig: string, plugin: Plugin): Promise +export async function tsPath(root: string, orig: string | undefined, plugin?: Plugin): Promise +export async function tsPath(root: string, orig: string | undefined, plugin?: Plugin): Promise { const rootPlugin = plugin?.options.isRoot ? plugin : Cache.getInstance().get('rootPlugin') + if (!orig) return orig + orig = orig.startsWith(root) ? orig : join(root, orig) + // NOTE: The order of these checks matter! if (settings.tsnodeEnabled === false) { @@ -260,57 +240,6 @@ function shouldUseOriginal(root: string, orig: string, plugin?: Plugin): string return orig } - return false -} - -/** - * @deprecated use tsPath instead since it's able to merge extended tsconfig.json files - */ -export function tsPathSync(root: string, orig: string, plugin: Plugin): string -/** - * @deprecated use tsPath instead since it's able to merge extended tsconfig.json files - */ -export function tsPathSync(root: string, orig: string | undefined, plugin?: Plugin): string | undefined -/** - * Convert a path from the compiled ./lib files to the ./src typescript source - * this is for developing typescript plugins/CLIs - * if there is a tsconfig and the original sources exist, it attempts to require ts-node - * - * @deprecated use tsPath instead since it's able to merge extended tsconfig.json files - */ -export function tsPathSync(root: string, orig: string | undefined, plugin?: Plugin): string | undefined { - if (!orig) return orig - orig = orig.startsWith(root) ? orig : join(root, orig) - - const originalPath = shouldUseOriginal(root, orig, plugin) - if (originalPath) return originalPath - - try { - const tsconfig = loadTSConfigSync(root) - if (!tsconfig) return orig - - registerTSNode(root, tsconfig) - return determinePath(root, orig, tsconfig) - } catch (error: any) { - debug(error) - return orig - } -} - -/** - * Convert a path from the compiled ./lib files to the ./src typescript source - * this is for developing typescript plugins/CLIs - * if there is a tsconfig and the original sources exist, it attempts to require ts-node - */ -export async function tsPath(root: string, orig: string, plugin: Plugin): Promise -export async function tsPath(root: string, orig: string | undefined, plugin?: Plugin): Promise -export async function tsPath(root: string, orig: string | undefined, plugin?: Plugin): Promise { - if (!orig) return orig - orig = orig.startsWith(root) ? orig : join(root, orig) - - const originalPath = shouldUseOriginal(root, orig, plugin) - if (originalPath) return originalPath - try { const tsconfig = await loadTSConfig(root) if (!tsconfig) return orig diff --git a/src/interfaces/plugin.ts b/src/interfaces/plugin.ts index 994a42216..28945e1a6 100644 --- a/src/interfaces/plugin.ts +++ b/src/interfaces/plugin.ts @@ -38,6 +38,7 @@ export interface Plugin { alias: string readonly commandIDs: string[] commands: Command.Loadable[] + readonly commandsDir: string | undefined findCommand(id: string, opts: {must: true}): Promise findCommand(id: string, opts?: {must: boolean}): Promise | undefined readonly hasManifest: boolean diff --git a/src/interfaces/ts-config.ts b/src/interfaces/ts-config.ts index 9d8b1ff22..33db32a4b 100644 --- a/src/interfaces/ts-config.ts +++ b/src/interfaces/ts-config.ts @@ -13,6 +13,7 @@ export interface TSConfig { sourceMap?: boolean target?: string } + extends?: string 'ts-node'?: { esm?: boolean experimentalSpecifierResolution?: 'explicit' | 'node' From 9cc9126e68582e959239665489777b32ae70e159 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 08:35:18 -0600 Subject: [PATCH 06/18] test: remove tsPathSync test --- test/config/config.flexible.test.ts | 2 + test/config/config.test.ts | 2 + test/config/ts-node.test.ts | 67 ----------------------------- 3 files changed, 4 insertions(+), 67 deletions(-) diff --git a/test/config/config.flexible.test.ts b/test/config/config.flexible.test.ts index b68162c2c..a2b0b7486 100644 --- a/test/config/config.flexible.test.ts +++ b/test/config/config.flexible.test.ts @@ -105,6 +105,7 @@ describe('Config with flexible taxonomy', () => { hasManifest: false, isRoot: false, options: {root: ''}, + commandsDir: './lib/commands', } const pluginB: IPlugin = { @@ -127,6 +128,7 @@ describe('Config with flexible taxonomy', () => { hasManifest: false, isRoot: false, options: {root: ''}, + commandsDir: './lib/commands', } const plugins = new Map().set(pluginA.name, pluginA).set(pluginB.name, pluginB) diff --git a/test/config/config.test.ts b/test/config/config.test.ts index 1fd875216..76cf0cb9f 100644 --- a/test/config/config.test.ts +++ b/test/config/config.test.ts @@ -294,6 +294,7 @@ describe('Config', () => { hasManifest: false, isRoot: false, options: {root: ''}, + commandsDir: './lib/commands', } const pluginB: IPlugin = { @@ -316,6 +317,7 @@ describe('Config', () => { hasManifest: false, isRoot: false, options: {root: ''}, + commandsDir: './lib/commands', } const plugins = new Map().set(pluginA.name, pluginA).set(pluginB.name, pluginB) let test = fancy diff --git a/test/config/ts-node.test.ts b/test/config/ts-node.test.ts index d1d946478..c583716ca 100644 --- a/test/config/ts-node.test.ts +++ b/test/config/ts-node.test.ts @@ -22,73 +22,6 @@ const DEFAULT_TS_CONFIG: Interfaces.TSConfig = { }, } -describe('tsPathSync', () => { - let sandbox: SinonSandbox - - beforeEach(() => { - sandbox = createSandbox() - sandbox.stub(util, 'existsSync').returns(true) - sandbox.stub(tsNode, 'register') - }) - - afterEach(() => { - sandbox.restore() - // Clear caches so that unit tests don't affect each other - // @ts-expect-error because TS_CONFIGS is not exported - // eslint-disable-next-line import/namespace - configTsNode.TS_CONFIGS = {} - // @ts-expect-error because REGISTERED is not exported - // eslint-disable-next-line import/namespace - configTsNode.REGISTERED = new Set() - }) - - it('should resolve a .js file to ts src', () => { - sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - const result = configTsNode.tsPathSync(root, jsCompiled) - expect(result).to.equal(join(root, tsModule)) - }) - - it('should resolve a module file to ts src', () => { - sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - const result = configTsNode.tsPathSync(root, jsCompiledModule) - expect(result).to.equal(join(root, tsModule)) - }) - - it('should resolve a .ts file', () => { - sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - const result = configTsNode.tsPathSync(root, tsSource) - expect(result).to.equal(join(root, tsSource)) - }) - - it('should resolve .js with no rootDir or outDir', () => { - sandbox.stub(util, 'readJsonSync').returns({compilerOptions: {}}) - const result = configTsNode.tsPathSync(root, jsCompiled) - expect(result).to.equal(join(root, jsCompiled)) - }) - - it('should resolve to .ts file if enabled and prod', () => { - sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - settings.tsnodeEnabled = true - const originalNodeEnv = process.env.NODE_ENV - delete process.env.NODE_ENV - - const result = configTsNode.tsPathSync(root, jsCompiled) - expect(result).to.equal(join(root, tsModule)) - - process.env.NODE_ENV = originalNodeEnv - delete settings.tsnodeEnabled - }) - - it('should resolve to js if disabled', () => { - sandbox.stub(util, 'readJsonSync').returns(JSON.stringify(DEFAULT_TS_CONFIG)) - settings.tsnodeEnabled = false - const result = configTsNode.tsPathSync(root, jsCompiled) - expect(result).to.equal(join(root, jsCompiled)) - - delete settings.tsnodeEnabled - }) -}) - describe('tsPath', () => { let sandbox: SinonSandbox From b8bd6e881bb41284180d3cd8975c3729ddd25842 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 09:18:26 -0600 Subject: [PATCH 07/18] feat: pass entire tsconfig to ts-node --- src/config/ts-node.ts | 21 ++++++------ test/integration/esm-cjs.ts | 67 +++++++++++++++++++------------------ 2 files changed, 45 insertions(+), 43 deletions(-) diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index 07d0b1992..fc136da11 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -73,32 +73,31 @@ function registerTSNode(root: string, tsconfig: TSConfig): void { } } else if (tsconfig.compilerOptions.rootDir) { rootDirs.push(join(root, tsconfig.compilerOptions.rootDir)) + } else if (tsconfig.compilerOptions.baseUrl) { + rootDirs.push(join(root, tsconfig.compilerOptions.baseUrl)) } else { rootDirs.push(join(root, 'src')) } + // Because we need to provide a modified `rootDirs` to ts-node, we need to + // remove `baseUrl` and `rootDir` from `compilerOptions` so that they + // don't conflict. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const {baseUrl, rootDir, ...rest} = tsconfig.compilerOptions + const conf: TSNode.RegisterOptions = { compilerOptions: { - emitDecoratorMetadata: tsconfig.compilerOptions.emitDecoratorMetadata ?? false, - esModuleInterop: tsconfig.compilerOptions.esModuleInterop, - experimentalDecorators: tsconfig.compilerOptions.experimentalDecorators ?? false, - module: tsconfig.compilerOptions.module ?? 'commonjs', + ...rest, rootDirs, - sourceMap: tsconfig.compilerOptions.sourceMap ?? true, - target: tsconfig.compilerOptions.target ?? 'es2019', typeRoots, - ...(tsconfig.compilerOptions.moduleResolution - ? {moduleResolution: tsconfig.compilerOptions.moduleResolution} - : {}), - ...(tsconfig.compilerOptions.jsx ? {jsx: tsconfig.compilerOptions.jsx} : {}), }, + ...tsconfig['ts-node'], cwd: root, esm: tsconfig['ts-node']?.esm ?? true, experimentalSpecifierResolution: tsconfig['ts-node']?.experimentalSpecifierResolution ?? 'explicit', scope: true, scopeDir: root, skipProject: true, - swc: tsconfig['ts-node']?.swc ?? false, transpileOnly: true, } diff --git a/test/integration/esm-cjs.ts b/test/integration/esm-cjs.ts index f642c6e48..9c6166a55 100644 --- a/test/integration/esm-cjs.ts +++ b/test/integration/esm-cjs.ts @@ -236,7 +236,10 @@ type PluginConfig = { noLinkCore: options.noLinkCore ?? false, }) - const result = await options.executor.executeCommand(`plugins:link ${pluginExecutor.pluginDir}`, options.script) + const result = await options.executor.executeCommand( + `plugins:link ${pluginExecutor.pluginDir} --no-install`, + options.script, + ) expect(result.code).to.equal(0) const pluginsResult = await options.executor.executeCommand('plugins', options.script) @@ -406,43 +409,43 @@ type PluginConfig = { } const cjsTests = async () => { - await test('Install CJS plugin to CJS root plugin', async () => { - await installTest(PLUGINS.cjs2, cjsExecutor) - }) + // await test('Install CJS plugin to CJS root plugin', async () => { + // await installTest(PLUGINS.cjs2, cjsExecutor) + // }) - await test('Install ESM plugin to CJS root plugin', async () => { - await installTest(PLUGINS.esm1, cjsExecutor) - }) + // await test('Install ESM plugin to CJS root plugin', async () => { + // await installTest(PLUGINS.esm1, cjsExecutor) + // }) await test('Link CJS plugin to CJS root plugin', async () => { await linkTest(PLUGINS.cjs2, cjsExecutor) }) - await test('Link ESM plugin to CJS root plugin', async () => { - // We don't use linkTest here because that would test that the - // ESM plugin is auto-transpiled which we're not supporting at the moment. - const plugin = PLUGINS.esm2 - - await linkPlugin({executor: cjsExecutor, plugin, script: 'run'}) - - // test bin/run - await runCommand({ - executor: cjsExecutor, - plugin, - script: 'run', - expectStrings: [plugin.commandText, plugin.hookText], - }) - - // test bin/dev - await runCommand({ - executor: cjsExecutor, - plugin, - script: 'dev', - expectStrings: [plugin.commandText, plugin.hookText], - }) - - await cleanUp({executor: cjsExecutor, plugin, script: 'run'}) - }) + // await test('Link ESM plugin to CJS root plugin', async () => { + // // We don't use linkTest here because that would test that the + // // ESM plugin is auto-transpiled which we're not supporting at the moment. + // const plugin = PLUGINS.esm2 + + // await linkPlugin({executor: cjsExecutor, plugin, script: 'run'}) + + // // test bin/run + // await runCommand({ + // executor: cjsExecutor, + // plugin, + // script: 'run', + // expectStrings: [plugin.commandText, plugin.hookText], + // }) + + // // test bin/dev + // await runCommand({ + // executor: cjsExecutor, + // plugin, + // script: 'dev', + // expectStrings: [plugin.commandText, plugin.hookText], + // }) + + // await cleanUp({executor: cjsExecutor, plugin, script: 'run'}) + // }) } const esmTests = async () => { From b025f673750a85aced3749458f5ecaae290b25b6 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 09:38:42 -0600 Subject: [PATCH 08/18] fix: handle invalid json --- src/config/ts-node.ts | 15 +++++++++------ test/config/ts-node.test.ts | 12 ++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index fc136da11..34ffecd87 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -4,7 +4,7 @@ import * as TSNode from 'ts-node' import {memoizedWarn} from '../errors' import {Plugin, TSConfig} from '../interfaces' import {settings} from '../settings' -import {existsSync, safeReadJson} from '../util/fs' +import {existsSync, readJson} from '../util/fs' import {isProd} from '../util/util' import Cache from './cache' import {Debug} from './util' @@ -19,7 +19,8 @@ async function loadTSConfig(root: string): Promise { try { if (TS_CONFIGS[root]) return TS_CONFIGS[root] const tsconfigPath = join(root, 'tsconfig.json') - const tsconfig = await safeReadJson(tsconfigPath) + const tsconfig = await readJson(tsconfigPath) + if (!tsconfig || Object.keys(tsconfig.compilerOptions).length === 0) return TS_CONFIGS[root] = tsconfig @@ -40,9 +41,11 @@ async function loadTSConfig(root: string): Promise { } return TS_CONFIGS[root] - } catch { - debug(`Could not parse tsconfig.json. Skipping ts-node registration for ${root}.`) - memoizedWarn(`Could not parse tsconfig.json for ${root}. Falling back to compiled source.`) + } catch (error) { + if (error instanceof SyntaxError) { + debug(`Could not parse tsconfig.json. Skipping ts-node registration for ${root}.`) + memoizedWarn(`Could not parse tsconfig.json for ${root}. Falling back to compiled source.`) + } } } @@ -84,11 +87,11 @@ function registerTSNode(root: string, tsconfig: TSConfig): void { // don't conflict. // eslint-disable-next-line @typescript-eslint/no-unused-vars const {baseUrl, rootDir, ...rest} = tsconfig.compilerOptions - const conf: TSNode.RegisterOptions = { compilerOptions: { ...rest, rootDirs, + // target: tsconfig.compilerOptions.target ?? 'es2019', typeRoots, }, ...tsconfig['ts-node'], diff --git a/test/config/ts-node.test.ts b/test/config/ts-node.test.ts index c583716ca..aafa534ce 100644 --- a/test/config/ts-node.test.ts +++ b/test/config/ts-node.test.ts @@ -43,31 +43,31 @@ describe('tsPath', () => { }) it('should resolve a .js file to ts src', async () => { - sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + sandbox.stub(util, 'readJson').resolves(DEFAULT_TS_CONFIG) const result = await configTsNode.tsPath(root, jsCompiled) expect(result).to.equal(join(root, tsModule)) }) it('should resolve a module file to ts src', async () => { - sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + sandbox.stub(util, 'readJson').resolves(DEFAULT_TS_CONFIG) const result = await configTsNode.tsPath(root, jsCompiledModule) expect(result).to.equal(join(root, tsModule)) }) it('should resolve a .ts file', async () => { - sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + sandbox.stub(util, 'readJson').resolves(DEFAULT_TS_CONFIG) const result = await configTsNode.tsPath(root, tsSource) expect(result).to.equal(join(root, tsSource)) }) it('should resolve .js with no rootDir or outDir', async () => { - sandbox.stub(util, 'safeReadJson').resolves({compilerOptions: {}}) + sandbox.stub(util, 'readJson').resolves({compilerOptions: {}}) const result = await configTsNode.tsPath(root, jsCompiled) expect(result).to.equal(join(root, jsCompiled)) }) it('should resolve to .ts file if enabled and prod', async () => { - sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + sandbox.stub(util, 'readJson').resolves(DEFAULT_TS_CONFIG) settings.tsnodeEnabled = true const originalNodeEnv = process.env.NODE_ENV delete process.env.NODE_ENV @@ -80,7 +80,7 @@ describe('tsPath', () => { }) it('should resolve to js if disabled', async () => { - sandbox.stub(util, 'safeReadJson').resolves(DEFAULT_TS_CONFIG) + sandbox.stub(util, 'readJson').resolves(DEFAULT_TS_CONFIG) settings.tsnodeEnabled = false const result = await configTsNode.tsPath(root, jsCompiled) expect(result).to.equal(join(root, jsCompiled)) From b64b9d7d2c29f0e91d8de2731a16d38ad444da88 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 09:44:13 -0600 Subject: [PATCH 09/18] chore: clean up --- src/config/plugin.ts | 15 ++++++--------- src/config/ts-node.ts | 19 +++++++++++-------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/config/plugin.ts b/src/config/plugin.ts index 2e660c53c..3e25aa7e2 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -121,15 +121,14 @@ export class Plugin implements IPlugin { }) const fetch = async () => { - const commandsDir = await this.getCommandsDir() - if (!commandsDir) return + if (!this.commandsDir) return let module let isESM: boolean | undefined let filePath: string | undefined try { ;({filePath, isESM, module} = cachedCommandCanBeUsed(this.manifest, id) ? await loadWithDataFromManifest(this.manifest.commands[id], this.root) - : await loadWithData(this, join(commandsDir ?? this.pjson.oclif.commands, ...id.split(':')))) + : await loadWithData(this, join(this.commandsDir ?? this.pjson.oclif.commands, ...id.split(':')))) this._debug(isESM ? '(import)' : '(require)', filePath) } catch (error: any) { if (!opts.must && error.code === 'MODULE_NOT_FOUND') return @@ -152,12 +151,11 @@ export class Plugin implements IPlugin { } public async getCommandIDs(): Promise { - const commandsDir = await this.getCommandsDir() - if (!commandsDir) return [] + if (!this.commandsDir) return [] const marker = Performance.mark(OCLIF_MARKER_OWNER, `plugin.getCommandIDs#${this.name}`, {plugin: this.name}) - this._debug(`loading IDs from ${commandsDir}`) - const files = await globby(GLOB_PATTERNS, {cwd: commandsDir}) + this._debug(`loading IDs from ${this.commandsDir}`) + const files = await globby(GLOB_PATTERNS, {cwd: this.commandsDir}) const ids = processCommandIds(files) this._debug('found commands', ids) marker?.addDetails({count: ids.length}) @@ -256,11 +254,10 @@ export class Plugin implements IPlugin { } } - const commandIDs = await this.getCommandIDs() const manifest = { commands: ( await Promise.all( - commandIDs.map(async (id) => { + this.commandIDs.map(async (id) => { try { const cached = await cacheCommand(await this.findCommand(id, {must: true}), this, respectNoCacheDefault) if (this.flexibleTaxonomy) { diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index 34ffecd87..ad215b4d6 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -49,7 +49,10 @@ async function loadTSConfig(root: string): Promise { } } -function registerTSNode(root: string, tsconfig: TSConfig): void { +async function registerTSNode(root: string): Promise { + const tsconfig = await loadTSConfig(root) + if (!tsconfig) return + if (REGISTERED.has(root)) return debug('registering ts-node at', root) const tsNodePath = require.resolve('ts-node', {paths: [root, __dirname]}) @@ -91,7 +94,6 @@ function registerTSNode(root: string, tsconfig: TSConfig): void { compilerOptions: { ...rest, rootDirs, - // target: tsconfig.compilerOptions.target ?? 'es2019', typeRoots, }, ...tsconfig['ts-node'], @@ -108,6 +110,8 @@ function registerTSNode(root: string, tsconfig: TSConfig): void { REGISTERED.add(root) debug('tsconfig: %O', tsconfig) debug('ts-node options: %O', conf) + + return tsconfig } /** @@ -150,7 +154,10 @@ function cannotUseTsNode(root: string, plugin: Plugin | undefined, isProduction: /** * Determine the path to the source file from the compiled ./lib files */ -function determinePath(root: string, orig: string, tsconfig: TSConfig): string { +async function determinePath(root: string, orig: string): Promise { + const tsconfig = await registerTSNode(root) + if (!tsconfig) return orig + debug(`determining path for ${orig}`) const {baseUrl, outDir, rootDir, rootDirs} = tsconfig.compilerOptions const rootDirPath = rootDir ?? (rootDirs ?? [])[0] ?? baseUrl @@ -243,11 +250,7 @@ export async function tsPath(root: string, orig: string | undefined, plugin?: Pl } try { - const tsconfig = await loadTSConfig(root) - if (!tsconfig) return orig - - registerTSNode(root, tsconfig) - return determinePath(root, orig, tsconfig) + return await determinePath(root, orig) } catch (error: any) { debug(error) return orig From 8904dd2b5750f21acef38da1a79fbd43d1569aeb Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 10:03:51 -0600 Subject: [PATCH 10/18] fix: return tsconfig for previously registered --- src/config/ts-node.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index ad215b4d6..0f822dba4 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -52,8 +52,8 @@ async function loadTSConfig(root: string): Promise { async function registerTSNode(root: string): Promise { const tsconfig = await loadTSConfig(root) if (!tsconfig) return + if (REGISTERED.has(root)) return tsconfig - if (REGISTERED.has(root)) return debug('registering ts-node at', root) const tsNodePath = require.resolve('ts-node', {paths: [root, __dirname]}) debug('ts-node path:', tsNodePath) From 36b660a8145506c968d0f72edae5ec4b49d54f12 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 11:10:05 -0600 Subject: [PATCH 11/18] test: code coverage --- src/errors/index.ts | 5 +++-- test/config/ts-node.test.ts | 33 +++++++++++++++++++++++++++++---- test/config/typescript.test.ts | 3 +-- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/errors/index.ts b/src/errors/index.ts index 873d61635..edb57969c 100644 --- a/src/errors/index.ts +++ b/src/errors/index.ts @@ -1,3 +1,4 @@ +import write from '../cli-ux/write' import {OclifError, PrettyPrintableError} from '../interfaces' import {config} from './config' import {CLIError, addOclifExitCode} from './errors/cli' @@ -34,7 +35,7 @@ export function error(input: Error | string, options: {exit?: false | number} & if (options.exit === false) { const message = prettyPrint(err) - console.error(message) + if (message) write.stderr(message) if (config.errorLogger) config.errorLogger.log(err?.stack ?? '') } else throw err } @@ -51,7 +52,7 @@ export function warn(input: Error | string): void { } const message = prettyPrint(err) - console.error(message) + if (message) write.stderr(message) if (config.errorLogger) config.errorLogger.log(err?.stack ?? '') } diff --git a/test/config/ts-node.test.ts b/test/config/ts-node.test.ts index aafa534ce..4ae980595 100644 --- a/test/config/ts-node.test.ts +++ b/test/config/ts-node.test.ts @@ -3,8 +3,9 @@ import {join, resolve} from 'node:path' import {SinonSandbox, createSandbox} from 'sinon' import * as tsNode from 'ts-node' -import {Interfaces, settings} from '../../src' +import write from '../../src/cli-ux/write' import * as configTsNode from '../../src/config/ts-node' +import {Interfaces, settings} from '../../src/index' import * as util from '../../src/util/fs' const root = resolve(__dirname, 'fixtures/typescript') @@ -27,7 +28,6 @@ describe('tsPath', () => { beforeEach(() => { sandbox = createSandbox() - // sandbox.stub(util, 'existsSync').returns(true) sandbox.stub(tsNode, 'register') }) @@ -60,8 +60,25 @@ describe('tsPath', () => { expect(result).to.equal(join(root, tsSource)) }) - it('should resolve .js with no rootDir or outDir', async () => { - sandbox.stub(util, 'readJson').resolves({compilerOptions: {}}) + it('should resolve a .ts file using baseUrl', async () => { + sandbox.stub(util, 'readJson').resolves({ + compilerOptions: { + baseUrl: '.src/', + outDir: 'lib', + }, + }) + const result = await configTsNode.tsPath(root, tsSource) + expect(result).to.equal(join(root, tsSource)) + }) + + it('should resolve .ts with no outDir', async () => { + sandbox.stub(util, 'readJson').resolves({compilerOptions: {rootDir: 'src'}}) + const result = await configTsNode.tsPath(root, tsSource) + expect(result).to.equal(join(root, tsSource)) + }) + + it('should resolve .js with no rootDir and outDir', async () => { + sandbox.stub(util, 'readJson').resolves({compilerOptions: {strict: true}}) const result = await configTsNode.tsPath(root, jsCompiled) expect(result).to.equal(join(root, jsCompiled)) }) @@ -87,4 +104,12 @@ describe('tsPath', () => { delete settings.tsnodeEnabled }) + + it('should handle SyntaxError', async () => { + sandbox.stub(util, 'readJson').throws(new SyntaxError('Unexpected token } in JSON at position 0')) + const stderrStub = sandbox.stub(write, 'stderr') + const result = await configTsNode.tsPath(root, tsSource) + expect(result).to.equal(join(root, tsSource)) + expect(stderrStub.firstCall.firstArg).to.include('Falling back to compiled source') + }) }) diff --git a/test/config/typescript.test.ts b/test/config/typescript.test.ts index ea06c28cc..441e9fb75 100644 --- a/test/config/typescript.test.ts +++ b/test/config/typescript.test.ts @@ -36,8 +36,7 @@ describe('typescript', () => { }) withConfig.stdout().it('runs init hook', async (ctx) => { - // to-do: fix union types - await (ctx.config.runHook as any)('init', {id: 'myid', argv: ['foo']}) + await ctx.config.runHook('init', {id: 'myid', argv: ['foo']}) expect(ctx.stdout).to.equal('running ts init hook\n') }) }) From dd7bb5fdf0e68dd19e4a1dd2f8400674762f35ee Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 11:16:17 -0600 Subject: [PATCH 12/18] test: handle line breaks --- test/config/ts-node.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/config/ts-node.test.ts b/test/config/ts-node.test.ts index 4ae980595..85a5e0bd5 100644 --- a/test/config/ts-node.test.ts +++ b/test/config/ts-node.test.ts @@ -1,6 +1,7 @@ import {expect} from 'chai' import {join, resolve} from 'node:path' import {SinonSandbox, createSandbox} from 'sinon' +import stripAnsi from 'strip-ansi' import * as tsNode from 'ts-node' import write from '../../src/cli-ux/write' @@ -110,6 +111,6 @@ describe('tsPath', () => { const stderrStub = sandbox.stub(write, 'stderr') const result = await configTsNode.tsPath(root, tsSource) expect(result).to.equal(join(root, tsSource)) - expect(stderrStub.firstCall.firstArg).to.include('Falling back to compiled source') + expect(stripAnsi(stderrStub.firstCall.firstArg).split('\n').at(-1)).to.include('Falling back to compiled source') }) }) From a01e14cc14c7dddacb45b298a4defc8286142fe7 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 11:26:51 -0600 Subject: [PATCH 13/18] test: handle line breaks --- src/errors/index.ts | 4 ++-- test/config/ts-node.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/errors/index.ts b/src/errors/index.ts index edb57969c..70643db14 100644 --- a/src/errors/index.ts +++ b/src/errors/index.ts @@ -35,7 +35,7 @@ export function error(input: Error | string, options: {exit?: false | number} & if (options.exit === false) { const message = prettyPrint(err) - if (message) write.stderr(message) + if (message) write.stderr(message + '\n') if (config.errorLogger) config.errorLogger.log(err?.stack ?? '') } else throw err } @@ -52,7 +52,7 @@ export function warn(input: Error | string): void { } const message = prettyPrint(err) - if (message) write.stderr(message) + if (message) write.stderr(message + '\n') if (config.errorLogger) config.errorLogger.log(err?.stack ?? '') } diff --git a/test/config/ts-node.test.ts b/test/config/ts-node.test.ts index 85a5e0bd5..801620a0b 100644 --- a/test/config/ts-node.test.ts +++ b/test/config/ts-node.test.ts @@ -111,6 +111,6 @@ describe('tsPath', () => { const stderrStub = sandbox.stub(write, 'stderr') const result = await configTsNode.tsPath(root, tsSource) expect(result).to.equal(join(root, tsSource)) - expect(stripAnsi(stderrStub.firstCall.firstArg).split('\n').at(-1)).to.include('Falling back to compiled source') + expect(stripAnsi(stderrStub.firstCall.firstArg).split('\n').join(' ')).to.include('Falling back to compiled source') }) }) From 980904297ff3f8200dffdf7ea261f8f9b6d9bb7b Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Mon, 30 Oct 2023 11:36:46 -0600 Subject: [PATCH 14/18] test: use different text for warning assertion --- test/config/ts-node.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/config/ts-node.test.ts b/test/config/ts-node.test.ts index 801620a0b..83ba9861b 100644 --- a/test/config/ts-node.test.ts +++ b/test/config/ts-node.test.ts @@ -111,6 +111,8 @@ describe('tsPath', () => { const stderrStub = sandbox.stub(write, 'stderr') const result = await configTsNode.tsPath(root, tsSource) expect(result).to.equal(join(root, tsSource)) - expect(stripAnsi(stderrStub.firstCall.firstArg).split('\n').join(' ')).to.include('Falling back to compiled source') + expect(stripAnsi(stderrStub.firstCall.firstArg).split('\n').join(' ')).to.include( + 'Warning: Could not parse tsconfig.json', + ) }) }) From 72e2c41a04aae65a01b394dc26df19ceafb37789 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Tue, 31 Oct 2023 17:01:43 -0600 Subject: [PATCH 15/18] chore: code review --- src/config/plugin.ts | 36 ++++++++++----------- src/config/ts-node.ts | 13 +++----- test/integration/esm-cjs.ts | 62 ++++++++++++++++++------------------- 3 files changed, 53 insertions(+), 58 deletions(-) diff --git a/src/config/plugin.ts b/src/config/plugin.ts index 3e25aa7e2..5860173eb 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -63,7 +63,7 @@ export class Plugin implements IPlugin { children: Plugin[] = [] - commandIDs!: string[] + commandIDs: string[] = [] commands!: Command.Loadable[] @@ -150,23 +150,6 @@ export class Plugin implements IPlugin { return cmd } - public async getCommandIDs(): Promise { - if (!this.commandsDir) return [] - - const marker = Performance.mark(OCLIF_MARKER_OWNER, `plugin.getCommandIDs#${this.name}`, {plugin: this.name}) - this._debug(`loading IDs from ${this.commandsDir}`) - const files = await globby(GLOB_PATTERNS, {cwd: this.commandsDir}) - const ids = processCommandIds(files) - this._debug('found commands', ids) - marker?.addDetails({count: ids.length}) - marker?.stop() - return ids - } - - public async getCommandsDir(): Promise { - return tsPath(this.root, this.pjson.oclif.commands, this) - } - public async load(): Promise { this.type = this.options.type ?? 'core' this.tag = this.options.tag @@ -304,6 +287,23 @@ export class Plugin implements IPlugin { return err } + private async getCommandIDs(): Promise { + if (!this.commandsDir) return [] + + const marker = Performance.mark(OCLIF_MARKER_OWNER, `plugin.getCommandIDs#${this.name}`, {plugin: this.name}) + this._debug(`loading IDs from ${this.commandsDir}`) + const files = await globby(GLOB_PATTERNS, {cwd: this.commandsDir}) + const ids = processCommandIds(files) + this._debug('found commands', ids) + marker?.addDetails({count: ids.length}) + marker?.stop() + return ids + } + + private async getCommandsDir(): Promise { + return tsPath(this.root, this.pjson.oclif.commands, this) + } + private warn(err: CLIError | Error | string, scope?: string): void { if (this.warned) return if (typeof err === 'string') err = new Error(err) diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index 0f822dba4..24cbc54cf 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -28,16 +28,11 @@ async function loadTSConfig(root: string): Promise { if (tsconfig.extends) { const {parse} = await import('tsconfck') const result = await parse(tsconfigPath, {}) - let tsNodeOpts = {} - for (const extended of (result.extended ?? []).reverse()) { - if (extended.tsconfig['ts-node']) { - tsNodeOpts = {...tsNodeOpts, ...extended.tsconfig['ts-node']} - } - } - - const final = {...result.tsconfig, 'ts-node': tsNodeOpts} + const tsNodeOpts = Object.fromEntries( + (result.extended ?? []).flatMap((e) => Object.entries(e.tsconfig['ts-node'] ?? {})), + ) - TS_CONFIGS[root] = final + TS_CONFIGS[root] = {...result.tsconfig, 'ts-node': tsNodeOpts} } return TS_CONFIGS[root] diff --git a/test/integration/esm-cjs.ts b/test/integration/esm-cjs.ts index 9c6166a55..7dc33f94d 100644 --- a/test/integration/esm-cjs.ts +++ b/test/integration/esm-cjs.ts @@ -409,43 +409,43 @@ type PluginConfig = { } const cjsTests = async () => { - // await test('Install CJS plugin to CJS root plugin', async () => { - // await installTest(PLUGINS.cjs2, cjsExecutor) - // }) + await test('Install CJS plugin to CJS root plugin', async () => { + await installTest(PLUGINS.cjs2, cjsExecutor) + }) - // await test('Install ESM plugin to CJS root plugin', async () => { - // await installTest(PLUGINS.esm1, cjsExecutor) - // }) + await test('Install ESM plugin to CJS root plugin', async () => { + await installTest(PLUGINS.esm1, cjsExecutor) + }) await test('Link CJS plugin to CJS root plugin', async () => { await linkTest(PLUGINS.cjs2, cjsExecutor) }) - // await test('Link ESM plugin to CJS root plugin', async () => { - // // We don't use linkTest here because that would test that the - // // ESM plugin is auto-transpiled which we're not supporting at the moment. - // const plugin = PLUGINS.esm2 - - // await linkPlugin({executor: cjsExecutor, plugin, script: 'run'}) - - // // test bin/run - // await runCommand({ - // executor: cjsExecutor, - // plugin, - // script: 'run', - // expectStrings: [plugin.commandText, plugin.hookText], - // }) - - // // test bin/dev - // await runCommand({ - // executor: cjsExecutor, - // plugin, - // script: 'dev', - // expectStrings: [plugin.commandText, plugin.hookText], - // }) - - // await cleanUp({executor: cjsExecutor, plugin, script: 'run'}) - // }) + await test('Link ESM plugin to CJS root plugin', async () => { + // We don't use linkTest here because that would test that the + // ESM plugin is auto-transpiled which we're not supporting at the moment. + const plugin = PLUGINS.esm2 + + await linkPlugin({executor: cjsExecutor, plugin, script: 'run'}) + + // test bin/run + await runCommand({ + executor: cjsExecutor, + plugin, + script: 'run', + expectStrings: [plugin.commandText, plugin.hookText], + }) + + // test bin/dev + await runCommand({ + executor: cjsExecutor, + plugin, + script: 'dev', + expectStrings: [plugin.commandText, plugin.hookText], + }) + + await cleanUp({executor: cjsExecutor, plugin, script: 'run'}) + }) } const esmTests = async () => { From ca475f5179b22efb60161d30be01483442d15c1f Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 1 Nov 2023 08:38:00 -0600 Subject: [PATCH 16/18] chore: code review --- src/config/plugin.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/config/plugin.ts b/src/config/plugin.ts index 5860173eb..21c483935 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -67,7 +67,7 @@ export class Plugin implements IPlugin { commands!: Command.Loadable[] - commandsDir!: string | undefined + commandsDir: string | undefined hasManifest = false @@ -183,11 +183,14 @@ export class Plugin implements IPlugin { this.commandsDir = await this.getCommandsDir() this.commandIDs = await this.getCommandIDs() - this.hooks = {} - for (const [k, v] of Object.entries(this.pjson.oclif.hooks ?? {})) { - // eslint-disable-next-line no-await-in-loop - this.hooks[k] = await Promise.all(castArray(v).map(async (i) => tsPath(this.root, i, this))) - } + this.hooks = Object.fromEntries( + await Promise.all( + Object.entries(this.pjson.oclif.hooks ?? {}).map(async ([k, v]) => [ + k, + await Promise.all(castArray(v).map(async (i) => tsPath(this.root, i, this))), + ]), + ), + ) this.manifest = await this._manifest() this.commands = Object.entries(this.manifest.commands) From b10562d5a2b90e99dcf39c469dbeddda7fec468a Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 1 Nov 2023 14:08:34 -0600 Subject: [PATCH 17/18] fix: find-root debug logs --- package.json | 1 + src/config/plugin.ts | 2 +- src/util/find-root.ts | 32 +++++++++++++++++++++++++++----- src/util/fs.ts | 3 --- yarn.lock | 12 ++++++++++++ 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index a9946ab2d..d2f532524 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "@types/chai-as-promised": "^7.1.5", "@types/clean-stack": "^2.1.1", "@types/cli-progress": "^3.11.0", + "@types/debug": "^4.1.10", "@types/ejs": "^3.1.3", "@types/indent-string": "^4.0.1", "@types/js-yaml": "^3.12.7", diff --git a/src/config/plugin.ts b/src/config/plugin.ts index 21c483935..46c1a2052 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -162,7 +162,7 @@ export class Plugin implements IPlugin { this.type === 'link' && !this.parent ? this.options.root : await findRoot(this.options.name, this.options.root) if (!root) throw new CLIError(`could not find package.json with ${inspect(this.options)}`) this.root = root - this._debug('reading %s plugin %s', this.type, root) + this._debug(`loading ${this.type} plugin from ${root}`) this.pjson = await readJson(join(root, 'package.json')) this.flexibleTaxonomy = this.options?.flexibleTaxonomy || this.pjson.oclif?.flexibleTaxonomy || false this.moduleType = this.pjson.type === 'module' ? 'module' : 'commonjs' diff --git a/src/util/find-root.ts b/src/util/find-root.ts index 9ee8f3c6c..82362b93c 100644 --- a/src/util/find-root.ts +++ b/src/util/find-root.ts @@ -1,11 +1,14 @@ /* eslint-disable no-await-in-loop */ import type {PackageInformation, PackageLocator, getPackageInformation} from 'pnpapi' +import makeDebug from 'debug' import {basename, dirname, join} from 'node:path' import {PJSON} from '../interfaces' import {safeReadJson} from './fs' +const debug = makeDebug('find-root') + // essentially just "cd .." function* up(from: string) { while (dirname(from) !== from) { @@ -26,7 +29,9 @@ async function findPluginRoot(root: string, name?: string) { // If we know the plugin name then we just need to traverse the file // system until we find the directory that matches the plugin name. if (name) { + debug.extend(name)(`Finding root starting at ${root}`) for (const next of up(root)) { + debug.extend(name)(`Checking ${next}`) if (next.endsWith(basename(name))) return next } } @@ -44,6 +49,7 @@ async function findPluginRoot(root: string, name?: string) { try { const cur = join(next, 'package.json') + debug.extend('root-plugin')(`Checking ${cur}`) if (await safeReadJson(cur)) return dirname(cur) } catch {} } @@ -56,6 +62,7 @@ async function findPluginRoot(root: string, name?: string) { * See https://github.com/oclif/config/pull/289#issuecomment-983904051 */ async function findRootLegacy(name: string | undefined, root: string): Promise { + debug.extend(name ?? 'root-plugin')('Finding root using legacy method') for (const next of up(root)) { let cur if (name) { @@ -106,9 +113,9 @@ const isPeerDependency = ( */ function findPnpRoot(name: string, root: string): string | undefined { maybeRequirePnpApi(root) - if (!pnp) return + debug.extend(name)('Finding root for using pnp method') const seen = new Set() const traverseDependencyTree = (locator: PackageLocator, parentPkg?: PackageInformation): string | undefined => { @@ -165,15 +172,30 @@ function findPnpRoot(name: string, root: string): string | undefined { */ export async function findRoot(name: string | undefined, root: string) { if (name) { + debug.extend(name)(`Finding root using ${root}`) let pkgPath try { pkgPath = require.resolve(name, {paths: [root]}) - } catch {} + debug.extend(name)(`Found starting point with require.resolve`) + } catch { + debug.extend(name)(`require.resolve could not find plugin starting point`) + } - if (pkgPath) return findPluginRoot(dirname(pkgPath), name) + if (pkgPath) { + const found = await findPluginRoot(dirname(pkgPath), name) + if (found) { + debug.extend(name)(`Found root at ${found}`) + return found + } + } - return process.versions.pnp ? findPnpRoot(name, root) : findRootLegacy(name, root) + const found = process.versions.pnp ? findPnpRoot(name, root) : await findRootLegacy(name, root) + debug.extend(name)(found ? `Found root at ${found}` : 'No root found!') + return found } - return findPluginRoot(root) + debug.extend('root-plugin')(`Finding root plugin using ${root}`) + const found = await findPluginRoot(root) + debug.extend('root-plugin')(found ? `Found root at ${found}` : 'No root found!') + return found } diff --git a/src/util/fs.ts b/src/util/fs.ts index ff78464cc..50e04a99f 100644 --- a/src/util/fs.ts +++ b/src/util/fs.ts @@ -2,8 +2,6 @@ import {Stats, existsSync as fsExistsSync, readFileSync} from 'node:fs' import {readFile, stat} from 'node:fs/promises' import {join} from 'node:path' -const debug = require('debug') - export function requireJson(...pathParts: string[]): T { return JSON.parse(readFileSync(join(...pathParts), 'utf8')) } @@ -51,7 +49,6 @@ export const fileExists = async (input: string): Promise => { } export async function readJson(path: string): Promise { - debug('config')('readJson %s', path) const contents = await readFile(path, 'utf8') return JSON.parse(contents) as T } diff --git a/yarn.lock b/yarn.lock index 347a5e844..f57258534 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1017,6 +1017,13 @@ resolved "https://registry.yarnpkg.com/@types/color-name/-/color-name-1.1.1.tgz#1c1261bbeaa10a8055bbc5d8ab84b7b2afc846a0" integrity sha512-rr+OQyAjxze7GgWrSaJwydHStIhHq2lvY3BOC2Mj7KnzI7XK0Uw1TOOdI9lDoajEbSWLiYgoo4f1R51erQfhPQ== +"@types/debug@^4.1.10": + version "4.1.10" + resolved "https://registry.yarnpkg.com/@types/debug/-/debug-4.1.10.tgz#f23148a6eb771a34c466a4fc28379d8101e84494" + integrity sha512-tOSCru6s732pofZ+sMv9o4o3Zc+Sa8l3bxd/tweTQudFn06vAzb13ZX46Zi6m6EJ+RUbRTHvgQJ1gBtSgkaUYA== + dependencies: + "@types/ms" "*" + "@types/ejs@^3.1.3": version "3.1.3" resolved "https://registry.yarnpkg.com/@types/ejs/-/ejs-3.1.3.tgz#ad91d1dd6e24fb60bbf96c534bce58b95eef9b57" @@ -1077,6 +1084,11 @@ resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-10.0.2.tgz#96d63314255540a36bf24da094cce7a13668d73b" integrity sha512-NaHL0+0lLNhX6d9rs+NSt97WH/gIlRHmszXbQ/8/MV/eVcFNdeJ/GYhrFuUc8K7WuPhRhTSdMkCp8VMzhUq85w== +"@types/ms@*": + version "0.7.33" + resolved "https://registry.yarnpkg.com/@types/ms/-/ms-0.7.33.tgz#80bf1da64b15f21fd8c1dc387c31929317d99ee9" + integrity sha512-AuHIyzR5Hea7ij0P9q7vx7xu4z0C28ucwjAZC0ja7JhINyCnOw8/DnvAPQQ9TfOlCtZAmCERKQX9+o1mgQhuOQ== + "@types/node-notifier@^8.0.2": version "8.0.2" resolved "https://registry.yarnpkg.com/@types/node-notifier/-/node-notifier-8.0.2.tgz#77c5de29c6e8adb915222b01864128cc3e78d553" From 2131d2a56cfcc3f2446aeaba031dea53c294057a Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 1 Nov 2023 15:43:31 -0600 Subject: [PATCH 18/18] fix: merge ts-node opts in reverse --- src/config/ts-node.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index 24cbc54cf..28d2ed7a0 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -27,9 +27,9 @@ async function loadTSConfig(root: string): Promise { if (tsconfig.extends) { const {parse} = await import('tsconfck') - const result = await parse(tsconfigPath, {}) + const result = await parse(tsconfigPath) const tsNodeOpts = Object.fromEntries( - (result.extended ?? []).flatMap((e) => Object.entries(e.tsconfig['ts-node'] ?? {})), + (result.extended ?? []).flatMap((e) => Object.entries(e.tsconfig['ts-node'] ?? {})).reverse(), ) TS_CONFIGS[root] = {...result.tsconfig, 'ts-node': tsNodeOpts}