From 8e217a6b9a0d5824113f0c68c9be009440331f23 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Wed, 18 Oct 2023 14:30:48 -0600 Subject: [PATCH 1/5] feat: support pnp plugins --- package.json | 1 + src/config/plugin.ts | 72 ++++++++++++++++++++++++++++++++++++++++++++ yarn.lock | 5 +++ 3 files changed, 78 insertions(+) diff --git a/package.json b/package.json index 048a6ebd2..6c21a8438 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "@types/mocha": "^10.0.2", "@types/node": "^18", "@types/node-notifier": "^8.0.2", + "@types/pnpapi": "^0.0.4", "@types/slice-ansi": "^4.0.0", "@types/strip-ansi": "^5.2.1", "@types/supports-color": "^8.1.1", diff --git a/src/config/plugin.ts b/src/config/plugin.ts index 12be13015..eccd3f35f 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -1,5 +1,8 @@ /* eslint-disable no-await-in-loop */ +import type {PackageInformation, PackageLocator, getPackageInformation} from 'pnpapi' + import {sync} from 'globby' +import {createRequire} from 'node:module' import {basename, dirname, join, parse, relative, sep} from 'node:path' import {inspect} from 'node:util' @@ -96,7 +99,76 @@ async function findRootLegacy(name: string | undefined, root: string): Promise PackageLocator + getDependencyTreeRoots: () => PackageLocator[] +} + +function maybeRequirePnpApi(root: string): unknown { + if (pnp) return pnp + const otherModuleRequire = createRequire(root) + pnp = otherModuleRequire('pnpapi') + return pnp +} + +const getKey = (locator: PackageLocator | string | [string, string] | undefined) => JSON.stringify(locator) +const isPeerDependency = ( + pkg: PackageInformation | undefined, + parentPkg: PackageInformation | undefined, + name: string, +) => getKey(pkg?.packageDependencies.get(name)) === getKey(parentPkg?.packageDependencies.get(name)) + +/** + * Traverse PnP dependency tree to find package root + * + * Implementation taken from https://yarnpkg.com/advanced/pnpapi#traversing-the-dependency-tree + */ +function findPackageWithYarn(name: string, root: string): string | undefined { + maybeRequirePnpApi(root) + const seen = new Set() + + const traverseDependencyTree = (locator: PackageLocator, parentPkg?: PackageInformation): string | undefined => { + // Prevent infinite recursion when A depends on B which depends on A + const key = getKey(locator) + if (seen.has(key)) return + + const pkg = pnp.getPackageInformation(locator) + + if (locator.name === name) { + return pkg.packageLocation + } + + seen.add(key) + + for (const [name, referencish] of pkg.packageDependencies) { + // Unmet peer dependencies + if (referencish === null) continue + + // Avoid iterating on peer dependencies - very expensive + if (parentPkg !== null && isPeerDependency(pkg, parentPkg, name)) continue + + const childLocator = pnp.getLocator(name, referencish) + const foundSomething = traverseDependencyTree(childLocator, pkg) + if (foundSomething) return foundSomething + } + + // Important: This `delete` here causes the traversal to go over nodes even + // if they have already been traversed in another branch. If you don't need + // that, remove this line for a hefty speed increase. + seen.delete(key) + } + + // Iterate on each workspace + for (const locator of pnp.getDependencyTreeRoots()) { + const foundSomething = traverseDependencyTree(locator) + if (foundSomething) return foundSomething + } +} + async function findRoot(name: string | undefined, root: string) { + if (process.versions.pnp && name) return findPackageWithYarn(name, root) + if (name) { let pkgPath try { diff --git a/yarn.lock b/yarn.lock index df6c932f7..db106b39d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1133,6 +1133,11 @@ resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.1.tgz#d3357479a0fdfdd5907fe67e17e0a85c906e1301" integrity sha512-Gj7cI7z+98M282Tqmp2K5EIsoouUEzbBJhQQzDE3jSIRk6r9gsz0oUokqIUR4u1R3dMHo0pDHM7sNOHyhulypw== +"@types/pnpapi@^0.0.4": + version "0.0.4" + resolved "https://registry.yarnpkg.com/@types/pnpapi/-/pnpapi-0.0.4.tgz#8e9efb1f898246e120d9546c3407a20080576133" + integrity sha512-GWOPbNkJcIjb+CM2tyWI1/dwVq3pO7w+OEnjkYEIX09U7K3xkuZ2+/2JXjaeLc8uAHp2F/V2fsdRlKKvVjdXAg== + "@types/semver@^7.5.0": version "7.5.2" resolved "https://registry.yarnpkg.com/@types/semver/-/semver-7.5.2.tgz#31f6eec1ed7ec23f4f05608d3a2d381df041f564" From 914f1482643988de4ddd1952aebb8e207fe3d8a5 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 19 Oct 2023 12:52:30 -0600 Subject: [PATCH 2/5] refactor: improve implementation --- src/config/plugin.ts | 22 +++++++++++++--------- src/config/ts-node.ts | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/config/plugin.ts b/src/config/plugin.ts index eccd3f35f..e9f833748 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -2,7 +2,6 @@ import type {PackageInformation, PackageLocator, getPackageInformation} from 'pnpapi' import {sync} from 'globby' -import {createRequire} from 'node:module' import {basename, dirname, join, parse, relative, sep} from 'node:path' import {inspect} from 'node:util' @@ -45,7 +44,7 @@ function* up(from: string) { yield from } -async function findSourcesRoot(root: string, name?: string) { +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) { @@ -107,8 +106,13 @@ let pnp: { function maybeRequirePnpApi(root: string): unknown { if (pnp) return pnp - const otherModuleRequire = createRequire(root) - pnp = otherModuleRequire('pnpapi') + // Require pnpapi directly from the plugin. + // The pnpapi module is only available if running in a pnp environment. + // Because of that we have to require it from the root. + // Solution taken from here: https://github.com/yarnpkg/berry/issues/1467#issuecomment-642869600 + // eslint-disable-next-line node/no-missing-require + pnp = require(require.resolve('pnpapi', {paths: [root]})) + return pnp } @@ -124,7 +128,7 @@ const isPeerDependency = ( * * Implementation taken from https://yarnpkg.com/advanced/pnpapi#traversing-the-dependency-tree */ -function findPackageWithYarn(name: string, root: string): string | undefined { +function findPnpRoot(name: string, root: string): string | undefined { maybeRequirePnpApi(root) const seen = new Set() @@ -167,18 +171,18 @@ function findPackageWithYarn(name: string, root: string): string | undefined { } async function findRoot(name: string | undefined, root: string) { - if (process.versions.pnp && name) return findPackageWithYarn(name, root) - if (name) { let pkgPath try { pkgPath = resolvePackage(name, {paths: [root]}) } catch {} - return pkgPath ? findSourcesRoot(dirname(pkgPath), name) : findRootLegacy(name, root) + if (pkgPath) return findPluginRoot(dirname(pkgPath), name) + + return process.versions.pnp ? findPnpRoot(name, root) : findRootLegacy(name, root) } - return findSourcesRoot(root) + return findPluginRoot(root) } const cachedCommandCanBeUsed = (manifest: Manifest | undefined, id: string): boolean => diff --git a/src/config/ts-node.ts b/src/config/ts-node.ts index 7a0328350..4630a1fdd 100644 --- a/src/config/ts-node.ts +++ b/src/config/ts-node.ts @@ -24,7 +24,7 @@ function loadTSConfig(root: string): TSConfig | undefined { typescript = require('typescript') } catch { try { - typescript = require(join(root, 'node_modules', 'typescript')) + typescript = require(require.resolve('typescript', {paths: [root, __dirname]})) } catch { debug(`Could not find typescript dependency. Skipping ts-node registration for ${root}.`) memoizedWarn( From c5d87fc9bb30973fb055143c92df7bc27de16554 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 19 Oct 2023 13:34:50 -0600 Subject: [PATCH 3/5] feat: better support yarn PnP --- src/config/plugin.ts | 161 +------------------------------------- src/config/util.ts | 4 - src/util/find-root.ts | 174 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 178 insertions(+), 161 deletions(-) create mode 100644 src/util/find-root.ts diff --git a/src/config/plugin.ts b/src/config/plugin.ts index e9f833748..183ce2e1d 100644 --- a/src/config/plugin.ts +++ b/src/config/plugin.ts @@ -1,8 +1,5 @@ -/* eslint-disable no-await-in-loop */ -import type {PackageInformation, PackageLocator, getPackageInformation} from 'pnpapi' - import {sync} from 'globby' -import {basename, dirname, join, parse, relative, sep} from 'node:path' +import {join, parse, relative, sep} from 'node:path' import {inspect} from 'node:util' import {Command} from '../command' @@ -14,10 +11,11 @@ import {Topic} from '../interfaces/topic' import {loadWithData, loadWithDataFromManifest} from '../module-loader' import {OCLIF_MARKER_OWNER, Performance} from '../performance' import {cacheCommand} from '../util/cache-command' -import {readJson, requireJson, safeReadJson} from '../util/fs' +import {findRoot} from '../util/find-root' +import {readJson, requireJson} from '../util/fs' import {compact, isProd, mapValues} from '../util/util' import {tsPath} from './ts-node' -import {Debug, getCommandIdPermutations, resolvePackage} from './util' +import {Debug, getCommandIdPermutations} from './util' const _pjson = requireJson(__dirname, '..', '..', 'package.json') @@ -34,157 +32,6 @@ function topicsToArray(input: any, base?: string): Topic[] { }) } -// essentially just "cd .." -function* up(from: string) { - while (dirname(from) !== from) { - yield from - from = dirname(from) - } - - yield from -} - -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) { - for (const next of up(root)) { - if (next.endsWith(basename(name))) return next - } - } - - // If there's no plugin name (typically just the root plugin), then we need - // to traverse the file system until we find a directory with a package.json - for (const next of up(root)) { - // Skip the bin directory - if ( - basename(dirname(next)) === 'bin' && - ['dev', 'dev.cmd', 'dev.js', 'run', 'run.cmd', 'run.js'].includes(basename(next)) - ) { - continue - } - - try { - const cur = join(next, 'package.json') - if (await safeReadJson(cur)) return dirname(cur) - } catch {} - } -} - -/** - * Find package root for packages installed into node_modules. This will go up directories - * until it finds a node_modules directory with the plugin installed into it - * - * This is needed because some oclif plugins do not declare the `main` field in their package.json - * https://github.com/oclif/config/pull/289#issuecomment-983904051 - * - * @returns string - * @param name string - * @param root string - */ -async function findRootLegacy(name: string | undefined, root: string): Promise { - for (const next of up(root)) { - let cur - if (name) { - cur = join(next, 'node_modules', name, 'package.json') - if (await safeReadJson(cur)) return dirname(cur) - - const pkg = await safeReadJson(join(next, 'package.json')) - if (pkg?.name === name) return next - } else { - cur = join(next, 'package.json') - if (await safeReadJson(cur)) return dirname(cur) - } - } -} - -let pnp: { - getPackageInformation: typeof getPackageInformation - getLocator: (name: string, reference: string | [string, string]) => PackageLocator - getDependencyTreeRoots: () => PackageLocator[] -} - -function maybeRequirePnpApi(root: string): unknown { - if (pnp) return pnp - // Require pnpapi directly from the plugin. - // The pnpapi module is only available if running in a pnp environment. - // Because of that we have to require it from the root. - // Solution taken from here: https://github.com/yarnpkg/berry/issues/1467#issuecomment-642869600 - // eslint-disable-next-line node/no-missing-require - pnp = require(require.resolve('pnpapi', {paths: [root]})) - - return pnp -} - -const getKey = (locator: PackageLocator | string | [string, string] | undefined) => JSON.stringify(locator) -const isPeerDependency = ( - pkg: PackageInformation | undefined, - parentPkg: PackageInformation | undefined, - name: string, -) => getKey(pkg?.packageDependencies.get(name)) === getKey(parentPkg?.packageDependencies.get(name)) - -/** - * Traverse PnP dependency tree to find package root - * - * Implementation taken from https://yarnpkg.com/advanced/pnpapi#traversing-the-dependency-tree - */ -function findPnpRoot(name: string, root: string): string | undefined { - maybeRequirePnpApi(root) - const seen = new Set() - - const traverseDependencyTree = (locator: PackageLocator, parentPkg?: PackageInformation): string | undefined => { - // Prevent infinite recursion when A depends on B which depends on A - const key = getKey(locator) - if (seen.has(key)) return - - const pkg = pnp.getPackageInformation(locator) - - if (locator.name === name) { - return pkg.packageLocation - } - - seen.add(key) - - for (const [name, referencish] of pkg.packageDependencies) { - // Unmet peer dependencies - if (referencish === null) continue - - // Avoid iterating on peer dependencies - very expensive - if (parentPkg !== null && isPeerDependency(pkg, parentPkg, name)) continue - - const childLocator = pnp.getLocator(name, referencish) - const foundSomething = traverseDependencyTree(childLocator, pkg) - if (foundSomething) return foundSomething - } - - // Important: This `delete` here causes the traversal to go over nodes even - // if they have already been traversed in another branch. If you don't need - // that, remove this line for a hefty speed increase. - seen.delete(key) - } - - // Iterate on each workspace - for (const locator of pnp.getDependencyTreeRoots()) { - const foundSomething = traverseDependencyTree(locator) - if (foundSomething) return foundSomething - } -} - -async function findRoot(name: string | undefined, root: string) { - if (name) { - let pkgPath - try { - pkgPath = resolvePackage(name, {paths: [root]}) - } catch {} - - if (pkgPath) return findPluginRoot(dirname(pkgPath), name) - - return process.versions.pnp ? findPnpRoot(name, root) : findRootLegacy(name, root) - } - - return findPluginRoot(root) -} - const cachedCommandCanBeUsed = (manifest: Manifest | undefined, id: string): boolean => Boolean(manifest?.commands[id] && 'isESM' in manifest.commands[id] && 'relativePath' in manifest.commands[id]) diff --git a/src/config/util.ts b/src/config/util.ts index bbcb2bed6..77b33da03 100644 --- a/src/config/util.ts +++ b/src/config/util.ts @@ -1,9 +1,5 @@ const debug = require('debug') -export function resolvePackage(id: string, paths: {paths: string[]}): string { - return require.resolve(id, paths) -} - function displayWarnings() { if (process.listenerCount('warning') > 1) return process.on('warning', (warning: any) => { diff --git a/src/util/find-root.ts b/src/util/find-root.ts new file mode 100644 index 000000000..e8e5542fc --- /dev/null +++ b/src/util/find-root.ts @@ -0,0 +1,174 @@ +/* eslint-disable no-await-in-loop */ +import type {PackageInformation, PackageLocator, getPackageInformation} from 'pnpapi' + +import {basename, dirname, join} from 'node:path' + +import {PJSON} from '../interfaces' +import {safeReadJson} from './fs' + +// essentially just "cd .." +function* up(from: string) { + while (dirname(from) !== from) { + yield from + from = dirname(from) + } + + yield from +} + +/** + * Return the plugin root directory from a given file. This will `cd` up the file system until it finds + * a package.json and then return the dirname of that path. + * + * Example: node_modules/@oclif/plugin-version/dist/index.js -> node_modules/@oclif/plugin-version + */ +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) { + for (const next of up(root)) { + if (next.endsWith(basename(name))) return next + } + } + + // If there's no plugin name (typically just the root plugin), then we need + // to traverse the file system until we find a directory with a package.json + for (const next of up(root)) { + // Skip the bin directory + if ( + basename(dirname(next)) === 'bin' && + ['dev', 'dev.cmd', 'dev.js', 'run', 'run.cmd', 'run.js'].includes(basename(next)) + ) { + continue + } + + try { + const cur = join(next, 'package.json') + if (await safeReadJson(cur)) return dirname(cur) + } catch {} + } +} + +/** + * Find plugin root directory for plugins installed into node_modules that don't have a `main` or `export`. + * This will go up directories until it finds a directory with the plugin installed into it. + * + * See https://github.com/oclif/config/pull/289#issuecomment-983904051 + */ +async function findRootLegacy(name: string | undefined, root: string): Promise { + for (const next of up(root)) { + let cur + if (name) { + cur = join(next, 'node_modules', name, 'package.json') + if (await safeReadJson(cur)) return dirname(cur) + + const pkg = await safeReadJson(join(next, 'package.json')) + if (pkg?.name === name) return next + } else { + cur = join(next, 'package.json') + if (await safeReadJson(cur)) return dirname(cur) + } + } +} + +let pnp: { + getPackageInformation: typeof getPackageInformation + getLocator: (name: string, reference: string | [string, string]) => PackageLocator + getDependencyTreeRoots: () => PackageLocator[] +} + +/** + * The pnpapi module is only available if running in a pnp environment. Because of that + * we have to require it from the plugin. + * + * Solution taken from here: https://github.com/yarnpkg/berry/issues/1467#issuecomment-642869600 + */ +function maybeRequirePnpApi(root: string): unknown { + if (pnp) return pnp + // eslint-disable-next-line node/no-missing-require + pnp = require(require.resolve('pnpapi', {paths: [root]})) + return pnp +} + +const getKey = (locator: PackageLocator | string | [string, string] | undefined) => JSON.stringify(locator) +const isPeerDependency = ( + pkg: PackageInformation | undefined, + parentPkg: PackageInformation | undefined, + name: string, +) => getKey(pkg?.packageDependencies.get(name)) === getKey(parentPkg?.packageDependencies.get(name)) + +/** + * Traverse PnP dependency tree to find plugin root directory. + * + * Implementation adapted from https://yarnpkg.com/advanced/pnpapi#traversing-the-dependency-tree + */ +function findPnpRoot(name: string, root: string): string | undefined { + maybeRequirePnpApi(root) + const seen = new Set() + + const traverseDependencyTree = (locator: PackageLocator, parentPkg?: PackageInformation): string | undefined => { + // Prevent infinite recursion when A depends on B which depends on A + const key = getKey(locator) + if (seen.has(key)) return + + const pkg = pnp.getPackageInformation(locator) + + if (locator.name === name) { + return pkg.packageLocation + } + + seen.add(key) + + for (const [name, referencish] of pkg.packageDependencies) { + // Unmet peer dependencies + if (referencish === null) continue + + // Avoid iterating on peer dependencies - very expensive + if (parentPkg !== null && isPeerDependency(pkg, parentPkg, name)) continue + + const childLocator = pnp.getLocator(name, referencish) + const foundSomething = traverseDependencyTree(childLocator, pkg) + if (foundSomething) return foundSomething + } + + // Important: This `delete` here causes the traversal to go over nodes even + // if they have already been traversed in another branch. If you don't need + // that, remove this line for a hefty speed increase. + seen.delete(key) + } + + // Iterate on each workspace + for (const locator of pnp.getDependencyTreeRoots()) { + const foundSomething = traverseDependencyTree(locator) + if (foundSomething) return foundSomething + } +} + +/** + * Returns the root directory of the plugin. + * + * It first attempts to use require.resolve to find the plugin root. + * If that returns a path, it will `cd` up the file system until if finds the package.json for the plugin + * Example: node_modules/@oclif/plugin-version/dist/index.js -> node_modules/@oclif/plugin-version + * + * If require.resolve throws an error, it will attempt to find the plugin root by traversing the file system. + * If we're in a PnP environment (determined by process.versions.pnp), it will use the pnpapi module to + * traverse the dependency tree. Otherwise, it will traverse the node_modules until it finds a package.json + * with a matching name. + * + * If no path is found, undefined is returned which will eventually result in a thrown Error from Plugin. + */ +export async function findRoot(name: string | undefined, root: string) { + if (name) { + let pkgPath + try { + pkgPath = require.resolve(name, {paths: [root]}) + } catch {} + + if (pkgPath) return findPluginRoot(dirname(pkgPath), name) + + return process.versions.pnp ? findPnpRoot(name, root) : findRootLegacy(name, root) + } + + return findPluginRoot(root) +} From 6baa39fcc2c03b80eddd43b411f8c42d5f3ab743 Mon Sep 17 00:00:00 2001 From: svc-cli-bot Date: Thu, 19 Oct 2023 21:00:51 +0000 Subject: [PATCH 4/5] chore(release): 3.4.1-dev.0 [skip ci] --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d5ee8d1a9..67367f59a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@oclif/core", "description": "base library for oclif CLIs", - "version": "3.4.0", + "version": "3.4.1-dev.0", "author": "Salesforce", "bugs": "https://github.com/oclif/core/issues", "dependencies": { From c2fc8d2cfe4360602ad8be9ae5c4fe1947c9ef27 Mon Sep 17 00:00:00 2001 From: Mike Donnalley Date: Thu, 19 Oct 2023 15:37:05 -0600 Subject: [PATCH 5/5] fix: safely fail if pnpapi cannot be found --- src/util/find-root.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/util/find-root.ts b/src/util/find-root.ts index e8e5542fc..9ee8f3c6c 100644 --- a/src/util/find-root.ts +++ b/src/util/find-root.ts @@ -85,9 +85,11 @@ let pnp: { */ function maybeRequirePnpApi(root: string): unknown { if (pnp) return pnp - // eslint-disable-next-line node/no-missing-require - pnp = require(require.resolve('pnpapi', {paths: [root]})) - return pnp + try { + // eslint-disable-next-line node/no-missing-require + pnp = require(require.resolve('pnpapi', {paths: [root]})) + return pnp + } catch {} } const getKey = (locator: PackageLocator | string | [string, string] | undefined) => JSON.stringify(locator) @@ -104,6 +106,9 @@ const isPeerDependency = ( */ function findPnpRoot(name: string, root: string): string | undefined { maybeRequirePnpApi(root) + + if (!pnp) return + const seen = new Set() const traverseDependencyTree = (locator: PackageLocator, parentPkg?: PackageInformation): string | undefined => {