Skip to content

Commit 501e738

Browse files
committed
fix(npx): link and run own bins directly when possible
This follows a similar approach to local and global bins that already exist. This is achieved by linking a packages own bin script to `node_modules/.bin` and the package within `node_modules/` similar to if it had been installed with `--install-links=false`. Then the linked bin is run and the symlinks are cleaned up immediately after. This has the same effect as loading the current package into the `npx` cache and running it, except it is quicker to not have to run any Arborist commands.
1 parent 2939744 commit 501e738

File tree

3 files changed

+61
-26
lines changed

3 files changed

+61
-26
lines changed

package-lock.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15105,6 +15105,7 @@
1510515105
"@npmcli/arborist": "^6.1.2",
1510615106
"@npmcli/ci-detect": "^3.0.1",
1510715107
"@npmcli/run-script": "^6.0.0",
15108+
"bin-links": "^4.0.1",
1510815109
"chalk": "^4.1.0",
1510915110
"npm-package-arg": "^10.0.0",
1511015111
"npmlog": "^7.0.1",
@@ -15118,7 +15119,6 @@
1511815119
"devDependencies": {
1511915120
"@npmcli/eslint-config": "^4.0.0",
1512015121
"@npmcli/template-oss": "4.10.0",
15121-
"bin-links": "^4.0.1",
1512215122
"minify-registry-metadata": "^2.2.0",
1512315123
"mkdirp": "^1.0.4",
1512415124
"tap": "^16.0.1"

workspaces/libnpmexec/lib/index.js

Lines changed: 59 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict'
22

3-
const { mkdir } = require('fs/promises')
3+
const { mkdir, symlink, unlink, rm } = require('fs/promises')
44
const { promisify } = require('util')
55

66
const Arborist = require('@npmcli/arborist')
@@ -12,6 +12,8 @@ const npmlog = require('npmlog')
1212
const pacote = require('pacote')
1313
const read = promisify(require('read'))
1414
const semver = require('semver')
15+
const binLinks = require('bin-links')
16+
binLinks.linkBins = require('bin-links/lib/link-bins')
1517

1618
const { fileExists, localFileExists } = require('./file-exists.js')
1719
const getBinFromManifest = require('./get-bin-from-manifest.js')
@@ -117,31 +119,64 @@ const exec = async (opts) => {
117119
// - in the local tree
118120
// - globally
119121
if (needPackageCommandSwap) {
120-
let localManifest
121-
try {
122-
localManifest = await pacote.manifest(path, flatOptions)
123-
} catch {
124-
// no local package.json? no problem, move one.
125-
}
126-
if (localManifest?.bin?.[args[0]]) {
127-
// we have to install the local package into the npx cache so that its
128-
// bin links get set up
129-
packages.push(path)
130-
yes = true
131-
flatOptions.installLinks = false
132-
} else {
133-
const dir = dirname(dirname(localBin))
134-
const localBinPath = await localFileExists(dir, args[0], '/')
135-
if (localBinPath) {
136-
binPaths.push(localBinPath)
137-
return await run()
138-
} else if (globalPath && await fileExists(`${globalBin}/${args[0]}`)) {
139-
binPaths.push(globalBin)
140-
return await run()
122+
const localManifest = await pacote.manifest(path, flatOptions).catch(() => null)
123+
const manifestBinPath = localManifest?.bin?.[args[0]]
124+
if (manifestBinPath && await fileExists(resolve(path, manifestBinPath))) {
125+
const tmpNmPath = resolve(path, 'node_modules', localManifest.name)
126+
const tmpDir = await mkdir(dirname(tmpNmPath), { recursive: true })
127+
await symlink(path, tmpNmPath, 'dir')
128+
129+
// only link the bin that we already know exists in case anything else would fail
130+
const binOpts = {
131+
force: false,
132+
path: tmpNmPath,
133+
pkg: { bin: { [args[0]]: manifestBinPath } },
134+
}
135+
// binLinks returns null if it was created and false if not so if we have
136+
// a valid path here then we can keep going. if we did not create a bin
137+
// here then keep trying the next steps below, since there is probably
138+
// a bin that is already linked there which we will run.
139+
const linkedBins = await binLinks.linkBins(binOpts).catch(() => []).then((r) => {
140+
return r[0] === null ? binLinks.getPaths(binOpts) : []
141+
})
142+
143+
const cleanupLinks = async () => {
144+
// always unlink symlinks when we are done
145+
await unlink(tmpNmPath)
146+
if (linkedBins.length) {
147+
await Promise.all(linkedBins.map(b => unlink(b)))
148+
}
149+
// Only if mkdir indicated that it created a dir should we cleanup
150+
// that directory
151+
if (tmpDir) {
152+
await rm(tmpDir, { recursive: true, force: true })
153+
}
154+
}
155+
156+
if (linkedBins.length) {
157+
binPaths.push(path)
158+
return await run().finally(cleanupLinks)
159+
} else {
160+
// if we didnt create a bin then we still might need to cleanup the
161+
// symlinked directory we created
162+
await cleanupLinks()
141163
}
142-
// We swap out args[0] with the bin from the manifest later
143-
packages.push(args[0])
144164
}
165+
166+
const localBinDir = dirname(dirname(localBin))
167+
const localBinPath = await localFileExists(localBinDir, args[0], '/')
168+
if (localBinPath) {
169+
binPaths.push(localBinPath)
170+
return await run()
171+
}
172+
173+
if (globalPath && await fileExists(`${globalBin}/${args[0]}`)) {
174+
binPaths.push(globalBin)
175+
return await run()
176+
}
177+
178+
// We swap out args[0] with the bin from the manifest later
179+
packages.push(args[0])
145180
}
146181

147182
// Resolve any directory specs so that the npx directory is unique to the

workspaces/libnpmexec/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
"devDependencies": {
5353
"@npmcli/eslint-config": "^4.0.0",
5454
"@npmcli/template-oss": "4.10.0",
55-
"bin-links": "^4.0.1",
5655
"minify-registry-metadata": "^2.2.0",
5756
"mkdirp": "^1.0.4",
5857
"tap": "^16.0.1"
@@ -61,6 +60,7 @@
6160
"@npmcli/arborist": "^6.1.2",
6261
"@npmcli/ci-detect": "^3.0.1",
6362
"@npmcli/run-script": "^6.0.0",
63+
"bin-links": "^4.0.1",
6464
"chalk": "^4.1.0",
6565
"npm-package-arg": "^10.0.0",
6666
"npmlog": "^7.0.1",

0 commit comments

Comments
 (0)