Skip to content

Commit

Permalink
feat: discrete npm doctor commands (#5888)
Browse files Browse the repository at this point in the history
* fix: output doctor checks as they finish
* feat: allow for discrete npm doctor checks
* feat: add environment check to npm doctor
* chore: refactor command
  • Loading branch information
wraithgar authored Dec 7, 2022
1 parent 83fb125 commit cf57ffa
Show file tree
Hide file tree
Showing 5 changed files with 903 additions and 359 deletions.
6 changes: 4 additions & 2 deletions docs/lib/content/commands/npm-doctor.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ Also, in addition to this, there are also very many issue reports due to
using old versions of npm. Since npm is constantly improving, running
`npm@latest` is better than an old version.

`npm doctor` verifies the following items in your environment, and if there
are any recommended changes, it will display them.
`npm doctor` verifies the following items in your environment, and if
there are any recommended changes, it will display them. By default npm
runs all of these checks. You can limit what checks are ran by
specifying them as extra arguments.

#### `npm ping`

Expand Down
230 changes: 162 additions & 68 deletions lib/commands/doctor.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
const cacache = require('cacache')
const fs = require('fs')
const fetch = require('make-fetch-happen')
const table = require('text-table')
const Table = require('cli-table3')
const which = require('which')
const pacote = require('pacote')
const { resolve } = require('path')
const semver = require('semver')
const { promisify } = require('util')
const log = require('../utils/log-shim.js')
const ansiTrim = require('../utils/ansi-trim.js')
const ping = require('../utils/ping.js')
const {
registry: { default: defaultRegistry },
Expand All @@ -17,6 +16,7 @@ const lstat = promisify(fs.lstat)
const readdir = promisify(fs.readdir)
const access = promisify(fs.access)
const { R_OK, W_OK, X_OK } = fs.constants

const maskLabel = mask => {
const label = []
if (mask & R_OK) {
Expand All @@ -34,76 +34,105 @@ const maskLabel = mask => {
return label.join(', ')
}

const subcommands = [
{
groups: ['ping', 'registry'],
title: 'npm ping',
cmd: 'checkPing',
}, {
groups: ['versions'],
title: 'npm -v',
cmd: 'getLatestNpmVersion',
}, {
groups: ['versions'],
title: 'node -v',
cmd: 'getLatestNodejsVersion',
}, {
groups: ['registry'],
title: 'npm config get registry',
cmd: 'checkNpmRegistry',
}, {
groups: ['environment'],
title: 'git executable in PATH',
cmd: 'getGitPath',
}, {
groups: ['environment'],
title: 'global bin folder in PATH',
cmd: 'getBinPath',
}, {
groups: ['permissions', 'cache'],
title: 'Perms check on cached files',
cmd: 'checkCachePermission',
windows: false,
}, {
groups: ['permissions'],
title: 'Perms check on local node_modules',
cmd: 'checkLocalModulesPermission',
windows: false,
}, {
groups: ['permissions'],
title: 'Perms check on global node_modules',
cmd: 'checkGlobalModulesPermission',
windows: false,
}, {
groups: ['permissions'],
title: 'Perms check on local bin folder',
cmd: 'checkLocalBinPermission',
windows: false,
}, {
groups: ['permissions'],
title: 'Perms check on global bin folder',
cmd: 'checkGlobalBinPermission',
windows: false,
}, {
groups: ['cache'],
title: 'Verify cache contents',
cmd: 'verifyCachedFiles',
windows: false,
},
// TODO:
// group === 'dependencies'?
// - ensure arborist.loadActual() runs without errors and no invalid edges
// - ensure package-lock.json matches loadActual()
// - verify loadActual without hidden lock file matches hidden lockfile
// group === '???'
// - verify all local packages have bins linked
// What is the fix for these?
]
const BaseCommand = require('../base-command.js')
class Doctor extends BaseCommand {
static description = 'Check your npm environment'
static name = 'doctor'
static params = ['registry']
static ignoreImplicitWorkspace = false
static usage = [`[${subcommands.flatMap(s => s.groups)
.filter((value, index, self) => self.indexOf(value) === index)
.join('] [')}]`]

static subcommands = subcommands

// minimum width of check column, enough for the word `Check`
#checkWidth = 5

async exec (args) {
log.info('Running checkup')
let allOk = true

// each message is [title, ok, message]
const messages = []

const actions = [
['npm ping', 'checkPing', []],
['npm -v', 'getLatestNpmVersion', []],
['node -v', 'getLatestNodejsVersion', []],
['npm config get registry', 'checkNpmRegistry', []],
['which git', 'getGitPath', []],
...(process.platform === 'win32'
? []
: [
[
'Perms check on cached files',
'checkFilesPermission',
[this.npm.cache, true, R_OK],
], [
'Perms check on local node_modules',
'checkFilesPermission',
[this.npm.localDir, true, R_OK | W_OK, true],
], [
'Perms check on global node_modules',
'checkFilesPermission',
[this.npm.globalDir, false, R_OK],
], [
'Perms check on local bin folder',
'checkFilesPermission',
[this.npm.localBin, false, R_OK | W_OK | X_OK, true],
], [
'Perms check on global bin folder',
'checkFilesPermission',
[this.npm.globalBin, false, X_OK],
],
]),
[
'Verify cache contents',
'verifyCachedFiles',
[this.npm.flatOptions.cache],
],
// TODO:
// - ensure arborist.loadActual() runs without errors and no invalid edges
// - ensure package-lock.json matches loadActual()
// - verify loadActual without hidden lock file matches hidden lockfile
// - verify all local packages have bins linked
]
const actions = this.actions(args)
this.#checkWidth = actions.reduce((length, item) =>
Math.max(item.title.length, length), this.#checkWidth)

if (!this.npm.silent) {
this.output(['Check', 'Value', 'Recommendation/Notes'].map(h => this.npm.chalk.underline(h)))
}
// Do the actual work
for (const [msg, fn, args] of actions) {
const line = [msg]
for (const { title, cmd } of actions) {
const item = [title]
try {
line.push(true, await this[fn](...args))
} catch (er) {
line.push(false, er)
item.push(true, await this[cmd]())
} catch (err) {
item.push(false, err)
}
messages.push(line)
}

const outHead = ['Check', 'Value', 'Recommendation/Notes'].map(h => this.npm.chalk.underline(h))
let allOk = true
const outBody = messages.map(item => {
if (!item[1]) {
allOk = false
item[0] = this.npm.chalk.red(item[0])
Expand All @@ -112,18 +141,18 @@ class Doctor extends BaseCommand {
} else {
item[1] = this.npm.chalk.green('ok')
}
return item
})
const outTable = [outHead, ...outBody]
const tableOpts = {
stringLength: s => ansiTrim(s).length,
if (!this.npm.silent) {
this.output(item)
}
}

if (!this.npm.silent) {
this.npm.output(table(outTable, tableOpts))
}
if (!allOk) {
throw new Error('Some problems found. See above for recommendations.')
if (this.npm.silent) {
/* eslint-disable-next-line max-len */
throw new Error('Some problems found. Check logs or disable silent mode for recommendations.')
} else {
throw new Error('Some problems found. See above for recommendations.')
}
}
}

Expand Down Expand Up @@ -191,6 +220,35 @@ class Doctor extends BaseCommand {
}
}

async getBinPath (dir) {
const tracker = log.newItem('getBinPath', 1)
tracker.info('getBinPath', 'Finding npm global bin in your PATH')
if (!process.env.PATH.includes(this.npm.globalBin)) {
throw new Error(`Add ${this.npm.globalBin} to your $PATH`)
}
return this.npm.globalBin
}

async checkCachePermission () {
return this.checkFilesPermission(this.npm.cache, true, R_OK)
}

async checkLocalModulesPermission () {
return this.checkFilesPermission(this.npm.localDir, true, R_OK | W_OK, true)
}

async checkGlobalModulesPermission () {
return this.checkFilesPermission(this.npm.globalDir, false, R_OK)
}

async checkLocalBinPermission () {
return this.checkFilesPermission(this.npm.localBin, false, R_OK | W_OK | X_OK, true)
}

async checkGlobalBinPermission () {
return this.checkFilesPermission(this.npm.globalBin, false, X_OK)
}

async checkFilesPermission (root, shouldOwn, mask, missingOk) {
let ok = true

Expand Down Expand Up @@ -264,7 +322,7 @@ class Doctor extends BaseCommand {
try {
return await which('git').catch(er => {
tracker.warn(er)
throw "Install git and ensure it's in your PATH."
throw new Error("Install git and ensure it's in your PATH.")
})
} finally {
tracker.finish()
Expand Down Expand Up @@ -312,6 +370,42 @@ class Doctor extends BaseCommand {
return `using default registry (${defaultRegistry})`
}
}

output (row) {
const t = new Table({
chars: { top: '',
'top-mid': '',
'top-left': '',
'top-right': '',
bottom: '',
'bottom-mid': '',
'bottom-left': '',
'bottom-right': '',
left: '',
'left-mid': '',
mid: '',
'mid-mid': '',
right: '',
'right-mid': '',
middle: ' ' },
style: { 'padding-left': 0, 'padding-right': 0 },
colWidths: [this.#checkWidth, 6],
})
t.push(row)
this.npm.output(t.toString())
}

actions (params) {
return this.constructor.subcommands.filter(subcmd => {
if (process.platform === 'win32' && subcmd.windows === false) {
return false
}
if (params.length) {
return params.some(param => subcmd.groups.includes(param))
}
return true
})
}
}

module.exports = Doctor
Loading

0 comments on commit cf57ffa

Please sign in to comment.