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

fix(exec): look in workspace and root for bin entries #7569

Merged
merged 1 commit into from
May 29, 2024
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
22 changes: 14 additions & 8 deletions lib/commands/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,8 @@ class Exec extends BaseCommand {
}

async callExec (args, { name, locationMsg, runPath } = {}) {
// This is where libnpmexec will look for locally installed packages at the project level
const localPrefix = this.npm.localPrefix
// This is where libnpmexec will look for locally installed packages at the workspace level
let localBin = this.npm.localBin
let path = localPrefix
let pkgPath = this.npm.localPrefix

// This is where libnpmexec will actually run the scripts from
if (!runPath) {
Expand All @@ -54,7 +51,7 @@ class Exec extends BaseCommand {
localBin = resolve(this.npm.localDir, name, 'node_modules', '.bin')
// We also need to look for `bin` entries in the workspace package.json
// libnpmexec will NOT look in the project root for the bin entry
path = runPath
pkgPath = runPath

This comment was marked as spam.

}

const call = this.npm.config.get('call')
Expand Down Expand Up @@ -84,16 +81,25 @@ class Exec extends BaseCommand {
// we explicitly set packageLockOnly to false because if it's true
// when we try to install a missing package, we won't actually install it
packageLockOnly: false,
// copy args so they dont get mutated
args: [...args],
// what the user asked to run args[0] is run by default
args: [...args], // copy args so they dont get mutated
// specify a custom command to be run instead of args[0]
call,
chalk,
// where to look for bins globally, if a file matches call or args[0] it is called
globalBin,
// where to look for packages globally, if a package matches call or args[0] it is called
globalPath,
// where to look for bins locally, if a file matches call or args[0] it is called
localBin,
locationMsg,
// packages that need to be installed
packages,
path,
// path where node_modules is
path: this.npm.localPrefix,
// where to look for package.json#bin entries first
pkgPath,
// cwd to run from
runPath,
scriptShell,
yes,
Expand Down
49 changes: 32 additions & 17 deletions workspaces/libnpmexec/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const getBinFromManifest = require('./get-bin-from-manifest.js')
const noTTY = require('./no-tty.js')
const runScript = require('./run-script.js')
const isWindows = require('./is-windows.js')
const { dirname, resolve } = require('path')
const { dirname, resolve } = require('node:path')

const binPaths = []

Expand Down Expand Up @@ -73,6 +73,11 @@ const missingFromTree = async ({ spec, tree, flatOptions, isNpxTree }) => {
}
}

// see if the package.json at `path` has an entry that matches `cmd`
const hasPkgBin = (path, cmd, flatOptions) =>
pacote.manifest(path, flatOptions)
.then(manifest => manifest?.bin?.[cmd]).catch(() => null)

const exec = async (opts) => {
const {
args = [],
Expand All @@ -89,6 +94,13 @@ const exec = async (opts) => {
...flatOptions
} = opts

let pkgPaths = opts.pkgPath
if (typeof pkgPaths === 'string') {
pkgPaths = [pkgPaths]
}
if (!pkgPaths) {
pkgPaths = ['.']
}
let yes = opts.yes
const run = () => runScript({
args,
Expand All @@ -106,28 +118,31 @@ const exec = async (opts) => {
return run()
}

// Look in the local tree too
pkgPaths.push(path)

let needPackageCommandSwap = (args.length > 0) && (packages.length === 0)
// If they asked for a command w/o specifying a package, see if there is a
// bin that directly matches that name:
// - in the local package itself
// - in the local tree
// - in any local packages (pkgPaths can have workspaces in them or just the root)
// - in the local tree (path)
// - globally
if (needPackageCommandSwap) {
let localManifest
try {
localManifest = await pacote.manifest(path, flatOptions)
} catch {
// no local package.json? no problem, move one.
// Local packages and local tree
for (const p of pkgPaths) {
if (await hasPkgBin(p, args[0], flatOptions)) {
// we have to install the local package into the npx cache so that its
// bin links get set up
flatOptions.installLinks = false
// args[0] will exist when the package is installed
packages.push(p)
yes = true
needPackageCommandSwap = false
break
}
}
if (localManifest?.bin?.[args[0]]) {
// we have to install the local package into the npx cache so that its
// bin links get set up
flatOptions.installLinks = false
// args[0] will exist when the package is installed
packages.push(path)
yes = true
needPackageCommandSwap = false
} else {
if (needPackageCommandSwap) {
// no bin entry in local packages or in tree, now we look for binPaths
const dir = dirname(dirname(localBin))
const localBinPath = await localFileExists(dir, args[0], '/')
if (localBinPath) {
Expand Down
2 changes: 1 addition & 1 deletion workspaces/libnpmexec/test/local.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ t.test('bin in local pkg', async t => {
await binLinks(existingPkg.pkg)

t.match(await fs.readdir(resolve(path, 'node_modules', '.bin')), ['conflicting-bin'])
await exec({ localBin, args: ['conflicting-bin'] })
await exec({ pkgPath: path, localBin, args: ['conflicting-bin'] })
// local bin was called for conflicting-bin
t.match(await readOutput('conflicting-bin'), {
value: 'LOCAL PKG',
Expand Down
Loading