Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: discrete npm doctor commands #5888

Merged
merged 4 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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