Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

Commit

Permalink
feat: add walk up dir lookup to satisfy local bins
Browse files Browse the repository at this point in the history
Currently @npmcli/run-script already supports this walk up lookup logic
that allows any nested folder to use a bin that is located inside a
node_modules/.bin folder higher in the directory tree.

This commit adds the same logic from:
https://github.com/npm/run-script/blob/47a4d539fb07220e7215cc0e482683b76407ef9b/lib/set-path.js#L24-L28
to libnpmexec so that users may use a binary from a dependency of a
nested workspace in the context of that workspaces' folder.

Fixes: npm/cli#2826

PR-URL: #2
Credit: @ruyadorno
Close: #2
Reviewed-by: @isaacs
  • Loading branch information
ruyadorno committed Apr 30, 2021
1 parent 52607bf commit 4876c76
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 12 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ await libexec({
- `call`: An alternative command to run when using `packages` option **String**, defaults to empty string.
- `cache`: The path location to where the npm cache folder is placed **String**
- `color`: Output should use color? **Boolean**, defaults to `false`
- `localBin`: Location to the `node_modules/.bin` folder of the local project **String**, defaults to empty string.
- `localBin`: Location to the `node_modules/.bin` folder of the local project to start scanning for bin files **String**, defaults to `./node_modules/.bin`. **libexec** will walk up the directory structure looking for `node_modules/.bin` folders in parent folders that might satisfy the current `arg` and will use that bin if found.
- `locationMsg`: Overrides "at location" message when entering interactive mode **String**
- `log`: Sets an optional logger **Object**, defaults to `proc-log` module usage.
- `globalBin`: Location to the global space bin folder, same as: `$(npm bin -g)` **String**, defaults to empty string.
Expand Down
29 changes: 29 additions & 0 deletions lib/file-exists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const { resolve } = require('path')
const { promisify } = require('util')
const stat = promisify(require('fs').stat)
const walkUp = require('walk-up-path')

const fileExists = (file) => stat(file)
.then((stat) => stat.isFile())
.catch(() => false)

const localFileExists = async (dir, binName, root = '/') => {
root = resolve(root).toLowerCase()

for (const path of walkUp(resolve(dir))) {
const binDir = resolve(path, 'node_modules', '.bin')

if (await fileExists(resolve(binDir, binName)))
return binDir

if (path.toLowerCase() === root)
return false
}

return false
}

module.exports = {
fileExists,
localFileExists,
}
16 changes: 7 additions & 9 deletions lib/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const { delimiter, resolve } = require('path')
const { delimiter, dirname, resolve } = require('path')
const { promisify } = require('util')
const read = promisify(require('read'))
const stat = promisify(require('fs').stat)

const Arborist = require('@npmcli/arborist')
const ciDetect = require('@npmcli/ci-detect')
Expand All @@ -12,15 +11,12 @@ const pacote = require('pacote')
const readPackageJson = require('read-package-json-fast')

const cacheInstallDir = require('./cache-install-dir.js')
const { fileExists, localFileExists } = require('./file-exists.js')
const getBinFromManifest = require('./get-bin-from-manifest.js')
const manifestMissing = require('./manifest-missing.js')
const noTTY = require('./no-tty.js')
const runScript = require('./run-script.js')

const fileExists = (file) => stat(file)
.then((stat) => stat.isFile())
.catch(() => false)

/* istanbul ignore next */
const PATH = (
process.env.PATH || process.env.Path || process.env.path
Expand All @@ -31,7 +27,7 @@ const exec = async (opts) => {
args = [],
call = '',
color = false,
localBin = '',
localBin = resolve('./node_modules/.bin'),
locationMsg = undefined,
globalBin = '',
output,
Expand Down Expand Up @@ -72,8 +68,10 @@ const exec = async (opts) => {
// the behavior of treating the single argument as a package name
if (needPackageCommandSwap) {
let binExists = false
if (await fileExists(`${localBin}/${args[0]}`)) {
pathArr.unshift(localBin)
const dir = dirname(dirname(localBin))
const localBinPath = await localFileExists(dir, args[0])
if (localBinPath) {
pathArr.unshift(localBinPath)
binExists = true
} else if (await fileExists(`${globalBin}/${args[0]}`)) {
pathArr.unshift(globalBin)
Expand Down
3 changes: 2 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"pacote": "^11.3.1",
"proc-log": "^1.0.0",
"read": "^1.0.7",
"read-package-json-fast": "^2.0.2"
"read-package-json-fast": "^2.0.2",
"walk-up-path": "^1.0.0"
}
}
14 changes: 14 additions & 0 deletions test/file-exists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const t = require('tap')
const { localFileExists } = require('../lib/file-exists.js')

t.test('missing root value', async t => {
const dir = t.testdir({
b: {
c: {},
},
})

// root value a is not part of the file system hierarchy
const fileExists = await localFileExists(dir, 'foo', 'a')
t.equal(fileExists, false, 'should return false on missing root')
})
74 changes: 74 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,77 @@ t.test('sane defaults', async t => {
t.ok(fs.statSync(resolve(workdir, 'index.js')).isFile(),
'ran create-index pkg')
})

t.only('workspaces', async t => {
const pkg = {
name: '@ruyadorno/create-index',
version: '2.0.0',
bin: {
'create-index': './index.js',
},
}
const path = t.testdir({
cache: {},
node_modules: {
'.bin': {},
'@ruyadorno': {
'create-index': {
'package.json': JSON.stringify(pkg),
'index.js': `#!/usr/bin/env node
require('fs').writeFileSync('resfile', 'LOCAL PKG')`,
},
},
a: t.fixture('symlink', '../a'),
},
'package.json': JSON.stringify({
name: 'project',
workspaces: ['a'],
}),
a: {
'package.json': JSON.stringify({
name: 'a',
version: '1.0.0',
dependencies: {
'@ruyadorno/create-index': '^2.0.0',
},
}),
}
})
const runPath = path
const cache = resolve(path, 'cache')

const executable =
resolve(path, 'node_modules/@ruyadorno/create-index/index.js')
fs.chmodSync(executable, 0o775)

await binLinks({
path: resolve(path, 'node_modules/@ruyadorno/create-index'),
pkg,
})

// runs at the project level
await libexec({
...baseOpts,
args: ['create-index'],
localBin: resolve(path, 'node_modules/.bin'),
cache,
path,
runPath,
})

const res = fs.readFileSync(resolve(path, 'resfile')).toString()
t.equal(res, 'LOCAL PKG', 'should run existing bin from project level')

// runs at the child workspace level
await libexec({
...baseOpts,
args: ['create-index'],
cache,
localBin: resolve(path, 'a/node_modules/.bin'),
path: resolve(path, 'a'),
runPath: resolve(path, 'a'),
})

const wRes = fs.readFileSync(resolve(path, 'a/resfile')).toString()
t.equal(wRes, 'LOCAL PKG', 'should run existing bin from workspace level')
})

0 comments on commit 4876c76

Please sign in to comment.