Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: better support yarn PnP #839

Merged
merged 8 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
85 changes: 4 additions & 81 deletions src/config/plugin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable no-await-in-loop */
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'
Expand All @@ -12,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<PJSON>(__dirname, '..', '..', 'package.json')

Expand All @@ -32,83 +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 findSourcesRoot(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<PJSON>(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<string | undefined> {
for (const next of up(root)) {
let cur
if (name) {
cur = join(next, 'node_modules', name, 'package.json')
if (await safeReadJson<PJSON>(cur)) return dirname(cur)

const pkg = await safeReadJson<PJSON>(join(next, 'package.json'))
if (pkg?.name === name) return next
} else {
cur = join(next, 'package.json')
if (await safeReadJson<PJSON>(cur)) return dirname(cur)
}
}
}

async function findRoot(name: string | undefined, root: string) {
if (name) {
let pkgPath
try {
pkgPath = resolvePackage(name, {paths: [root]})
} catch {}

return pkgPath ? findSourcesRoot(dirname(pkgPath), name) : findRootLegacy(name, root)
}

return findSourcesRoot(root)
}

const cachedCommandCanBeUsed = (manifest: Manifest | undefined, id: string): boolean =>
Boolean(manifest?.commands[id] && 'isESM' in manifest.commands[id] && 'relativePath' in manifest.commands[id])

Expand Down
2 changes: 1 addition & 1 deletion src/config/ts-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,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(
Expand Down
4 changes: 0 additions & 4 deletions src/config/util.ts
Original file line number Diff line number Diff line change
@@ -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) => {
Expand Down
179 changes: 179 additions & 0 deletions src/util/find-root.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/* 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<PJSON>(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<string | undefined> {
for (const next of up(root)) {
let cur
if (name) {
cur = join(next, 'node_modules', name, 'package.json')
if (await safeReadJson<PJSON>(cur)) return dirname(cur)

const pkg = await safeReadJson<PJSON>(join(next, 'package.json'))
if (pkg?.name === name) return next
} else {
cur = join(next, 'package.json')
if (await safeReadJson<PJSON>(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
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)
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)

if (!pnp) return

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
}
Comment on lines +145 to +149
Copy link

@danieltroger danieltroger Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I referenced with

If there are multiple versions of the plugin that's trying to be found, the wrong one might be selected (root is ignored)

In #806

And I'm not sure if the issue is fixed?

Suppose that someone has two workspaces and they each contain a different version of the oclif plugin, this wouldn't necessarily take the oclif plugin in the correct workspace, but rather the first one found. This might lead to frustrating issues where someone updates their plugin version (in the workspace they have that oclif version installed in) but it doesn't do anything because oclif picks a workspace that has another version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danieltroger This might be a scenario where I prefer to wait until someone actually has this issue. Also I see this code path as a fallback for when people do not specify a main or exports, which is something we strongly advise against because it's much more performant to do a require.resolve than it is to traverse the dependency tree for a matching plugin.

So if someone were to have such an issue, I would first suggest that they define main or exports in their package.json.

}

/**
* 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)
}
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,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"
Expand Down
Loading