From 59743972c2ae1d2dd601aaa6c59974c686b1cb29 Mon Sep 17 00:00:00 2001 From: Gar Date: Mon, 13 Sep 2021 10:31:02 -0700 Subject: [PATCH] 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: https://github.com/npm/cli/pull/3747 Credit: @wraithgar Close: #3747 Reviewed-by: @nlf --- lib/utils/did-you-mean.js | 29 +++++++------- test/lib/utils/did-you-mean.js | 71 ++++++++++++++++++++++------------ 2 files changed, 61 insertions(+), 39 deletions(-) diff --git a/lib/utils/did-you-mean.js b/lib/utils/did-you-mean.js index 0cfdd035255eb..c324253af2406 100644 --- 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 index 15712b665be6e..1285d5300853b 100644 --- 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') -})