fix: look up local command bins from local tree (#5273)

Author: wraithgar

workspaces/libnpmexec/lib/index.js

@@ -28,35 +28,37 @@ const binPaths = []
 const manifests = new Map()
 
 const getManifest = async (spec, flatOptions) => {
-  if (!manifests.get(spec.raw)) {
+  if (!manifests.has(spec.raw)) {
     const manifest = await pacote.manifest(spec, { ...flatOptions, preferOnline: true })
     manifests.set(spec.raw, manifest)
   }
   return manifests.get(spec.raw)
 }
 
 // Returns the required manifest if the spec is missing from the tree
+// Returns the found node if it is in the tree
 const missingFromTree = async ({ spec, tree, flatOptions }) => {
   if (spec.registry && (spec.rawSpec === '' || spec.type !== 'tag')) {
     // registry spec that is not a specific tag.
     const nodesBySpec = tree.inventory.query('packageName', spec.name)
     for (const node of nodesBySpec) {
       if (spec.type === 'tag') {
         // package requested by name only
-        return
+        return { node }
       } else if (spec.type === 'version') {
         // package requested by specific version
         if (node.pkgid === spec.raw) {
-          return
+          return { node }
         }
       } else {
         // package requested by version range, only remaining registry type
         if (semver.satisfies(node.package.version, spec.rawSpec)) {
-          return
+          return { node }
         }
       }
     }
-    return await getManifest(spec, flatOptions)
+    const manifest = await getManifest(spec, flatOptions)
+    return { manifest }
   } else {
     // non-registry spec, or a specific tag.  Look up manifest and check
     // resolved to see if it's in the tree.
@@ -65,10 +67,10 @@ const missingFromTree = async ({ spec, tree, flatOptions }) => {
     for (const node of nodesByManifest) {
       if (node.package.resolved === manifest._resolved) {
         // we have a package by the same name and the same resolved destination, nothing to add.
-        return
+        return { node }
       }
     }
-    return manifest
+    return { manifest }
   }
 }
 
@@ -132,35 +134,37 @@ const exec = async (opts) => {
 
   // Find anything that isn't installed locally
   const needInstall = []
-  await Promise.all(packages.map(async pkg => {
+  let commandManifest
+  await Promise.all(packages.map(async (pkg, i) => {
     const spec = npa(pkg, path)
-    const manifest = await missingFromTree({ spec, tree: localTree, flatOptions })
+    const { manifest, node } = await missingFromTree({ spec, tree: localTree, flatOptions })
     if (manifest) {
       // Package does not exist in the local tree
       needInstall.push({ spec, manifest })
+      if (i === 0) {
+        commandManifest = manifest
+      }
+    } else if (i === 0) {
+      // The node.package has enough to look up the bin
+      commandManifest = node.package
     }
   }))
 
   if (needPackageCommandSwap) {
-    // Either we have a scoped package or the bin of our package we inferred
-    // from arg[0] might not be identical to the package name
     const spec = npa(args[0])
-    let commandManifest
-    if (needInstall.length === 0) {
-      commandManifest = await getManifest(spec, flatOptions)
-    } else {
-      commandManifest = needInstall[0].manifest
-    }
 
     args[0] = getBinFromManifest(commandManifest)
 
-    // See if the package is installed globally, and run the translated bin
-    const globalArb = new Arborist({ ...flatOptions, path: globalPath, global: true })
-    const globalTree = await globalArb.loadActual()
-    const globalManifest = await missingFromTree({ spec, tree: globalTree, flatOptions })
-    if (!globalManifest) {
-      binPaths.push(globalBin)
-      return await run()
+    if (needInstall.length > 0) {
+      // See if the package is installed globally, and run the translated bin
+      const globalArb = new Arborist({ ...flatOptions, path: globalPath, global: true })
+      const globalTree = await globalArb.loadActual()
+      const { manifest: globalManifest } =
+        await missingFromTree({ spec, tree: globalTree, flatOptions })
+      if (!globalManifest) {
+        binPaths.push(globalBin)
+        return await run()
+      }
     }
   }
 
@@ -183,7 +187,7 @@ const exec = async (opts) => {
     })
     const npxTree = await npxArb.loadActual()
     await Promise.all(needInstall.map(async ({ spec }) => {
-      const manifest = await missingFromTree({ spec, tree: npxTree, flatOptions })
+      const { manifest } = await missingFromTree({ spec, tree: npxTree, flatOptions })
       if (manifest) {
         // Manifest is not in npxCache, we need to install it there
         if (!spec.registry) {

workspaces/libnpmexec/test/index.js

@@ -66,6 +66,60 @@ require('fs').writeFileSync(process.argv.slice(2)[0], 'LOCAL PKG')`,
   t.equal(res, 'LOCAL PKG', 'should run local pkg bin script')
 })
 
+t.test('locally available pkg - by scoped name only', async t => {
+  const pkg = {
+    name: '@npmcli/npx-local-test',
+    version: '2.0.0',
+    bin: {
+      'npx-local-test': './index.js',
+    },
+  }
+  const path = t.testdir({
+    cache: {},
+    npxCache: {},
+    node_modules: {
+      '.bin': {},
+      '@npmcli': {
+        'npx-local-test': {
+          'package.json': JSON.stringify(pkg),
+          'index.js': `#!/usr/bin/env node
+  require('fs').writeFileSync(process.argv.slice(2)[0], 'LOCAL PKG')`,
+        },
+      },
+    },
+    'package.json': JSON.stringify({
+      name: 'pkg',
+      dependencies: {
+        '@npmcli/npx-local-test': '^2.0.0',
+      },
+    }),
+  })
+  const runPath = path
+  const cache = resolve(path, 'cache')
+  const npxCache = resolve(path, 'npxCache')
+
+  const executable =
+    resolve(path, 'node_modules/@npmcli/npx-local-test/index.js')
+  fs.chmodSync(executable, 0o775)
+
+  await binLinks({
+    path: resolve(path, 'node_modules/@npmcli/npx-local-test'),
+    pkg,
+  })
+
+  await libexec({
+    ...baseOpts,
+    cache,
+    npxCache,
+    args: ['@npmcli/npx-local-test', 'resfile'],
+    path,
+    runPath,
+  })
+
+  const res = fs.readFileSync(resolve(path, 'resfile')).toString()
+  t.equal(res, 'LOCAL PKG', 'should run local pkg bin script')
+})
+
 t.test('locally available pkg - by name', async t => {
   const pkg = {
     name: '@ruyadorno/create-index',