fix(did-you-mean): succeed if cwd is not a package

The did-you-mean code was trying to parse a local package.json to suggest scripts and bins, which was causing an exception if you ran npm outside of a directory with a valid package.json. This fixes that. PR-URL: #3747 Credit: @wraithgar Close: #3747 Reviewed-by: @nlf
npm · Sep 13, 2021 · 5974397 · 5974397
1 parent ac8e4ad
commit 5974397
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 39 deletions.
diff --git a/lib/utils/did-you-mean.js b/lib/utils/did-you-mean.js
@@ -3,25 +3,26 @@ const readJson = require('read-package-json-fast')
 const { cmdList } = require('./cmd-list.js')
 
 const didYouMean = async (npm, path, scmd) => {
-  const bestCmd = cmdList
+  let best = cmdList
     .filter(cmd => distance(scmd, cmd) < scmd.length * 0.4 && scmd !== cmd)
     .map(str => `    npm ${str} # ${npm.commands[str].description}`)
 
-  const pkg = await readJson(`${path}/package.json`)
-  const { scripts } = pkg
   // We would already be suggesting this in `npm x` so omit them here
   const runScripts = ['stop', 'start', 'test', 'restart']
-  const bestRun = Object.keys(scripts || {})
-    .filter(cmd => distance(scmd, cmd) < scmd.length * 0.4 &&
-      !runScripts.includes(cmd))
-    .map(str => `    npm run ${str} # run the "${str}" package script`)
-
-  const { bin } = pkg
-  const bestBin = Object.keys(bin || {})
-    .filter(cmd => distance(scmd, cmd) < scmd.length * 0.4)
-    .map(str => `    npm exec ${str} # run the "${str}" command from either this or a remote npm package`)
-
-  const best = [...bestCmd, ...bestRun, ...bestBin]
+  try {
+    const { bin, scripts } = await readJson(`${path}/package.json`)
+    best = best.concat(
+      Object.keys(scripts || {})
+        .filter(cmd => distance(scmd, cmd) < scmd.length * 0.4 &&
+          !runScripts.includes(cmd))
+        .map(str => `    npm run ${str} # run the "${str}" package script`),
+      Object.keys(bin || {})
+        .filter(cmd => distance(scmd, cmd) < scmd.length * 0.4)
+        .map(str => `    npm exec ${str} # run the "${str}" command from either this or a remote npm package`)
+    )
+  } catch (_) {
+    // gracefully ignore not being in a folder w/ a package.json
+  }
 
   if (best.length === 0)
     return ''

diff --git a/test/lib/utils/did-you-mean.js b/test/lib/utils/did-you-mean.js
@@ -5,34 +5,55 @@ const dym = require('../../../lib/utils/did-you-mean.js')
 t.test('did-you-mean', t => {
   npm.load(err => {
     t.notOk(err)
-    t.test('nistall', async t => {
-      const result = await dym(npm, npm.localPrefix, 'nistall')
-      t.match(result, 'npm install')
-    })
-    t.test('sttest', async t => {
-      const result = await dym(npm, npm.localPrefix, 'sttest')
-      t.match(result, 'npm test')
-      t.match(result, 'npm run posttest')
+    t.test('with package.json', t => {
+      const testdir = t.testdir({
+        'package.json': JSON.stringify({
+          bin: {
+            npx: 'exists',
+          },
+          scripts: {
+            install: 'exists',
+            posttest: 'exists',
+          },
+        }),
+      })
+      t.test('nistall', async t => {
+        const result = await dym(npm, testdir, 'nistall')
+        t.match(result, 'npm install')
+      })
+      t.test('sttest', async t => {
+        const result = await dym(npm, testdir, 'sttest')
+        t.match(result, 'npm test')
+        t.match(result, 'npm run posttest')
+      })
+      t.test('npz', async t => {
+        const result = await dym(npm, testdir, 'npxx')
+        t.match(result, 'npm exec npx')
+      })
+      t.test('qwuijbo', async t => {
+        const result = await dym(npm, testdir, 'qwuijbo')
+        t.match(result, '')
+      })
+      t.end()
     })
-    t.test('npz', async t => {
-      const result = await dym(npm, npm.localPrefix, 'npxx')
-      t.match(result, 'npm exec npx')
+    t.test('with no package.json', t => {
+      const testdir = t.testdir({})
+      t.test('nistall', async t => {
+        const result = await dym(npm, testdir, 'nistall')
+        t.match(result, 'npm install')
+      })
+      t.end()
     })
-    t.test('qwuijbo', async t => {
-      const result = await dym(npm, npm.localPrefix, 'qwuijbo')
-      t.match(result, '')
+    t.test('missing bin and script properties', async t => {
+      const testdir = t.testdir({
+        'package.json': JSON.stringify({
+          name: 'missing-bin',
+        }),
+      })
+
+      const result = await dym(npm, testdir, 'nistall')
+      t.match(result, 'npm install')
     })
     t.end()
   })
 })
-
-t.test('missing bin and script properties', async t => {
-  const path = t.testdir({
-    'package.json': JSON.stringify({
-      name: 'missing-bin',
-    }),
-  })
-
-  const result = await dym(npm, path, 'nistall')
-  t.match(result, 'npm install')
-})