Skip to content

Commit

Permalink
chore: refactor commands
Browse files Browse the repository at this point in the history
This is the first phase of refactoring the internal structure of the npm
commands to set us up for future changes.  This iteration changes the
function signature of `exec` for all the commands to be a async (no more
callbacks), and also groups all the commands into their own
subdirectory.

It also removes the Proxy `npm.commands` object, in favor of an
`npm.cmd` and `npm.exec` function that breaks up the two things that
proxy was doing. Namely, getting to the attributes of a given command
(`npm.cmd` now does this), and actually running the command `npm.exec`
does this.

PR-URL: #3959
Credit: @wraithgar
Close: #3959
Reviewed-by: @lukekarrys
  • Loading branch information
wraithgar committed Nov 3, 2021
1 parent 85d5919 commit 8ffeb71
Show file tree
Hide file tree
Showing 192 changed files with 11,724 additions and 14,685 deletions.
11 changes: 4 additions & 7 deletions lib/workspaces/arborist-cmd.js → lib/arborist-cmd.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// a list of workspace names and passes it on to new Arborist() to
// be able to run a filtered Arborist.reify() at some point.

const BaseCommand = require('../base-command.js')
const BaseCommand = require('./base-command.js')
class ArboristCmd extends BaseCommand {
get isArboristCmd () {
return true
Expand All @@ -17,12 +17,9 @@ class ArboristCmd extends BaseCommand {
]
}

execWorkspaces (args, filters, cb) {
this.setWorkspaces(filters, true)
.then(() => {
this.exec(args, cb)
})
.catch(er => cb(er))
async execWorkspaces (args, filters) {
await this.setWorkspaces(filters)
return this.exec(args)
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/base-command.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class BaseCommand {
})
}

execWorkspaces (args, filters, cb) {
async execWorkspaces (args, filters) {
throw Object.assign(
new Error('This command does not support workspaces.'),
{ code: 'ENOWORKSPACES' }
Expand Down
17 changes: 9 additions & 8 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ module.exports = async (process) => {

checkForUnsupportedNode()

const npm = require('../lib/npm.js')
const Npm = require('../lib/npm.js')
const npm = new Npm()
const exitHandler = require('../lib/utils/exit-handler.js')
exitHandler.setNpm(npm)

Expand All @@ -38,6 +39,7 @@ module.exports = async (process) => {

const updateNotifier = require('../lib/utils/update-notifier.js')

let cmd
// now actually fire up npm and run the command.
// this is how to use npm programmatically:
try {
Expand All @@ -55,24 +57,23 @@ module.exports = async (process) => {

updateNotifier(npm)

const cmd = npm.argv.shift()
cmd = npm.argv.shift()
if (!cmd) {
npm.output(npm.usage)
npm.output(await npm.usage)
process.exitCode = 1
return exitHandler()
}

const impl = npm.commands[cmd]
if (!impl) {
await npm.exec(cmd, npm.argv)
exitHandler()
} catch (err) {
if (err.code === 'EUNKNOWNCOMMAND') {
const didYouMean = require('./utils/did-you-mean.js')
const suggestions = await didYouMean(npm, npm.localPrefix, cmd)
npm.output(`Unknown command: "${cmd}"${suggestions}\n\nTo see a list of supported npm commands, run:\n npm help`)
process.exitCode = 1
return exitHandler()
}

impl(npm.argv, exitHandler)
} catch (err) {
return exitHandler(err)
}
}
12 changes: 4 additions & 8 deletions lib/access.js → lib/commands/access.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ const path = require('path')
const libaccess = require('libnpmaccess')
const readPackageJson = require('read-package-json-fast')

const otplease = require('./utils/otplease.js')
const getIdentity = require('./utils/get-identity.js')
const BaseCommand = require('./base-command.js')
const otplease = require('../utils/otplease.js')
const getIdentity = require('../utils/get-identity.js')
const BaseCommand = require('../base-command.js')

const subcommands = [
'public',
Expand Down Expand Up @@ -76,11 +76,7 @@ class Access extends BaseCommand {
}
}

exec (args, cb) {
this.access(args).then(() => cb()).catch(cb)
}

async access ([cmd, ...args]) {
async exec ([cmd, ...args]) {
if (!cmd)
throw this.usageError('Subcommand is required.')

Expand Down
18 changes: 7 additions & 11 deletions lib/adduser.js → lib/commands/adduser.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
const log = require('npmlog')
const replaceInfo = require('./utils/replace-info.js')
const BaseCommand = require('./base-command.js')
const replaceInfo = require('../utils/replace-info.js')
const BaseCommand = require('../base-command.js')
const authTypes = {
legacy: require('./auth/legacy.js'),
oauth: require('./auth/oauth.js'),
saml: require('./auth/saml.js'),
sso: require('./auth/sso.js'),
legacy: require('../auth/legacy.js'),
oauth: require('../auth/oauth.js'),
saml: require('../auth/saml.js'),
sso: require('../auth/sso.js'),
}

class AddUser extends BaseCommand {
Expand All @@ -24,11 +24,7 @@ class AddUser extends BaseCommand {
]
}

exec (args, cb) {
this.adduser(args).then(() => cb()).catch(cb)
}

async adduser (args) {
async exec (args) {
const { scope } = this.npm.flatOptions
const registry = this.getRegistry(this.npm.flatOptions)
const auth = this.getAuthType(this.npm.flatOptions)
Expand Down
12 changes: 4 additions & 8 deletions lib/audit.js → lib/commands/audit.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
const Arborist = require('@npmcli/arborist')
const auditReport = require('npm-audit-report')
const reifyFinish = require('./utils/reify-finish.js')
const auditError = require('./utils/audit-error.js')
const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js')
const reifyFinish = require('../utils/reify-finish.js')
const auditError = require('../utils/audit-error.js')
const ArboristWorkspaceCmd = require('../arborist-cmd.js')

class Audit extends ArboristWorkspaceCmd {
/* istanbul ignore next - see test/lib/load-all-commands.js */
Expand Down Expand Up @@ -47,11 +47,7 @@ class Audit extends ArboristWorkspaceCmd {
}
}

exec (args, cb) {
this.audit(args).then(() => cb()).catch(cb)
}

async audit (args) {
async exec (args) {
const reporter = this.npm.config.get('json') ? 'json' : 'detail'
const opts = {
...this.npm.flatOptions,
Expand Down
10 changes: 3 additions & 7 deletions lib/bin.js → lib/commands/bin.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const envPath = require('./utils/path.js')
const BaseCommand = require('./base-command.js')
const envPath = require('../utils/path.js')
const BaseCommand = require('../base-command.js')

class Bin extends BaseCommand {
static get description () {
Expand All @@ -14,11 +14,7 @@ class Bin extends BaseCommand {
return ['global']
}

exec (args, cb) {
this.bin(args).then(() => cb()).catch(cb)
}

async bin (args) {
async exec (args) {
const b = this.npm.bin
this.npm.output(b)
if (this.npm.config.get('global') && !envPath.includes(b))
Expand Down
7 changes: 4 additions & 3 deletions lib/birthday.js → lib/commands/birthday.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
const BaseCommand = require('./base-command.js')
const BaseCommand = require('../base-command.js')

class Birthday extends BaseCommand {
exec (args, cb) {
async exec () {
this.npm.config.set('package', ['@npmcli/npm-birthday'])
this.npm.config.set('yes', true)
return this.npm.commands.exec(['npm-birthday'], cb)
const exec = await this.npm.cmd('exec')
return exec.exec(['npm-birthday'])
}
}

Expand Down
12 changes: 4 additions & 8 deletions lib/bugs.js → lib/commands/bugs.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
const log = require('npmlog')
const pacote = require('pacote')
const openUrl = require('./utils/open-url.js')
const hostedFromMani = require('./utils/hosted-git-info-from-manifest.js')
const BaseCommand = require('./base-command.js')
const openUrl = require('../utils/open-url.js')
const hostedFromMani = require('../utils/hosted-git-info-from-manifest.js')
const BaseCommand = require('../base-command.js')

class Bugs extends BaseCommand {
static get description () {
Expand All @@ -22,11 +22,7 @@ class Bugs extends BaseCommand {
return ['browser', 'registry']
}

exec (args, cb) {
this.bugs(args).then(() => cb()).catch(cb)
}

async bugs (args) {
async exec (args) {
if (!args || !args.length)
args = ['.']

Expand Down
8 changes: 2 additions & 6 deletions lib/cache.js → lib/commands/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const pacote = require('pacote')
const path = require('path')
const rimraf = promisify(require('rimraf'))
const semver = require('semver')
const BaseCommand = require('./base-command.js')
const BaseCommand = require('../base-command.js')
const npa = require('npm-package-arg')
const jsonParse = require('json-parse-even-better-errors')
const localeCompare = require('@isaacs/string-locale-compare')('en')
Expand Down Expand Up @@ -104,11 +104,7 @@ class Cache extends BaseCommand {
}
}

exec (args, cb) {
this.cache(args).then(() => cb()).catch(cb)
}

async cache (args) {
async exec (args) {
const cmd = args.shift()
switch (cmd) {
case 'rm': case 'clear': case 'clean':
Expand Down
10 changes: 3 additions & 7 deletions lib/ci.js → lib/commands/ci.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const util = require('util')
const Arborist = require('@npmcli/arborist')
const rimraf = util.promisify(require('rimraf'))
const reifyFinish = require('./utils/reify-finish.js')
const reifyFinish = require('../utils/reify-finish.js')
const runScript = require('@npmcli/run-script')
const fs = require('fs')
const readdir = util.promisify(fs.readdir)
Expand All @@ -17,7 +17,7 @@ const removeNodeModules = async where => {
await Promise.all(entries.map(f => rimraf(`${path}/${f}`, rimrafOpts)))
process.emit('timeEnd', 'npm-ci:rm')
}
const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js')
const ArboristWorkspaceCmd = require('../arborist-cmd.js')

class CI extends ArboristWorkspaceCmd {
/* istanbul ignore next - see test/lib/load-all-commands.js */
Expand All @@ -39,11 +39,7 @@ class CI extends ArboristWorkspaceCmd {
]
}

exec (args, cb) {
this.ci().then(() => cb()).catch(cb)
}

async ci () {
async exec () {
if (this.npm.config.get('global')) {
const err = new Error('`npm ci` does not work for global packages')
err.code = 'ECIGLOBAL'
Expand Down
42 changes: 20 additions & 22 deletions lib/completion.js → lib/commands/completion.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@
// as an array.
//

const { definitions, shorthands } = require('./utils/config/index.js')
const deref = require('./utils/deref-command.js')
const { aliases, cmdList, plumbing } = require('./utils/cmd-list.js')
const { definitions, shorthands } = require('../utils/config/index.js')
const deref = require('../utils/deref-command.js')
const { aliases, cmdList, plumbing } = require('../utils/cmd-list.js')
const aliasNames = Object.keys(aliases)
const fullList = cmdList.concat(aliasNames).filter(c => !plumbing.includes(c))
const nopt = require('nopt')
const configNames = Object.keys(definitions)
const shorthandNames = Object.keys(shorthands)
const allConfs = configNames.concat(shorthandNames)
const isWindowsShell = require('./utils/is-windows-shell.js')
const fileExists = require('./utils/file-exists.js')
const isWindowsShell = require('../utils/is-windows-shell.js')
const fileExists = require('../utils/file-exists.js')

const { promisify } = require('util')
const BaseCommand = require('./base-command.js')
const BaseCommand = require('../base-command.js')

class Completion extends BaseCommand {
/* istanbul ignore next - see test/lib/load-all-commands.js */
Expand Down Expand Up @@ -75,11 +75,7 @@ class Completion extends BaseCommand {
return out
}

exec (args, cb) {
this.compl(args).then(() => cb()).catch(cb)
}

async compl (args) {
async exec (args) {
if (isWindowsShell) {
const msg = 'npm completion supported only in MINGW / Git bash on Windows'
throw Object.assign(new Error(msg), {
Expand Down Expand Up @@ -163,8 +159,8 @@ class Completion extends BaseCommand {
// at this point, if words[1] is some kind of npm command,
// then complete on it.
// otherwise, do nothing
const impl = this.npm.commands[cmd]
if (impl && impl.completion) {
const impl = this.npm.cmd(cmd)
if (impl.completion) {
const comps = await impl.completion(opts)
return this.wrap(opts, comps)
}
Expand Down Expand Up @@ -195,19 +191,11 @@ const dumpScript = async () => {
const fs = require('fs')
const readFile = promisify(fs.readFile)
const { resolve } = require('path')
const p = resolve(__dirname, 'utils/completion.sh')
const p = resolve(__dirname, '..', 'utils', 'completion.sh')

const d = (await readFile(p, 'utf8')).replace(/^#!.*?\n/, '')
await new Promise((res, rej) => {
let done = false
process.stdout.write(d, () => {
if (done)
return

done = true
res()
})

process.stdout.on('error', er => {
if (done)
return
Expand All @@ -224,11 +212,21 @@ const dumpScript = async () => {
// Really, one should not be tossing away EPIPE errors, or any
// errors, so casually. But, without this, `. <(npm completion)`
// can never ever work on OS X.
// TODO Ignoring coverage, see 'non EPIPE errors cause failures' test.
/* istanbul ignore next */
if (er.errno === 'EPIPE')
res()
else
rej(er)
})

process.stdout.write(d, () => {
if (done)
return

done = true
res()
})
})
}

Expand Down
Loading

0 comments on commit 8ffeb71

Please sign in to comment.