diff --git a/.eslintrc.local.json b/.eslintrc.local.json index e953021f7324c..2ff50f91ec326 100644 --- a/.eslintrc.local.json +++ b/.eslintrc.local.json @@ -1,14 +1,5 @@ { "rules": { - "no-shadow": "off", "no-console": "error" - }, - "overrides": [{ - "files": [ - "test/**" - ], - "rules": { - "no-console": "off" - } - }] + } } diff --git a/lib/arborist-cmd.js b/lib/arborist-cmd.js index 29efe984d9b26..42699ece364ad 100644 --- a/lib/arborist-cmd.js +++ b/lib/arborist-cmd.js @@ -17,22 +17,35 @@ class ArboristCmd extends BaseCommand { 'install-links', ] + static workspaces = true static ignoreImplicitWorkspace = false constructor (npm) { super(npm) - if (this.npm.config.isDefault('audit') - && (this.npm.global || this.npm.config.get('location') !== 'project') - ) { - this.npm.config.set('audit', false) - } else if (this.npm.global && this.npm.config.get('audit')) { - log.warn('config', - 'includes both --global and --audit, which is currently unsupported.') + + const { config } = this.npm + + // when location isn't set and global isn't true check for a package.json at + // the localPrefix and set the location to project if found + const locationProject = config.get('location') === 'project' || ( + config.isDefault('location') + // this is different then `npm.global` which falls back to checking + // location which we do not want to use here + && !config.get('global') + && npm.localPackage + ) + + // if audit is not set and we are in global mode and location is not project + // and we assume its not a project related context, then we set audit=false + if (config.isDefault('audit') && (this.npm.global || !locationProject)) { + config.set('audit', false) + } else if (this.npm.global && config.get('audit')) { + log.warn('config', 'includes both --global and --audit, which is currently unsupported.') } } - async execWorkspaces (args, filters) { - await this.setWorkspaces(filters) + async execWorkspaces (args) { + await this.setWorkspaces() return this.exec(args) } } diff --git a/lib/base-command.js b/lib/base-command.js index b57b7474a5efb..0adff8e5d95ea 100644 --- a/lib/base-command.js +++ b/lib/base-command.js @@ -8,12 +8,21 @@ const getWorkspaces = require('./workspaces/get-workspaces.js') const cmdAliases = require('./utils/cmd-list').aliases class BaseCommand { + static workspaces = false + static ignoreImplicitWorkspace = true + constructor (npm) { this.wrapWidth = 80 this.npm = npm - if (!this.skipConfigValidation) { - this.npm.config.validate() + const { config } = this.npm + + if (!this.constructor.skipConfigValidation) { + config.validate() + } + + if (config.get('workspaces') === false && config.get('workspace').length) { + throw new Error('Can not use --no-workspaces and --workspace at the same time') } } @@ -25,35 +34,31 @@ class BaseCommand { return this.constructor.description } - get ignoreImplicitWorkspace () { - return this.constructor.ignoreImplicitWorkspace - } - - get skipConfigValidation () { - return this.constructor.skipConfigValidation + get params () { + return this.constructor.params } get usage () { const usage = [ - `${this.constructor.description}`, + `${this.description}`, '', 'Usage:', ] if (!this.constructor.usage) { - usage.push(`npm ${this.constructor.name}`) + usage.push(`npm ${this.name}`) } else { - usage.push(...this.constructor.usage.map(u => `npm ${this.constructor.name} ${u}`)) + usage.push(...this.constructor.usage.map(u => `npm ${this.name} ${u}`)) } - if (this.constructor.params) { + if (this.params) { usage.push('') usage.push('Options:') usage.push(this.wrappedParams) } const aliases = Object.keys(cmdAliases).reduce((p, c) => { - if (cmdAliases[c] === this.constructor.name) { + if (cmdAliases[c] === this.name) { p.push(c) } return p @@ -68,7 +73,7 @@ class BaseCommand { } usage.push('') - usage.push(`Run "npm help ${this.constructor.name}" for more info`) + usage.push(`Run "npm help ${this.name}" for more info`) return usage.join('\n') } @@ -77,7 +82,7 @@ class BaseCommand { let results = '' let line = '' - for (const param of this.constructor.params) { + for (const param of this.params) { const usage = `[${ConfigDefinitions[param].usage}]` if (line.length && line.length + usage.length > this.wrapWidth) { results = [results, line].filter(Boolean).join('\n') @@ -98,26 +103,48 @@ class BaseCommand { }) } - async execWorkspaces (args, filters) { - throw Object.assign(new Error('This command does not support workspaces.'), { - code: 'ENOWORKSPACES', - }) - } + async cmdExec (args) { + const { config } = this.npm - async setWorkspaces (filters) { - if (this.isArboristCmd) { - this.includeWorkspaceRoot = false + if (config.get('usage')) { + return this.npm.output(this.usage) } - const relativeFrom = relative(this.npm.localPrefix, process.cwd()).startsWith('..') - ? this.npm.localPrefix - : process.cwd() + const hasWsConfig = config.get('workspaces') || config.get('workspace').length + // if cwd is a workspace, the default is set to [that workspace] + const implicitWs = config.get('workspace', 'default').length + // (-ws || -w foo) && (cwd is not a workspace || command is not ignoring implicit workspaces) + if (hasWsConfig && (!implicitWs || !this.constructor.ignoreImplicitWorkspace)) { + if (this.npm.global) { + throw new Error('Workspaces not supported for global packages') + } + if (!this.constructor.workspaces) { + throw Object.assign(new Error('This command does not support workspaces.'), { + code: 'ENOWORKSPACES', + }) + } + return this.execWorkspaces(args) + } + + return this.exec(args) + } + + async setWorkspaces () { + const includeWorkspaceRoot = this.isArboristCmd + ? false + : this.npm.config.get('include-workspace-root') + + const prefixInsideCwd = relative(this.npm.localPrefix, process.cwd()).startsWith('..') + const relativeFrom = prefixInsideCwd ? this.npm.localPrefix : process.cwd() + + const filters = this.npm.config.get('workspace') const ws = await getWorkspaces(filters, { path: this.npm.localPrefix, - includeWorkspaceRoot: this.includeWorkspaceRoot, + includeWorkspaceRoot, relativeFrom, }) + this.workspaces = ws this.workspaceNames = [...ws.keys()] this.workspacePaths = [...ws.values()] diff --git a/lib/cli.js b/lib/cli.js index 9aaf6c593675a..007778aa4b986 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -68,6 +68,11 @@ module.exports = async process => { // leak any private CLI configs to other programs process.title = 'npm' + // if npm is called as "npmg" or "npm_g", then run in global mode. + if (process.argv[1][process.argv[1].length - 1] === 'g') { + process.argv.splice(1, 1, 'npm', '-g') + } + // Nothing should happen before this line if we can't guarantee it will // not have syntax errors in some version of node const validateEngines = createEnginesValidation() @@ -78,11 +83,6 @@ module.exports = async process => { const npm = new Npm() exitHandler.setNpm(npm) - // if npm is called as "npmg" or "npm_g", then run in global mode. - if (process.argv[1][process.argv[1].length - 1] === 'g') { - process.argv.splice(1, 1, 'npm', '-g') - } - // only log node and npm paths in argv initially since argv can contain // sensitive info. a cleaned version will be logged later const log = require('./utils/log-shim.js') @@ -112,6 +112,7 @@ module.exports = async process => { // this is how to use npm programmatically: try { await npm.load() + if (npm.config.get('version', 'cli')) { npm.output(npm.version) return exitHandler() @@ -130,7 +131,7 @@ module.exports = async process => { return exitHandler() } - await npm.exec(cmd, npm.argv) + await npm.exec(cmd) return exitHandler() } catch (err) { if (err.code === 'EUNKNOWNCOMMAND') { diff --git a/lib/commands/access.js b/lib/commands/access.js index d5ac5bb2f008e..23e51f071b112 100644 --- a/lib/commands/access.js +++ b/lib/commands/access.js @@ -37,8 +37,6 @@ class Access extends BaseCommand { 'registry', ] - static ignoreImplicitWorkspace = true - static usage = [ 'list packages [|| []', 'list collaborators [ []]', diff --git a/lib/commands/adduser.js b/lib/commands/adduser.js index 1e92b35f4a662..cd4cba60511cb 100644 --- a/lib/commands/adduser.js +++ b/lib/commands/adduser.js @@ -13,8 +13,6 @@ class AddUser extends BaseCommand { 'auth-type', ] - static ignoreImplicitWorkspace = true - async exec (args) { const scope = this.npm.config.get('scope') let registry = this.npm.config.get('registry') diff --git a/lib/commands/audit.js b/lib/commands/audit.js index feccefda0c904..13886ea6350b6 100644 --- a/lib/commands/audit.js +++ b/lib/commands/audit.js @@ -152,7 +152,7 @@ class VerifySignatures { const keys = await fetch.json('/-/npm/v1/keys', { ...this.npm.flatOptions, registry, - }).then(({ keys }) => keys.map((key) => ({ + }).then(({ keys: ks }) => ks.map((key) => ({ ...key, pemkey: `-----BEGIN PUBLIC KEY-----\n${key.key}\n-----END PUBLIC KEY-----`, }))).catch(err => { diff --git a/lib/commands/cache.js b/lib/commands/cache.js index 637085015fcb7..0ab40b9ed44a9 100644 --- a/lib/commands/cache.js +++ b/lib/commands/cache.js @@ -2,7 +2,7 @@ const cacache = require('cacache') const Arborist = require('@npmcli/arborist') const pacote = require('pacote') const fs = require('fs/promises') -const path = require('path') +const { join } = require('path') const semver = require('semver') const BaseCommand = require('../base-command.js') const npa = require('npm-package-arg') @@ -74,8 +74,6 @@ class Cache extends BaseCommand { 'verify', ] - static ignoreImplicitWorkspace = true - async completion (opts) { const argv = opts.conf.argv.remain if (argv.length === 2) { @@ -111,7 +109,7 @@ class Cache extends BaseCommand { // npm cache clean [pkg]* async clean (args) { - const cachePath = path.join(this.npm.cache, '_cacache') + const cachePath = join(this.npm.cache, '_cacache') if (args.length === 0) { if (!this.npm.config.get('force')) { throw new Error(`As of npm@5, the npm cache self-heals from corruption issues @@ -169,7 +167,7 @@ class Cache extends BaseCommand { } async verify () { - const cache = path.join(this.npm.cache, '_cacache') + const cache = join(this.npm.cache, '_cacache') const prefix = cache.indexOf(process.env.HOME) === 0 ? `~${cache.slice(process.env.HOME.length)}` : cache @@ -192,7 +190,7 @@ class Cache extends BaseCommand { // npm cache ls [--package ...] async ls (specs) { - const cachePath = path.join(this.npm.cache, '_cacache') + const cachePath = join(this.npm.cache, '_cacache') const cacheKeys = Object.keys(await cacache.ls(cachePath)) if (specs.length > 0) { // get results for each package spec specified diff --git a/lib/commands/completion.js b/lib/commands/completion.js index 8fc05b2e82313..f5604e099f9a2 100644 --- a/lib/commands/completion.js +++ b/lib/commands/completion.js @@ -31,6 +31,7 @@ const fs = require('fs/promises') const nopt = require('nopt') +const { resolve } = require('path') const { definitions, shorthands } = require('../utils/config/index.js') const { aliases, commands, plumbing } = require('../utils/cmd-list.js') @@ -40,21 +41,13 @@ const configNames = Object.keys(definitions) const shorthandNames = Object.keys(shorthands) const allConfs = configNames.concat(shorthandNames) const { isWindowsShell } = require('../utils/is-windows.js') -const fileExists = async (file) => { - try { - const stat = await fs.stat(file) - return stat.isFile() - } catch { - return false - } -} +const fileExists = (file) => fs.stat(file).then(s => s.isFile()).catch(() => false) const BaseCommand = require('../base-command.js') class Completion extends BaseCommand { static description = 'Tab Completion for npm' static name = 'completion' - static ignoreImplicitWorkspace = true // completion for the completion command async completion (opts) { @@ -62,7 +55,6 @@ class Completion extends BaseCommand { return } - const { resolve } = require('path') const [bashExists, zshExists] = await Promise.all([ fileExists(resolve(process.env.HOME, '.bashrc')), fileExists(resolve(process.env.HOME, '.zshrc')), @@ -93,7 +85,7 @@ class Completion extends BaseCommand { if (COMP_CWORD === undefined || COMP_LINE === undefined || COMP_POINT === undefined) { - return dumpScript() + return dumpScript(resolve(this.npm.npmRoot, 'lib', 'utils', 'completion.sh')) } // ok we're actually looking at the envs and outputting the suggestions @@ -150,9 +142,9 @@ class Completion extends BaseCommand { // take a little shortcut and use npm's arg parsing logic. // don't have to worry about the last arg being implicitly // boolean'ed, since the last block will catch that. - const types = Object.entries(definitions).reduce((types, [key, def]) => { - types[key] = def.type - return types + const types = Object.entries(definitions).reduce((acc, [key, def]) => { + acc[key] = def.type + return acc }, {}) const parsed = opts.conf = nopt(types, shorthands, partialWords.slice(0, -1), 0) @@ -196,10 +188,7 @@ class Completion extends BaseCommand { } } -const dumpScript = async () => { - const { resolve } = require('path') - const p = resolve(__dirname, '..', 'utils', 'completion.sh') - +const dumpScript = async (p) => { const d = (await fs.readFile(p, 'utf8')).replace(/^#!.*?\n/, '') await new Promise((res, rej) => { let done = false diff --git a/lib/commands/config.js b/lib/commands/config.js index 103fbb554e5d1..ac5a74d01f7de 100644 --- a/lib/commands/config.js +++ b/lib/commands/config.js @@ -112,11 +112,6 @@ class Config extends BaseCommand { } } - async execWorkspaces (args, filters) { - log.warn('config', 'This command does not support workspaces.') - return this.exec(args) - } - async exec ([action, ...args]) { log.disableProgress() try { @@ -251,14 +246,14 @@ ${defData} `.split('\n').join(EOL) await mkdir(dirname(file), { recursive: true }) await writeFile(file, tmpData, 'utf8') - await new Promise((resolve, reject) => { + await new Promise((res, rej) => { const [bin, ...args] = e.split(/\s+/) const editor = spawn(bin, [...args, file], { stdio: 'inherit' }) editor.on('exit', (code) => { if (code) { - return reject(new Error(`editor process exited with code: ${code}`)) + return rej(new Error(`editor process exited with code: ${code}`)) } - return resolve() + return res() }) }) } diff --git a/lib/commands/diff.js b/lib/commands/diff.js index c8fd734918d75..1f4bfd3eb1151 100644 --- a/lib/commands/diff.js +++ b/lib/commands/diff.js @@ -32,6 +32,7 @@ class Diff extends BaseCommand { 'include-workspace-root', ] + static workspaces = true static ignoreImplicitWorkspace = false async exec (args) { @@ -67,8 +68,8 @@ class Diff extends BaseCommand { return this.npm.output(res) } - async execWorkspaces (args, filters) { - await this.setWorkspaces(filters) + async execWorkspaces (args) { + await this.setWorkspaces() for (const workspacePath of this.workspacePaths) { this.top = workspacePath this.prefix = workspacePath diff --git a/lib/commands/dist-tag.js b/lib/commands/dist-tag.js index 8052e4f7e4e38..bc61a4691e55a 100644 --- a/lib/commands/dist-tag.js +++ b/lib/commands/dist-tag.js @@ -17,6 +17,7 @@ class DistTag extends BaseCommand { 'ls []', ] + static workspaces = true static ignoreImplicitWorkspace = false async completion (opts) { @@ -57,14 +58,14 @@ class DistTag extends BaseCommand { } } - async execWorkspaces ([cmdName, pkg, tag], filters) { + async execWorkspaces ([cmdName, pkg, tag]) { // cmdName is some form of list // pkg is one of: // - unset // - . // - .@version if (['ls', 'l', 'sl', 'list'].includes(cmdName) && (!pkg || pkg === '.' || /^\.@/.test(pkg))) { - return this.listWorkspaces(filters) + return this.listWorkspaces() } // pkg is unset @@ -73,12 +74,12 @@ class DistTag extends BaseCommand { // - . // - .@version if (!pkg && (!cmdName || cmdName === '.' || /^\.@/.test(cmdName))) { - return this.listWorkspaces(filters) + return this.listWorkspaces() } // anything else is just a regular dist-tag command // so we fallback to the non-workspaces implementation - log.warn('Ignoring workspaces for specified package') + log.warn('dist-tag', 'Ignoring workspaces for specified package') return this.exec([cmdName, pkg, tag]) } @@ -116,7 +117,7 @@ class DistTag extends BaseCommand { }, spec, } - await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts)) + await otplease(this.npm, reqOpts, o => regFetch(url, o)) this.npm.output(`+${t}: ${spec.name}@${version}`) } @@ -142,7 +143,7 @@ class DistTag extends BaseCommand { method: 'DELETE', spec, } - await otplease(this.npm, reqOpts, reqOpts => regFetch(url, reqOpts)) + await otplease(this.npm, reqOpts, o => regFetch(url, o)) this.npm.output(`-${tag}: ${spec.name}@${version}`) } @@ -172,8 +173,8 @@ class DistTag extends BaseCommand { } } - async listWorkspaces (filters) { - await this.setWorkspaces(filters) + async listWorkspaces () { + await this.setWorkspaces() for (const name of this.workspaceNames) { try { diff --git a/lib/commands/edit.js b/lib/commands/edit.js index 67ac32e017184..a671a5d6bad5d 100644 --- a/lib/commands/edit.js +++ b/lib/commands/edit.js @@ -51,23 +51,23 @@ class Edit extends BaseCommand { const dir = resolve(this.npm.dir, path) // graceful-fs does not promisify - await new Promise((resolve, reject) => { + await new Promise((res, rej) => { fs.lstat(dir, (err) => { if (err) { - return reject(err) + return rej(err) } - const [bin, ...args] = this.npm.config.get('editor').split(/\s+/) - const editor = cp.spawn(bin, [...args, dir], { stdio: 'inherit' }) + const [bin, ...spawnArgs] = this.npm.config.get('editor').split(/\s+/) + const editor = cp.spawn(bin, [...spawnArgs, dir], { stdio: 'inherit' }) editor.on('exit', async (code) => { if (code) { - return reject(new Error(`editor process exited with code: ${code}`)) + return rej(new Error(`editor process exited with code: ${code}`)) } try { await this.npm.exec('rebuild', [dir]) - } catch (err) { - reject(err) + } catch (execErr) { + rej(execErr) } - resolve() + res() }) }) }) diff --git a/lib/commands/exec.js b/lib/commands/exec.js index a77a6326c00f2..a5235c7845851 100644 --- a/lib/commands/exec.js +++ b/lib/commands/exec.js @@ -1,4 +1,4 @@ -const path = require('path') +const { resolve } = require('path') const libexec = require('libnpmexec') const BaseCommand = require('../base-command.js') @@ -20,10 +20,25 @@ class Exec extends BaseCommand { '--package=foo -c \' [args...]\'', ] + static workspaces = true static ignoreImplicitWorkspace = false static isShellout = true - async exec (_args, { locationMsg, runPath } = {}) { + async exec (args) { + return this.callExec(args) + } + + async execWorkspaces (args) { + await this.setWorkspaces() + + for (const [name, path] of this.workspaces) { + const locationMsg = + `in workspace ${this.npm.chalk.green(name)} at location:\n${this.npm.chalk.dim(path)}` + await this.callExec(args, { locationMsg, runPath: path }) + } + } + + async callExec (args, { locationMsg, runPath } = {}) { // This is where libnpmexec will look for locally installed packages const localPrefix = this.npm.localPrefix @@ -32,7 +47,6 @@ class Exec extends BaseCommand { runPath = process.cwd() } - const args = [..._args] const call = this.npm.config.get('call') let globalPath const { @@ -49,10 +63,10 @@ class Exec extends BaseCommand { // is invalid (i.e. no lib/node_modules). This is not a trivial thing to // untangle and fix so we work around it here. if (this.npm.localPrefix !== this.npm.globalPrefix) { - globalPath = path.resolve(globalDir, '..') + globalPath = resolve(globalDir, '..') } - if (call && _args.length) { + if (call && args.length) { throw this.usageError() } @@ -61,7 +75,8 @@ class Exec extends BaseCommand { // we explicitly set packageLockOnly to false because if it's true // when we try to install a missing package, we won't actually install it packageLockOnly: false, - args, + // copy args so they dont get mutated + args: [...args], call, localBin, locationMsg, @@ -75,16 +90,6 @@ class Exec extends BaseCommand { yes, }) } - - async execWorkspaces (args, filters) { - await this.setWorkspaces(filters) - - for (const [name, path] of this.workspaces) { - const locationMsg = - `in workspace ${this.npm.chalk.green(name)} at location:\n${this.npm.chalk.dim(path)}` - await this.exec(args, { locationMsg, runPath: path }) - } - } } module.exports = Exec diff --git a/lib/commands/find-dupes.js b/lib/commands/find-dupes.js index b99ea7a14eb21..b1a3120860366 100644 --- a/lib/commands/find-dupes.js +++ b/lib/commands/find-dupes.js @@ -18,7 +18,7 @@ class FindDupes extends ArboristWorkspaceCmd { ...super.params, ] - async exec (args, cb) { + async exec (args) { this.npm.config.set('dry-run', true) return this.npm.exec('dedupe', []) } diff --git a/lib/commands/help-search.js b/lib/commands/help-search.js index 488189bbbc5cd..afb82bfaca9ee 100644 --- a/lib/commands/help-search.js +++ b/lib/commands/help-search.js @@ -13,14 +13,13 @@ class HelpSearch extends BaseCommand { static name = 'help-search' static usage = [''] static params = ['long'] - static ignoreImplicitWorkspace = true async exec (args) { if (!args.length) { throw this.usageError() } - const docPath = path.resolve(__dirname, '..', '..', 'docs/content') + const docPath = path.resolve(this.npm.npmRoot, 'docs/content') const files = await glob(`${globify(docPath)}/*/*.md`) const data = await this.readFiles(files) const results = await this.searchFiles(args, data, files) @@ -142,7 +141,7 @@ class HelpSearch extends BaseCommand { formatResults (args, results) { const cols = Math.min(process.stdout.columns || Infinity, 80) + 1 - const out = results.map(res => { + const output = results.map(res => { const out = [res.cmd] const r = Object.keys(res.hits) .map(k => `${k}:${res.hits[k]}`) @@ -189,10 +188,10 @@ class HelpSearch extends BaseCommand { const finalOut = results.length && !this.npm.config.get('long') ? 'Top hits for ' + (args.map(JSON.stringify).join(' ')) + '\n' + '—'.repeat(cols - 1) + '\n' + - out + '\n' + + output + '\n' + '—'.repeat(cols - 1) + '\n' + '(run with -l or --long to see more context)' - : out + : output return finalOut.trim() } diff --git a/lib/commands/hook.js b/lib/commands/hook.js index 084741c0c5eea..b0f52a801f571 100644 --- a/lib/commands/hook.js +++ b/lib/commands/hook.js @@ -19,12 +19,8 @@ class Hook extends BaseCommand { 'update ', ] - static ignoreImplicitWorkspace = true - async exec (args) { - return otplease(this.npm, { - ...this.npm.flatOptions, - }, (opts) => { + return otplease(this.npm, { ...this.npm.flatOptions }, (opts) => { switch (args[0]) { case 'add': return this.add(args[1], args[2], args[3], opts) @@ -49,9 +45,7 @@ class Hook extends BaseCommand { this.npm.output(Object.keys(hook).join('\t')) this.npm.output(Object.keys(hook).map(k => hook[k]).join('\t')) } else if (!this.npm.silent) { - this.npm.output(`+ ${this.hookName(hook)} ${ - opts.unicode ? ' ➜ ' : ' -> ' - } ${hook.endpoint}`) + this.npm.output(`+ ${this.hookName(hook)} ${opts.unicode ? ' ➜ ' : ' -> '} ${hook.endpoint}`) } } @@ -104,9 +98,7 @@ class Hook extends BaseCommand { this.npm.output(Object.keys(hook).join('\t')) this.npm.output(Object.keys(hook).map(k => hook[k]).join('\t')) } else if (!this.npm.silent) { - this.npm.output(`- ${this.hookName(hook)} ${ - opts.unicode ? ' ✘ ' : ' X ' - } ${hook.endpoint}`) + this.npm.output(`- ${this.hookName(hook)} ${opts.unicode ? ' ✘ ' : ' X '} ${hook.endpoint}`) } } @@ -118,9 +110,7 @@ class Hook extends BaseCommand { this.npm.output(Object.keys(hook).join('\t')) this.npm.output(Object.keys(hook).map(k => hook[k]).join('\t')) } else if (!this.npm.silent) { - this.npm.output(`+ ${this.hookName(hook)} ${ - opts.unicode ? ' ➜ ' : ' -> ' - } ${hook.endpoint}`) + this.npm.output(`+ ${this.hookName(hook)} ${opts.unicode ? ' ➜ ' : ' -> '} ${hook.endpoint}`) } } diff --git a/lib/commands/init.js b/lib/commands/init.js index 02a43b0ef0960..03a686365cdfe 100644 --- a/lib/commands/init.js +++ b/lib/commands/init.js @@ -30,19 +30,20 @@ class Init extends BaseCommand { '<@scope> (same as `npx <@scope>/create`)', ] + static workspaces = true static ignoreImplicitWorkspace = false async exec (args) { // npm exec style if (args.length) { - return (await this.execCreate({ args, path: process.cwd() })) + return await this.execCreate(args) } // no args, uses classic init-package-json boilerplate await this.template() } - async execWorkspaces (args, filters) { + async execWorkspaces (args) { // if the root package is uninitiated, take care of it first if (this.npm.flatOptions.includeWorkspaceRoot) { await this.exec(args) @@ -52,6 +53,10 @@ class Init extends BaseCommand { // ensure the command throw if no package.json is found before trying // to create a workspace package.json file or its folders const pkg = await rpj(resolve(this.npm.localPrefix, 'package.json')) + + // these are workspaces that are being created, so we cant use + // this.setWorkspaces() + const filters = this.npm.config.get('workspace') const wPath = filterArg => resolve(this.npm.localPrefix, filterArg) const workspacesPaths = [] @@ -61,8 +66,8 @@ class Init extends BaseCommand { const path = wPath(filterArg) await mkdir(path, { recursive: true }) workspacesPaths.push(path) - await this.execCreate({ args, path }) - await this.setWorkspace({ pkg, workspacePath: path }) + await this.execCreate(args, path) + await this.setWorkspace(pkg, path) } return } @@ -73,14 +78,14 @@ class Init extends BaseCommand { await mkdir(path, { recursive: true }) workspacesPaths.push(path) await this.template(path) - await this.setWorkspace({ pkg, workspacePath: path }) + await this.setWorkspace(pkg, path) } // reify packages once all workspaces have been initialized await this.update(workspacesPaths) } - async execCreate ({ args, path }) { + async execCreate (args, path = process.cwd()) { const [initerName, ...otherArgs] = args let packageName = initerName @@ -95,8 +100,7 @@ class Init extends BaseCommand { const req = npa(initerName) if (req.type === 'git' && req.hosted) { const { user, project } = req.hosted - packageName = initerName - .replace(user + '/' + project, user + '/create-' + project) + packageName = initerName.replace(`${user}/${project}`, `${user}/create-${project}`) } else if (req.registry) { packageName = `${req.name.replace(/^(@[^/]+\/)?/, '$1create-')}@${req.rawSpec}` } else { @@ -174,7 +178,7 @@ class Init extends BaseCommand { }) } - async setWorkspace ({ pkg, workspacePath }) { + async setWorkspace (pkg, workspacePath) { const workspaces = await mapWorkspaces({ cwd: this.npm.localPrefix, pkg }) // skip setting workspace if current package.json glob already satisfies it @@ -210,9 +214,7 @@ class Init extends BaseCommand { // translate workspaces paths into an array containing workspaces names const workspaces = [] for (const path of workspacesPaths) { - const pkgPath = resolve(path, 'package.json') - const { name } = await rpj(pkgPath) - .catch(() => ({})) + const { name } = await rpj(resolve(path, 'package.json')).catch(() => ({})) if (name) { workspaces.push(name) diff --git a/lib/commands/install-ci-test.js b/lib/commands/install-ci-test.js index 9977a2edc5641..f7a357ba6e124 100644 --- a/lib/commands/install-ci-test.js +++ b/lib/commands/install-ci-test.js @@ -7,7 +7,7 @@ class InstallCITest extends CI { static description = 'Install a project with a clean slate and run tests' static name = 'install-ci-test' - async exec (args, cb) { + async exec (args) { await this.npm.exec('ci', args) return this.npm.exec('test', []) } diff --git a/lib/commands/install-test.js b/lib/commands/install-test.js index 191d70909f9e7..11f22e535403c 100644 --- a/lib/commands/install-test.js +++ b/lib/commands/install-test.js @@ -7,7 +7,7 @@ class InstallTest extends Install { static description = 'Install package(s) and run tests' static name = 'install-test' - async exec (args, cb) { + async exec (args) { await this.npm.exec('install', args) return this.npm.exec('test', []) } diff --git a/lib/commands/login.js b/lib/commands/login.js index 7f6898d00ba93..dc4ed8a67acd9 100644 --- a/lib/commands/login.js +++ b/lib/commands/login.js @@ -13,8 +13,6 @@ class Login extends BaseCommand { 'auth-type', ] - static ignoreImplicitWorkspace = true - async exec (args) { const scope = this.npm.config.get('scope') let registry = this.npm.config.get('registry') diff --git a/lib/commands/logout.js b/lib/commands/logout.js index 7c2a7f0b2f830..aea5e93652b0e 100644 --- a/lib/commands/logout.js +++ b/lib/commands/logout.js @@ -11,8 +11,6 @@ class Logout extends BaseCommand { 'scope', ] - static ignoreImplicitWorkspace = true - async exec (args) { const registry = this.npm.config.get('registry') const scope = this.npm.config.get('scope') diff --git a/lib/commands/org.js b/lib/commands/org.js index f49556c8d6a19..575ff75e2a6cf 100644 --- a/lib/commands/org.js +++ b/lib/commands/org.js @@ -13,7 +13,6 @@ class Org extends BaseCommand { ] static params = ['registry', 'otp', 'json', 'parseable'] - static ignoreImplicitWorkspace = true async completion (opts) { const argv = opts.conf.argv.remain @@ -32,7 +31,7 @@ class Org extends BaseCommand { } } - async exec ([cmd, orgname, username, role], cb) { + async exec ([cmd, orgname, username, role]) { return otplease(this.npm, { ...this.npm.flatOptions, }, opts => { @@ -139,15 +138,15 @@ class Org extends BaseCommand { this.npm.output(JSON.stringify(roster, null, 2)) } else if (opts.parseable) { this.npm.output(['user', 'role'].join('\t')) - Object.keys(roster).forEach(user => { - this.npm.output([user, roster[user]].join('\t')) + Object.keys(roster).forEach(u => { + this.npm.output([u, roster[u]].join('\t')) }) } else if (!this.npm.silent) { const table = new Table({ head: ['user', 'role'] }) Object.keys(roster) .sort() - .forEach(user => { - table.push([user, roster[user]]) + .forEach(u => { + table.push([u, roster[u]]) }) this.npm.output(table.toString()) } diff --git a/lib/commands/outdated.js b/lib/commands/outdated.js index 9e2060658ed72..5e8a4e0d2168c 100644 --- a/lib/commands/outdated.js +++ b/lib/commands/outdated.js @@ -1,5 +1,5 @@ const os = require('os') -const path = require('path') +const { resolve } = require('path') const pacote = require('pacote') const table = require('text-table') const chalk = require('chalk') @@ -26,7 +26,7 @@ class Outdated extends ArboristWorkspaceCmd { ] async exec (args) { - const global = path.resolve(this.npm.globalDir, '..') + const global = resolve(this.npm.globalDir, '..') const where = this.npm.global ? global : this.npm.prefix diff --git a/lib/commands/owner.js b/lib/commands/owner.js index 824b64e044ecf..40f16332b2922 100644 --- a/lib/commands/owner.js +++ b/lib/commands/owner.js @@ -32,6 +32,7 @@ class Owner extends BaseCommand { 'ls ', ] + static workspaces = true static ignoreImplicitWorkspace = false async completion (opts) { @@ -82,8 +83,8 @@ class Owner extends BaseCommand { } } - async execWorkspaces ([action, ...args], filters) { - await this.setWorkspaces(filters) + async execWorkspaces ([action, ...args]) { + await this.setWorkspaces() // ls pkg or owner add/rm package if ((action === 'ls' && args.length > 0) || args.length > 1) { const implicitWorkspaces = this.npm.config.get('workspace', 'default') diff --git a/lib/commands/pack.js b/lib/commands/pack.js index c6a74804642f6..74e80e573c2e9 100644 --- a/lib/commands/pack.js +++ b/lib/commands/pack.js @@ -18,6 +18,7 @@ class Pack extends BaseCommand { ] static usage = [''] + static workspaces = true static ignoreImplicitWorkspace = false async exec (args) { @@ -64,7 +65,7 @@ class Pack extends BaseCommand { } } - async execWorkspaces (args, filters) { + async execWorkspaces (args) { // If they either ask for nothing, or explicitly include '.' in the args, // we effectively translate that into each workspace requested @@ -75,7 +76,7 @@ class Pack extends BaseCommand { return this.exec(args) } - await this.setWorkspaces(filters) + await this.setWorkspaces() return this.exec([...this.workspacePaths, ...args.filter(a => a !== '.')]) } } diff --git a/lib/commands/ping.js b/lib/commands/ping.js index 22039214689a9..5a651c4a6ab09 100644 --- a/lib/commands/ping.js +++ b/lib/commands/ping.js @@ -6,7 +6,6 @@ class Ping extends BaseCommand { static description = 'Ping npm registry' static params = ['registry'] static name = 'ping' - static ignoreImplicitWorkspace = true async exec (args) { log.notice('PING', this.npm.config.get('registry')) diff --git a/lib/commands/pkg.js b/lib/commands/pkg.js index 5fac9bfb54683..5cdcd207887c9 100644 --- a/lib/commands/pkg.js +++ b/lib/commands/pkg.js @@ -20,6 +20,7 @@ class Pkg extends BaseCommand { 'workspaces', ] + static workspaces = true static ignoreImplicitWorkspace = false async exec (args, { prefix } = {}) { @@ -49,8 +50,8 @@ class Pkg extends BaseCommand { } } - async execWorkspaces (args, filters) { - await this.setWorkspaces(filters) + async execWorkspaces (args) { + await this.setWorkspaces() const result = {} for (const [workspaceName, workspacePath] of this.workspaces.entries()) { this.prefix = workspacePath @@ -81,7 +82,7 @@ class Pkg extends BaseCommand { // only outputs if not running with workspaces config, // in case you're retrieving info for workspaces the pkgWorkspaces // will handle the output to make sure it get keyed by ws name - if (!this.workspaces) { + if (!this.npm.config.get('workspaces')) { this.npm.output(JSON.stringify(result, null, 2)) } diff --git a/lib/commands/prefix.js b/lib/commands/prefix.js index dd0e34c3d3bd9..264b819fc7692 100644 --- a/lib/commands/prefix.js +++ b/lib/commands/prefix.js @@ -5,7 +5,6 @@ class Prefix extends BaseCommand { static name = 'prefix' static params = ['global'] static usage = ['[-g]'] - static ignoreImplicitWorkspace = true async exec (args) { return this.npm.output(this.npm.prefix) diff --git a/lib/commands/profile.js b/lib/commands/profile.js index 27060cf73a650..e42ebb276d202 100644 --- a/lib/commands/profile.js +++ b/lib/commands/profile.js @@ -54,8 +54,6 @@ class Profile extends BaseCommand { 'otp', ] - static ignoreImplicitWorkspace = true - async completion (opts) { var argv = opts.conf.argv.remain @@ -221,7 +219,7 @@ class Profile extends BaseCommand { newUser[prop] = value - const result = await otplease(this.npm, conf, conf => npmProfile.set(newUser, conf)) + const result = await otplease(this.npm, conf, c => npmProfile.set(newUser, c)) if (this.npm.config.get('json')) { this.npm.output(JSON.stringify({ [prop]: result[prop] }, null, 2)) diff --git a/lib/commands/publish.js b/lib/commands/publish.js index 23323a174ed89..76faea9457f74 100644 --- a/lib/commands/publish.js +++ b/lib/commands/publish.js @@ -38,6 +38,7 @@ class Publish extends BaseCommand { ] static usage = [''] + static workspaces = true static ignoreImplicitWorkspace = false async exec (args) { @@ -123,7 +124,7 @@ class Publish extends BaseCommand { log.notice('', msg) if (!dryRun) { - await otplease(this.npm, opts, opts => libpub(manifest, tarballData, opts)) + await otplease(this.npm, opts, o => libpub(manifest, tarballData, o)) } if (spec.type === 'directory' && !ignoreScripts) { @@ -155,14 +156,14 @@ class Publish extends BaseCommand { return pkgContents } - async execWorkspaces (args, filters) { + async execWorkspaces (args) { // Suppresses JSON output in publish() so we can handle it here this.suppressOutput = true const results = {} const json = this.npm.config.get('json') const { silent } = this.npm - await this.setWorkspaces(filters) + await this.setWorkspaces() for (const [name, workspace] of this.workspaces.entries()) { let pkgContents diff --git a/lib/commands/query.js b/lib/commands/query.js index 5f05ab3164d7c..b5f4d8e57ddf5 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -41,6 +41,7 @@ class Query extends BaseCommand { static name = 'query' static usage = [''] + static workspaces = true static ignoreImplicitWorkspace = false static params = [ @@ -70,8 +71,8 @@ class Query extends BaseCommand { this.npm.output(this.parsedResponse) } - async execWorkspaces (args, filters) { - await this.setWorkspaces(filters) + async execWorkspaces (args) { + await this.setWorkspaces() const opts = { ...this.npm.flatOptions, path: this.npm.prefix, diff --git a/lib/commands/restart.js b/lib/commands/restart.js index 575928b2202cc..7ca2eb323da3c 100644 --- a/lib/commands/restart.js +++ b/lib/commands/restart.js @@ -8,7 +8,6 @@ class Restart extends LifecycleCmd { 'ignore-scripts', 'script-shell', ] - - static ignoreImplicitWorkspace = false } + module.exports = Restart diff --git a/lib/commands/root.js b/lib/commands/root.js index b814034def5ab..7749c602456b7 100644 --- a/lib/commands/root.js +++ b/lib/commands/root.js @@ -3,7 +3,6 @@ class Root extends BaseCommand { static description = 'Display npm root' static name = 'root' static params = ['global'] - static ignoreImplicitWorkspace = true async exec () { this.npm.output(this.npm.dir) diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js index 3852f7ba1820f..51746c5e5285d 100644 --- a/lib/commands/run-script.js +++ b/lib/commands/run-script.js @@ -41,6 +41,7 @@ class RunScript extends BaseCommand { static name = 'run-script' static usage = [' [-- ]'] + static workspaces = true static ignoreImplicitWorkspace = false static isShellout = true @@ -62,11 +63,11 @@ class RunScript extends BaseCommand { } } - async execWorkspaces (args, filters) { + async execWorkspaces (args) { if (args.length) { - return this.runWorkspaces(args, filters) + return this.runWorkspaces(args) } else { - return this.listWorkspaces(args, filters) + return this.listWorkspaces(args) } } @@ -121,11 +122,11 @@ class RunScript extends BaseCommand { banner: !this.npm.silent, } - for (const [event, args] of events) { + for (const [ev, evArgs] of events) { await runScript({ ...opts, - event, - args, + event: ev, + args: evArgs, }) } } @@ -200,7 +201,7 @@ class RunScript extends BaseCommand { async runWorkspaces (args, filters) { const res = [] - await this.setWorkspaces(filters) + await this.setWorkspaces() for (const workspacePath of this.workspacePaths) { const pkg = await rpj(`${workspacePath}/package.json`) @@ -233,7 +234,7 @@ class RunScript extends BaseCommand { } async listWorkspaces (args, filters) { - await this.setWorkspaces(filters) + await this.setWorkspaces() if (this.npm.silent) { return diff --git a/lib/commands/search.js b/lib/commands/search.js index 8751e9e7d22fd..7419e97454688 100644 --- a/lib/commands/search.js +++ b/lib/commands/search.js @@ -51,7 +51,6 @@ class Search extends BaseCommand { ] static usage = ['[search terms ...]'] - static ignoreImplicitWorkspace = true async exec (args) { const opts = { diff --git a/lib/commands/start.js b/lib/commands/start.js index d84ad23ebafa6..a16eade24d21e 100644 --- a/lib/commands/start.js +++ b/lib/commands/start.js @@ -8,7 +8,6 @@ class Start extends LifecycleCmd { 'ignore-scripts', 'script-shell', ] - - static ignoreImplicitWorkspace = false } + module.exports = Start diff --git a/lib/commands/stop.js b/lib/commands/stop.js index db497675a694b..ae3031f06dd96 100644 --- a/lib/commands/stop.js +++ b/lib/commands/stop.js @@ -8,7 +8,6 @@ class Stop extends LifecycleCmd { 'ignore-scripts', 'script-shell', ] - - static ignoreImplicitWorkspace = false } + module.exports = Stop diff --git a/lib/commands/test.js b/lib/commands/test.js index 43be934894dd7..eccc47fc3341c 100644 --- a/lib/commands/test.js +++ b/lib/commands/test.js @@ -8,7 +8,6 @@ class Test extends LifecycleCmd { 'ignore-scripts', 'script-shell', ] - - static ignoreImplicitWorkspace = false } + module.exports = Test diff --git a/lib/commands/token.js b/lib/commands/token.js index de8e61101d8ac..8da8311875714 100644 --- a/lib/commands/token.js +++ b/lib/commands/token.js @@ -14,7 +14,6 @@ class Token extends BaseCommand { static name = 'token' static usage = ['list', 'revoke ', 'create [--read-only] [--cidr=list]'] static params = ['read-only', 'cidr', 'registry', 'otp'] - static ignoreImplicitWorkspace = true async completion (opts) { const argv = opts.conf.argv.remain @@ -30,7 +29,7 @@ class Token extends BaseCommand { throw new Error(argv[2] + ' not recognized') } - async exec (args, cb) { + async exec (args) { log.gauge.show('token') if (args.length === 0) { return this.list() @@ -121,9 +120,7 @@ class Token extends BaseCommand { }) await Promise.all( toRemove.map(key => { - return otplease(this.npm, conf, conf => { - return profile.removeToken(key, conf) - }) + return otplease(this.npm, conf, c => profile.removeToken(key, c)) }) ) if (conf.json) { @@ -144,9 +141,7 @@ class Token extends BaseCommand { const validCIDR = this.validateCIDRList(cidr) log.info('token', 'creating') const result = await pulseTillDone.withPromise( - otplease(this.npm, conf, conf => { - return profile.createToken(password, readonly, validCIDR, conf) - }) + otplease(this.npm, conf, c => profile.createToken(password, readonly, validCIDR, c)) ) delete result.key delete result.updated @@ -216,7 +211,7 @@ class Token extends BaseCommand { } validateCIDRList (cidrs) { - const maybeList = cidrs ? (Array.isArray(cidrs) ? cidrs : [cidrs]) : [] + const maybeList = [].concat(cidrs).filter(Boolean) const list = maybeList.length === 1 ? maybeList[0].split(/,\s*/) : maybeList for (const cidr of list) { if (isCidrV6(cidr)) { diff --git a/lib/commands/uninstall.js b/lib/commands/uninstall.js index e4a193cc5ca4e..8c44f2e32106c 100644 --- a/lib/commands/uninstall.js +++ b/lib/commands/uninstall.js @@ -20,19 +20,13 @@ class Uninstall extends ArboristWorkspaceCmd { } async exec (args) { - // the /path/to/node_modules/.. - const path = this.npm.global - ? resolve(this.npm.globalDir, '..') - : this.npm.localPrefix - if (!args.length) { if (!this.npm.global) { throw new Error('Must provide a package name to remove') } else { - let pkg - try { - pkg = await rpj(resolve(this.npm.localPrefix, 'package.json')) + const pkg = await rpj(resolve(this.npm.localPrefix, 'package.json')) + args.push(pkg.name) } catch (er) { if (er.code !== 'ENOENT' && er.code !== 'ENOTDIR') { throw er @@ -40,11 +34,14 @@ class Uninstall extends ArboristWorkspaceCmd { throw this.usageError() } } - - args.push(pkg.name) } } + // the /path/to/node_modules/.. + const path = this.npm.global + ? resolve(this.npm.globalDir, '..') + : this.npm.localPrefix + const opts = { ...this.npm.flatOptions, path, diff --git a/lib/commands/unpublish.js b/lib/commands/unpublish.js index 268c8c3daedbb..9985e2e39f140 100644 --- a/lib/commands/unpublish.js +++ b/lib/commands/unpublish.js @@ -21,6 +21,7 @@ class Unpublish extends BaseCommand { static name = 'unpublish' static params = ['dry-run', 'force', 'workspace', 'workspaces'] static usage = ['[]'] + static workspaces = true static ignoreImplicitWorkspace = false async getKeysOfVersions (name, opts) { @@ -130,15 +131,15 @@ class Unpublish extends BaseCommand { } if (!dryRun) { - await otplease(this.npm, opts, opts => libunpub(spec, opts)) + await otplease(this.npm, opts, o => libunpub(spec, o)) } if (!silent) { this.npm.output(`- ${pkgName}${pkgVersion}`) } } - async execWorkspaces (args, filters) { - await this.setWorkspaces(filters) + async execWorkspaces (args) { + await this.setWorkspaces() const force = this.npm.config.get('force') if (!force) { diff --git a/lib/commands/update.js b/lib/commands/update.js index be9d35093d43b..fd30bcb41e2b3 100644 --- a/lib/commands/update.js +++ b/lib/commands/update.js @@ -40,9 +40,7 @@ class Update extends ArboristWorkspaceCmd { async exec (args) { const update = args.length === 0 ? true : args const global = path.resolve(this.npm.globalDir, '..') - const where = this.npm.global - ? global - : this.npm.prefix + const where = this.npm.global ? global : this.npm.prefix // In the context of `npm update` the save // config value should default to `false` diff --git a/lib/commands/version.js b/lib/commands/version.js index ab59fff5a308c..a523283671791 100644 --- a/lib/commands/version.js +++ b/lib/commands/version.js @@ -22,6 +22,7 @@ class Version extends BaseCommand { 'include-workspace-root', ] + static workspaces = true static ignoreImplicitWorkspace = false /* eslint-disable-next-line max-len */ @@ -60,12 +61,12 @@ class Version extends BaseCommand { } } - async execWorkspaces (args, filters) { + async execWorkspaces (args) { switch (args.length) { case 0: - return this.listWorkspaces(filters) + return this.listWorkspaces() case 1: - return this.changeWorkspaces(args, filters) + return this.changeWorkspaces(args) default: throw this.usageError() } @@ -80,9 +81,9 @@ class Version extends BaseCommand { return this.npm.output(`${prefix}${version}`) } - async changeWorkspaces (args, filters) { + async changeWorkspaces (args) { const prefix = this.npm.config.get('tag-version-prefix') - await this.setWorkspaces(filters) + await this.setWorkspaces() const updatedWorkspaces = [] for (const [name, path] of this.workspaces) { this.npm.output(name) @@ -120,9 +121,9 @@ class Version extends BaseCommand { } } - async listWorkspaces (filters) { + async listWorkspaces () { const results = {} - await this.setWorkspaces(filters) + await this.setWorkspaces() for (const path of this.workspacePaths) { const pj = resolve(path, 'package.json') // setWorkspaces has already parsed package.json so we know it won't error diff --git a/lib/commands/view.js b/lib/commands/view.js index 32b2d0f92a1a6..1875a84ec306b 100644 --- a/lib/commands/view.js +++ b/lib/commands/view.js @@ -31,8 +31,8 @@ class View extends BaseCommand { 'include-workspace-root', ] + static workspaces = true static ignoreImplicitWorkspace = false - static usage = ['[] [[.subfield]...]'] async completion (opts) { @@ -132,7 +132,7 @@ class View extends BaseCommand { } } - async execWorkspaces (args, filters) { + async execWorkspaces (args) { if (!args.length) { args = ['.'] } @@ -150,7 +150,7 @@ class View extends BaseCommand { args = [''] // getData relies on this } const results = {} - await this.setWorkspaces(filters) + await this.setWorkspaces() for (const name of this.workspaceNames) { const wsPkg = `${name}${pkg.slice(1)}` const [pckmnt, data] = await this.getData(wsPkg, args) @@ -317,13 +317,13 @@ class View extends BaseCommand { return msg.trim() } - prettyView (packument, manifest) { + prettyView (packu, manifest) { // More modern, pretty printing of default view const unicode = this.npm.config.get('unicode') const tags = [] - Object.keys(packument['dist-tags']).forEach((t) => { - const version = packument['dist-tags'][t] + Object.keys(packu['dist-tags']).forEach((t) => { + const version = packu['dist-tags'][t] tags.push(`${chalk.bold.green(t)}: ${version}`) }) const unpackedSize = manifest.dist.unpackedSize && @@ -333,10 +333,10 @@ class View extends BaseCommand { name: chalk.green(manifest.name), version: chalk.green(manifest.version), bins: Object.keys(manifest.bin || {}), - versions: chalk.yellow(packument.versions.length + ''), + versions: chalk.yellow(packu.versions.length + ''), description: manifest.description, deprecated: manifest.deprecated, - keywords: packument.keywords || [], + keywords: packu.keywords || [], license: typeof licenseField === 'string' ? licenseField : (licenseField.type || 'Proprietary'), @@ -347,9 +347,9 @@ class View extends BaseCommand { name: chalk.yellow(manifest._npmUser.name), email: chalk.cyan(manifest._npmUser.email), }), - modified: !packument.time ? undefined - : chalk.yellow(relativeDate(packument.time[manifest.version])), - maintainers: (packument.maintainers || []).map((u) => unparsePerson({ + modified: !packu.time ? undefined + : chalk.yellow(relativeDate(packu.time[manifest.version])), + maintainers: (packu.maintainers || []).map((u) => unparsePerson({ name: chalk.yellow(u.name), email: chalk.cyan(u.email), })), diff --git a/lib/commands/whoami.js b/lib/commands/whoami.js index 4497f9b3a542d..154cc870391ba 100644 --- a/lib/commands/whoami.js +++ b/lib/commands/whoami.js @@ -5,7 +5,6 @@ class Whoami extends BaseCommand { static description = 'Display npm username' static name = 'whoami' static params = ['registry'] - static ignoreImplicitWorkspace = true async exec (args) { const username = await getIdentity(this.npm, { ...this.npm.flatOptions }) diff --git a/lib/lifecycle-cmd.js b/lib/lifecycle-cmd.js index 41633a4ba389c..848771a38355e 100644 --- a/lib/lifecycle-cmd.js +++ b/lib/lifecycle-cmd.js @@ -5,12 +5,14 @@ const BaseCommand = require('./base-command.js') class LifecycleCmd extends BaseCommand { static usage = ['[-- ]'] static isShellout = true + static workspaces = true + static ignoreImplicitWorkspace = false - async exec (args, cb) { + async exec (args) { return this.npm.exec('run-script', [this.constructor.name, ...args]) } - async execWorkspaces (args, filters, cb) { + async execWorkspaces (args) { return this.npm.exec('run-script', [this.constructor.name, ...args]) } } diff --git a/lib/npm.js b/lib/npm.js index 0bdbcdb9efd8b..841d145ddcbad 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -20,25 +20,24 @@ const updateNotifier = require('./utils/update-notifier.js') const pkg = require('../package.json') const cmdList = require('./utils/cmd-list.js') -let warnedNonDashArg = false -const _load = Symbol('_load') - class Npm extends EventEmitter { static get version () { return pkg.version } - command = null updateNotification = null loadErr = null argv = [] + #command = null #runId = new Date().toISOString().replace(/[.:]/g, '_') #loadPromise = null #tmpFolder = null #title = 'npm' #argvClean = [] #chalk = null + #npmRoot = null + #warnedNonDashArg = false #outputBuffer = [] #logFile = new LogFile() @@ -52,12 +51,30 @@ class Npm extends EventEmitter { }, }) - config = new Config({ - npmPath: dirname(__dirname), - definitions, - flatten, - shorthands, - }) + // all these options are only used by tests in order to make testing more + // closely resemble real world usage. for now, npm has no programmatic API so + // it is ok to add stuff here, but we should not rely on it more than + // necessary. XXX: make these options not necessary by refactoring @npmcli/config + // - npmRoot: this is where npm looks for docs files and the builtin config + // - argv: this allows tests to extend argv in the same way the argv would + // be passed in via a CLI arg. + // - excludeNpmCwd: this is a hack to get @npmcli/config to stop walking up + // dirs to set a local prefix when it encounters the `npmRoot`. this + // allows tests created by tap inside this repo to not set the local + // prefix to `npmRoot` since that is the first dir it would encounter when + // doing implicit detection + constructor ({ npmRoot = dirname(__dirname), argv = [], excludeNpmCwd = false } = {}) { + super() + this.#npmRoot = npmRoot + this.config = new Config({ + npmPath: this.#npmRoot, + definitions, + flatten, + shorthands, + argv: [...process.argv, ...argv], + excludeNpmCwd, + }) + } get version () { return this.constructor.version @@ -89,44 +106,31 @@ class Npm extends EventEmitter { async cmd (cmd) { await this.load() - // when location isn't set and global isn't true - // check for a package.json at the localPrefix - // and set the location to project if found - // TODO: this logic can move to the config module loadLocalPrefix to - // avoid double stat calls and consolidate logic - if (this.config.isDefault('location') && !this.config.get('global')) { - const hasPackageJson = await fs.stat(resolve(this.config.localPrefix, 'package.json')) - .then((st) => st.isFile()) - .catch(() => false) - if (hasPackageJson) { - this.config.set('location', 'project') - } - } - - const command = this.deref(cmd) - if (!command) { + const cmdId = this.deref(cmd) + if (!cmdId) { throw Object.assign(new Error(`Unknown command ${cmd}`), { code: 'EUNKNOWNCOMMAND', }) } - const Impl = require(`./commands/${command}.js`) - const impl = new Impl(this) - return impl - } - // Call an npm command - async exec (cmd, args) { - const command = await this.cmd(cmd) - const timeEnd = this.time(`command:${cmd}`) + const Impl = require(`./commands/${cmdId}.js`) + const command = new Impl(this) // since 'test', 'start', 'stop', etc. commands re-enter this function // to call the run-script command, we need to only set it one time. - if (!this.command) { - process.env.npm_command = command.name - this.command = command.name - this.commandInstance = command + if (!this.#command) { + this.#command = command + process.env.npm_command = this.command } + return command + } + + // Call an npm command + async exec (cmd, args = this.argv) { + const command = await this.cmd(cmd) + const timeEnd = this.time(`command:${cmd}`) + // this is async but we dont await it, since its ok if it doesnt // finish before the command finishes running. it uses command and argv // so it must be initiated here, after the command name is set @@ -135,72 +139,27 @@ class Npm extends EventEmitter { // Options are prefixed by a hyphen-minus (-, \u2d). // Other dash-type chars look similar but are invalid. - if (!warnedNonDashArg) { - args - .filter(arg => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(arg)) - .forEach(arg => { - warnedNonDashArg = true - log.error( - 'arg', - 'Argument starts with non-ascii dash, this is probably invalid:', - arg - ) - }) - } - - const workspacesEnabled = this.config.get('workspaces') - // if cwd is a workspace, the default is set to [that workspace] - const implicitWorkspace = this.config.get('workspace', 'default').length > 0 - const workspacesFilters = this.config.get('workspace') - const includeWorkspaceRoot = this.config.get('include-workspace-root') - // only call execWorkspaces when we have workspaces explicitly set - // or when it is implicit and not in our ignore list - const hasWorkspaceFilters = workspacesFilters.length > 0 - const invalidWorkspaceConfig = workspacesEnabled === false && hasWorkspaceFilters - - // (-ws || -w foo) && (cwd is not a workspace || command is not ignoring implicit workspaces) - const filterByWorkspaces = (workspacesEnabled || hasWorkspaceFilters) && - (!implicitWorkspace || !command.ignoreImplicitWorkspace) - // normally this would go in the constructor, but our tests don't - // actually use a real npm object so this.npm.config isn't always - // populated. this is the compromise until we can make that a reality - // and then move this into the constructor. - command.workspaces = workspacesEnabled - command.workspacePaths = null - // normally this would be evaluated in base-command#setWorkspaces, see - // above for explanation - command.includeWorkspaceRoot = includeWorkspaceRoot - - let execPromise = Promise.resolve() - if (this.config.get('usage')) { - this.output(command.usage) - } else if (invalidWorkspaceConfig) { - execPromise = Promise.reject( - new Error('Can not use --no-workspaces and --workspace at the same time')) - } else if (filterByWorkspaces) { - if (this.global) { - execPromise = Promise.reject(new Error('Workspaces not supported for global packages')) - } else { - execPromise = command.execWorkspaces(args, workspacesFilters) + if (!this.#warnedNonDashArg) { + const nonDashArgs = args.filter(a => /^[\u2010-\u2015\u2212\uFE58\uFE63\uFF0D]/.test(a)) + if (nonDashArgs.length) { + this.#warnedNonDashArg = true + log.error( + 'arg', + 'Argument starts with non-ascii dash, this is probably invalid:', + nonDashArgs.join(', ') + ) } - } else { - execPromise = command.exec(args) } - return execPromise.finally(timeEnd) + return command.cmdExec(args).finally(timeEnd) } async load () { if (!this.#loadPromise) { - this.#loadPromise = this.time('npm:load', async () => { - await this[_load]().catch((er) => { - this.loadErr = er - throw er - }) - if (this.config.get('force')) { - log.warn('using --force', 'Recommended protections disabled.') - } - }) + this.#loadPromise = this.time('npm:load', () => this.#load().catch((er) => { + this.loadErr = er + throw er + })) } return this.#loadPromise } @@ -240,21 +199,17 @@ class Npm extends EventEmitter { this.#title = t } - async [_load] () { - const node = this.time('npm:load:whichnode', () => { - try { - return which.sync(process.argv[0]) - } catch { - // TODO should we throw here? + async #load () { + await this.time('npm:load:whichnode', async () => { + // TODO should we throw here? + const node = await which(process.argv[0]).catch(() => {}) + if (node && node.toUpperCase() !== process.execPath.toUpperCase()) { + log.verbose('node symlink', node) + process.execPath = node + this.config.execPath = node } }) - if (node && node.toUpperCase() !== process.execPath.toUpperCase()) { - log.verbose('node symlink', node) - process.execPath = node - this.config.execPath = node - } - await this.time('npm:load:configload', () => this.config.load()) // mkdir this separately since the logs dir can be set to @@ -323,6 +278,18 @@ class Npm extends EventEmitter { this.config.set('scope', `@${configScope}`, this.config.find('scope')) } }) + + if (this.config.get('force')) { + log.warn('using --force', 'Recommended protections disabled.') + } + } + + get isShellout () { + return this.#command?.constructor?.isShellout + } + + get command () { + return this.#command?.name } get flatOptions () { @@ -346,6 +313,10 @@ class Npm extends EventEmitter { return this.flatOptions.color } + get logColor () { + return this.flatOptions.logColor + } + get chalk () { if (!this.#chalk) { let level = chalk.level @@ -361,10 +332,6 @@ class Npm extends EventEmitter { return this.config.get('global') || this.config.get('location') === 'global' } - get logColor () { - return this.flatOptions.logColor - } - get silent () { return this.flatOptions.silent } @@ -401,6 +368,10 @@ class Npm extends EventEmitter { return this.#timers.file } + get npmRoot () { + return this.#npmRoot + } + get cache () { return this.config.get('cache') } @@ -425,6 +396,10 @@ class Npm extends EventEmitter { this.config.localPrefix = r } + get localPackage () { + return this.config.localPackage + } + get globalDir () { return process.platform !== 'win32' ? resolve(this.globalPrefix, 'lib', 'node_modules') diff --git a/lib/package-url-cmd.js b/lib/package-url-cmd.js index eac2bbe1b6d51..20e6a16fe1523 100644 --- a/lib/package-url-cmd.js +++ b/lib/package-url-cmd.js @@ -9,7 +9,6 @@ const log = require('./utils/log-shim') const BaseCommand = require('./base-command.js') class PackageUrlCommand extends BaseCommand { - static ignoreImplicitWorkspace = false static params = [ 'browser', 'registry', @@ -18,6 +17,8 @@ class PackageUrlCommand extends BaseCommand { 'include-workspace-root', ] + static workspaces = true + static ignoreImplicitWorkspace = false static usage = ['[ [ ...]]'] async exec (args) { @@ -41,11 +42,11 @@ class PackageUrlCommand extends BaseCommand { } } - async execWorkspaces (args, filters) { + async execWorkspaces (args) { if (args && args.length) { return this.exec(args) } - await this.setWorkspaces(filters) + await this.setWorkspaces() return this.exec(this.workspacePaths) } diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js index 0f401d6572a59..dd3d9946af819 100644 --- a/lib/utils/config/definitions.js +++ b/lib/utils/config/definitions.js @@ -2141,6 +2141,7 @@ define('unicode', { When set to true, npm uses unicode characters in the tree output. When false, it uses ascii characters instead of unicode glyphs. `, + flatten, }) define('update-notifier', { diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index aee376120ba27..72c7b9fe4553f 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -5,7 +5,21 @@ const replaceInfo = require('./replace-info.js') const { report } = require('./explain-eresolve.js') const log = require('./log-shim') -module.exports = (er, npm) => { +const messageText = msg => msg.map(line => line.slice(1).join(' ')).join('\n') + +const jsonError = (er, npm, { summary, detail }) => { + if (npm?.config.loaded && npm.config.get('json')) { + return { + error: { + code: er.code, + summary: messageText(summary), + detail: messageText(detail), + }, + } + } +} + +const errorMessage = (er, npm) => { const short = [] const detail = [] const files = [] @@ -329,7 +343,7 @@ module.exports = (er, npm) => { 'Actual: ' + JSON.stringify({ npm: npm.version, - node: npm.config.loaded ? npm.config.get('node-version') : process.version, + node: process.version, }), ].join('\n'), ]) @@ -402,5 +416,7 @@ module.exports = (er, npm) => { break } - return { summary: short, detail, files } + return { summary: short, detail, files, json: jsonError(er, npm, { summary: short, detail }) } } + +module.exports = errorMessage diff --git a/lib/utils/exit-handler.js b/lib/utils/exit-handler.js index a9e061de7a4a5..b5fc7042bd020 100644 --- a/lib/utils/exit-handler.js +++ b/lib/utils/exit-handler.js @@ -5,7 +5,6 @@ const log = require('./log-shim.js') const errorMessage = require('./error-message.js') const replaceInfo = require('./replace-info.js') -const messageText = msg => msg.map(line => line.slice(1).join(' ')).join('\n') const indent = (val) => Array.isArray(val) ? val.map(v => indent(v)) : ` ${val}` let npm = null // set by the cli @@ -144,7 +143,7 @@ const exitHandler = err => { // will presumably print its own errors and exit with a proper status // code if there's a problem. If we got an error with a code=0, then... // something else went wrong along the way, so maybe an npm problem? - const isShellout = npm.commandInstance && npm.commandInstance.constructor.isShellout + const isShellout = npm.isShellout const quietShellout = isShellout && typeof err.code === 'number' && err.code if (quietShellout) { exitCode = err.code @@ -181,7 +180,8 @@ const exitHandler = err => { } } - const { summary, detail, files = [] } = errorMessage(err, npm) + const { summary, detail, json, files = [] } = errorMessage(err, npm) + jsonError = json for (let [file, content] of files) { file = `${npm.logPath}${file}` @@ -189,8 +189,8 @@ const exitHandler = err => { try { fs.writeFileSync(file, content) detail.push(['', `\n\nFor a full report see:\n${file}`]) - } catch (err) { - log.warn('', `Could not write error message to ${file} due to ${err}`) + } catch (logFileErr) { + log.warn('', `Could not write error message to ${file} due to ${logFileErr}`) } } @@ -198,16 +198,6 @@ const exitHandler = err => { log.error(...errline) } - if (hasLoadedNpm && npm.config.get('json')) { - jsonError = { - error: { - code: err.code, - summary: messageText(summary), - detail: messageText(detail), - }, - } - } - if (typeof err.errno === 'number') { exitCode = err.errno } else if (typeof err.code === 'number') { diff --git a/lib/utils/explain-dep.js b/lib/utils/explain-dep.js index cd53a2269640e..58258026491dc 100644 --- a/lib/utils/explain-dep.js +++ b/lib/utils/explain-dep.js @@ -103,13 +103,13 @@ const explainDependents = ({ name, dependents }, depth, color) => { const maxLen = 50 const showNames = [] for (let i = max; i < dependents.length; i++) { - const { from: { name = 'the root project' } } = dependents[i] - len += name.length + const { from: { name: depName = 'the root project' } } = dependents[i] + len += depName.length if (len >= maxLen && i < dependents.length - 1) { showNames.push('...') break } - showNames.push(name) + showNames.push(depName) } const show = `(${showNames.join(', ')})` messages.push(`${dependents.length - max} more ${show}`) diff --git a/lib/utils/log-file.js b/lib/utils/log-file.js index b945eedbc96dd..f663997308ed6 100644 --- a/lib/utils/log-file.js +++ b/lib/utils/log-file.js @@ -1,5 +1,5 @@ const os = require('os') -const path = require('path') +const { join, dirname, basename } = require('path') const { format, promisify } = require('util') const glob = promisify(require('glob')) const MiniPass = require('minipass') @@ -197,7 +197,7 @@ class LogFiles { try { const logPath = this.#getLogFilePath() - const logGlob = path.join(path.dirname(logPath), path.basename(logPath) + const logGlob = join(dirname(logPath), basename(logPath) // tell glob to only match digits .replace(/\d/g, '[0123456789]') // Handle the old (prior to 8.2.0) log file names which did not have a diff --git a/lib/utils/npm-usage.js b/lib/utils/npm-usage.js index 947a3073bc5ff..b04ad33f9dd79 100644 --- a/lib/utils/npm-usage.js +++ b/lib/utils/npm-usage.js @@ -1,4 +1,3 @@ -const { dirname } = require('path') const { commands } = require('./cmd-list') const COL_MAX = 60 @@ -36,7 +35,7 @@ or on the command line via: npm --key=value More configuration info: npm help config Configuration fields: npm help 7 config -npm@${npm.version} ${dirname(dirname(__dirname))}` +npm@${npm.version} ${npm.npmRoot}` } const cmdNames = () => { diff --git a/lib/utils/open-url.js b/lib/utils/open-url.js index 379640773fa6e..f882d0c9d3934 100644 --- a/lib/utils/open-url.js +++ b/lib/utils/open-url.js @@ -31,7 +31,7 @@ const open = async (npm, url, errMsg, isFile) => { if (!/^https?:$/.test(new URL(url).protocol)) { throw new Error() } - } catch (_) { + } catch { throw new Error('Invalid URL: ' + url) } } diff --git a/lib/utils/queryable.js b/lib/utils/queryable.js index 7c5bb7fe87baf..6acc1758ceea7 100644 --- a/lib/utils/queryable.js +++ b/lib/utils/queryable.js @@ -1,5 +1,4 @@ const util = require('util') -const _data = Symbol('data') const _delete = Symbol('delete') const _append = Symbol('append') @@ -236,6 +235,8 @@ const setter = ({ data, key, value, force }) => { } class Queryable { + #data = null + constructor (obj) { if (!obj || typeof obj !== 'object') { throw Object.assign(new Error('Queryable needs an object to query properties from.'), { @@ -243,7 +244,7 @@ class Queryable { }) } - this[_data] = obj + this.#data = obj } query (queries) { @@ -251,12 +252,12 @@ class Queryable { // with the legacy API lib/view.js is consuming, if at some point // we refactor that command then we can revisit making this nicer if (queries === '') { - return { '': this[_data] } + return { '': this.#data } } const q = query => getter({ - data: this[_data], + data: this.#data, key: query, }) @@ -283,7 +284,7 @@ class Queryable { // and assigns `value` to the last property of the query chain set (query, value, { force } = {}) { setter({ - data: this[_data], + data: this.#data, key: query, value, force, @@ -293,14 +294,14 @@ class Queryable { // deletes the value of the property found at `query` delete (query) { setter({ - data: this[_data], + data: this.#data, key: query, value: _delete, }) } toJSON () { - return this[_data] + return this.#data } [util.inspect.custom] () { diff --git a/lib/utils/read-user-info.js b/lib/utils/read-user-info.js index ac24396c6abb9..26d5b36d55b58 100644 --- a/lib/utils/read-user-info.js +++ b/lib/utils/read-user-info.js @@ -28,7 +28,7 @@ function readOTP (msg = otpPrompt, otp, isRetry) { } return read({ prompt: msg, default: otp || '' }) - .then((otp) => readOTP(msg, otp, true)) + .then((rOtp) => readOTP(msg, rOtp, true)) } function readPassword (msg = passwordPrompt, password, isRetry) { @@ -37,7 +37,7 @@ function readPassword (msg = passwordPrompt, password, isRetry) { } return read({ prompt: msg, silent: true, default: password || '' }) - .then((password) => readPassword(msg, password, true)) + .then((rPassword) => readPassword(msg, rPassword, true)) } function readUsername (msg = usernamePrompt, username, isRetry) { @@ -51,7 +51,7 @@ function readUsername (msg = usernamePrompt, username, isRetry) { } return read({ prompt: msg, default: username || '' }) - .then((username) => readUsername(msg, username, true)) + .then((rUsername) => readUsername(msg, rUsername, true)) } function readEmail (msg = emailPrompt, email, isRetry) { diff --git a/lib/utils/reify-output.js b/lib/utils/reify-output.js index b5c3a593b8db0..5ac7fa4b01896 100644 --- a/lib/utils/reify-output.js +++ b/lib/utils/reify-output.js @@ -12,7 +12,7 @@ const log = require('./log-shim.js') const { depth } = require('treeverse') const ms = require('ms') -const auditReport = require('npm-audit-report') +const npmAuditReport = require('npm-audit-report') const { readTree: getFundingInfo } = require('libnpmfund') const auditError = require('./audit-error.js') @@ -112,7 +112,7 @@ const getAuditReport = (npm, report) => { const defaultAuditLevel = npm.command !== 'audit' ? 'none' : 'low' const auditLevel = npm.flatOptions.auditLevel || defaultAuditLevel - const res = auditReport(report, { + const res = npmAuditReport(report, { reporter, ...npm.flatOptions, auditLevel, diff --git a/lib/workspaces/get-workspaces.js b/lib/workspaces/get-workspaces.js index 373af1d689cc3..2ac043d5f3943 100644 --- a/lib/workspaces/get-workspaces.js +++ b/lib/workspaces/get-workspaces.js @@ -42,7 +42,7 @@ const getWorkspaces = async (filters, { path, includeWorkspaceRoot, relativeFrom let msg = '!' if (filters.length) { msg = `:\n ${filters.reduce( - (res, filterArg) => `${res} --workspace=${filterArg}`, '')}` + (acc, filterArg) => `${acc} --workspace=${filterArg}`, '')}` } throw new Error(`No workspaces found${msg}`) diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index 637da6f6feb7e..a89c8b72b7d58 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -39,7 +39,7 @@ class MockRegistry { t.fail(`Unmatched request: ${JSON.stringify(req.options, null, 2)}`) } if (debug) { - console.error('NO MATCH', t.name, req.options) + console.error('NO MATCH', t.name, req.options ? req.options : req.path) } } @@ -314,6 +314,20 @@ class MockRegistry { return nock } + getPackage (name, { times = 1, code = 200, query, resp = {} }) { + let nock = this.nock + nock = nock.get(`/${npa(name).escapedName}`).times(times) + if (query) { + nock = nock.query(query) + } + if (code === 404) { + nock = nock.reply(code, { error: 'Not found' }) + } else { + nock = nock.reply(code, resp) + } + this.nock = nock + } + async package ({ manifest, times = 1, query, tarballs }) { let nock = this.nock const spec = npa(manifest.name) diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 8038dfa30b6c2..d65e3d0745217 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -2380,6 +2380,7 @@ Array [ "tag", "tag-version-prefix", "umask", + "unicode", "user-agent", "workspace", "workspaces", @@ -2409,7 +2410,6 @@ Array [ "prefix", "timing", "tmp", - "unicode", "update-notifier", "usage", "userconfig", diff --git a/test/bin/npx-cli.js b/test/bin/npx-cli.js index b526f2dfbe32e..b456d9905f77d 100644 --- a/test/bin/npx-cli.js +++ b/test/bin/npx-cli.js @@ -1,12 +1,12 @@ const t = require('tap') +const mockGlobals = require('../fixtures/mock-globals') const npx = require.resolve('../../bin/npx-cli.js') const cli = require.resolve('../../lib/cli.js') const npm = require.resolve('../../bin/npm-cli.js') const logs = [] -console.error = (...msg) => logs.push(msg) - t.afterEach(() => (logs.length = 0)) +mockGlobals(t, { 'console.error': (...msg) => logs.push(msg) }) t.test('npx foo -> npm exec -- foo', t => { process.argv = ['node', npx, 'foo'] diff --git a/test/fixtures/clean-snapshot.js b/test/fixtures/clean-snapshot.js index b0ea28cee4d81..83ddc00f4b787 100644 --- a/test/fixtures/clean-snapshot.js +++ b/test/fixtures/clean-snapshot.js @@ -1,19 +1,43 @@ +const { relative, dirname } = require('path') + +// normalize line endings (for ini) +const cleanNewlines = (s) => s.replace(/\r\n/g, '\n') + // XXX: this also cleans quoted " in json snapshots // ideally this could be avoided but its easier to just // run this command inside cleanSnapshot -const normalizePath = (str) => str - .replace(/\r\n/g, '\n') // normalize line endings (for ini) +const normalizePath = (str) => cleanNewlines(str) .replace(/[A-z]:\\/g, '\\') // turn windows roots to posix ones .replace(/\\+/g, '/') // replace \ with / +const pathRegex = (p) => new RegExp(normalizePath(p), 'gi') + +// create a cwd replacer in the module scope, since some tests +// overwrite process.cwd() +const CWD = pathRegex(process.cwd()) +const TESTDIR = pathRegex(relative(process.cwd(), dirname(require.main.filename))) + const cleanCwd = (path) => normalizePath(path) - .replace(new RegExp(normalizePath(process.cwd()), 'g'), '{CWD}') + // repalce CWD, TESTDIR, and TAPDIR separately + .replace(CWD, '{CWD}') + .replace(TESTDIR, '{TESTDIR}') + .replace(/tap-testdir-[\w-.]+/gi, '{TAPDIR}') + // if everything ended up in line, reduce it all to CWD + .replace(/\{CWD\}\/\{TESTDIR\}\/\{TAPDIR\}/g, '{CWD}') + // replace for platform differences in global nodemodules + .replace(/lib\/node_modules/g, 'node_modules') + .replace(/global\/lib/g, 'global') const cleanDate = (str) => str.replace(/\d{4}-\d{2}-\d{2}T\d{2}[_:]\d{2}[_:]\d{2}[_:.]\d{3}Z/g, '{DATE}') +const cleanTime = str => str.replace(/in [0-9]+m?s\s*$/gm, 'in {TIME}') + module.exports = { normalizePath, + pathRegex, cleanCwd, cleanDate, + cleanTime, + cleanNewlines, } diff --git a/test/fixtures/merge-conflict.json b/test/fixtures/merge-conflict.json new file mode 100644 index 0000000000000..2591c62efb37a --- /dev/null +++ b/test/fixtures/merge-conflict.json @@ -0,0 +1,36 @@ +{ + "array": [ +<<<<<<< HEAD + 100, + { + "foo": "baz" + }, +||||||| merged common ancestors + 1, +======= + 111, + 1, + 2, + 3, + { + "foo": "bar" + }, +>>>>>>> a + 1 + ], + "a": { + "b": { +<<<<<<< HEAD + "c": { + "x": "bbbb" + } +||||||| merged common ancestors + "c": { + "x": "aaaa" + } +======= + "c": "xxxx" +>>>>>>> a + } + } +} diff --git a/test/fixtures/mock-globals.js b/test/fixtures/mock-globals.js index 29da2a48b092d..17b2b156c17ab 100644 --- a/test/fixtures/mock-globals.js +++ b/test/fixtures/mock-globals.js @@ -4,15 +4,25 @@ // Hopefully it can be removed for a feature in tap in the future const sep = '.' +const reLastSep = new RegExp(`\\${sep}(?=[^${sep}]+$)`) const has = (o, k) => Object.prototype.hasOwnProperty.call(o, k) const opd = (o, k) => Object.getOwnPropertyDescriptor(o, k) const po = (o) => Object.getPrototypeOf(o) const pojo = (o) => Object.prototype.toString.call(o) === '[object Object]' const last = (arr) => arr[arr.length - 1] -const splitLast = (str) => str.split(new RegExp(`\\${sep}(?=[^${sep}]+$)`)) const dupes = (arr) => arr.filter((k, i) => arr.indexOf(k) !== i) const dupesStartsWith = (arr) => arr.filter((k1) => arr.some((k2) => k2.startsWith(k1 + sep))) +const splitLast = (str) => { + const hasNerf = str.includes('process.env') && str.includes('//') + if (hasNerf) { + const startPosition = str.indexOf('//') + const index = str.lastIndexOf(sep, startPosition) + return [str.slice(0, index), str.slice(index + 1)] + } + return str.split(reLastSep) +} + // A weird getter that can look up keys on nested objects but also // match keys with dots in their names, eg { 'process.env': { TERM: 'a' } } // can be looked up with the key 'process.env.TERM' diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 8a744cd559eaf..0d9c98d6910b6 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -1,14 +1,45 @@ const os = require('os') const fs = require('fs').promises const path = require('path') +const tap = require('tap') +const errorMessage = require('../../lib/utils/error-message') const mockLogs = require('./mock-logs') const mockGlobals = require('./mock-globals') -const log = require('../../lib/utils/log-shim') -const envConfigKeys = Object.keys(require('../../lib/utils/config/definitions.js')) +const defExitCode = process.exitCode + +const setGlobalNodeModules = (globalDir) => { + const updateSymlinks = (obj, visit) => { + for (const [key, value] of Object.entries(obj)) { + if (/Fixture/.test(value.toString())) { + obj[key] = tap.fixture('symlink', path.join('..', value.content)) + } else if (typeof value === 'object') { + obj[key] = updateSymlinks(value, visit) + } + } + return obj + } -const RealMockNpm = (t, otherMocks = {}) => { + if (globalDir.lib) { + throw new Error('`globalPrefixDir` should not have a top-level `lib/` directory, only a ' + + 'top-level `node_modules/` dir that gets set in the correct location based on platform. ' + + `Received the following top level entries: ${Object.keys(globalDir).join(', ')}.` + ) + } + + if (process.platform !== 'win32' && globalDir.node_modules) { + const { node_modules: nm, ...rest } = globalDir + return { + ...rest, + lib: { node_modules: updateSymlinks(nm) }, + } + } + + return globalDir +} + +const getMockNpm = async (t, { mocks, globals, npm, init, load }) => { const mock = { - ...mockLogs(otherMocks), + ...mockLogs(mocks), outputs: [], outputErrors: [], joinedOutput: () => mock.outputs.map(o => o.join(' ')).join('\n'), @@ -16,11 +47,30 @@ const RealMockNpm = (t, otherMocks = {}) => { const Npm = t.mock('../../lib/npm.js', { '../../lib/utils/update-notifier.js': async () => {}, - ...otherMocks, + ...mocks, ...mock.logMocks, }) mock.Npm = class MockNpm extends Npm { + constructor () { + mockGlobals(t, globals) + super(npm) + } + + async exec (...args) { + const [res, err] = await super.exec(...args).then((r) => [r]).catch(e => [null, e]) + // This mimics how the exit handler flushes output for commands that have + // buffered output. It also uses the same json error processing from the + // error message fn. This is necessary for commands with buffered output + // to read the output after exec is called. This is not *exactly* how it + // works in practice, but it is close enough for now. + this.flushOutput(err ? errorMessage(err, this).json : null) + if (err) { + throw err + } + return res + } + // lib/npm.js tests needs this to actually test the function! originalOutput (...args) { super.output(...args) @@ -39,77 +89,91 @@ const RealMockNpm = (t, otherMocks = {}) => { } } - return mock -} - -const setLoglevel = (t, loglevel, reset = true) => { - if (t && reset) { - const _level = log.level - t.teardown(() => log.level = _level) + if (init) { + mock.npm = new mock.Npm() + if (load) { + await mock.npm.load() + } } - if (loglevel) { - // Set log level on the npmlog singleton and shared across everything - log.level = loglevel - } + return mock } -// Resolve some options to a function call with supplied args -const result = (fn, ...args) => typeof fn === 'function' ? fn(...args) : fn +const mockNpms = new Map() -const LoadMockNpm = async (t, { +const setupMockNpm = async (t, { init = true, load = init, + // preload a command + command = null, // string name of the command + exec = null, // optionally exec the command before returning + // test dirs prefixDir = {}, homeDir = {}, cacheDir = {}, - globalPrefixDir = { lib: {} }, - config = {}, - mocks = {}, + globalPrefixDir = { node_modules: {} }, otherDirs = {}, - globals = null, + // setup config, env vars, mocks, npm opts + config: _config = {}, + mocks = {}, + globals = {}, + npm: npmOpts = {}, + argv: rawArgv = [], } = {}) => { - // Mock some globals with their original values so they get torn down - // back to the original at the end of the test since they are manipulated - // by npm itself - const npmConfigEnv = {} - for (const key in process.env) { - if (key.startsWith('npm_config_')) { - npmConfigEnv[key] = undefined + // easy to accidentally forget to pass in tap + if (!(t instanceof tap.Test)) { + throw new Error('first argument must be a tap instance') + } + + // mockNpm is designed to only be run once per test chain so we assign it to + // the test in the cache and error if it is attempted to run again + let tapInstance = t + while (tapInstance) { + if (mockNpms.has(tapInstance)) { + throw new Error('mockNpm can only be called once in each t.test chain') } + tapInstance = tapInstance.parent + } + mockNpms.set(t, true) + + if (!init && load) { + throw new Error('cant `load` without `init`') + } + + if (!init && load) { + throw new Error('cant `load` without `init`') } + + const npmEnvs = Object.keys(process.env).filter(k => k.startsWith('npm_')) + + // These are globals manipulated by npm itself that we need to reset to their + // original values between tests mockGlobals(t, { process: { title: process.title, execPath: process.execPath, env: { - npm_command: process.env.npm_command, + NODE_ENV: process.env.NODE_ENV, COLOR: process.env.COLOR, - ...npmConfigEnv, + // further, these are npm controlled envs that we need to zero out before + // before the test. setting them to undefined ensures they are not set and + // also returned to their original value after the test + ...npmEnvs.reduce((acc, k) => { + acc[k] = undefined + return acc + }, {}), }, }, }) - const { Npm, ...rest } = RealMockNpm(t, mocks) - - // We want to fail fast when writing tests. Default this to 0 unless it was - // explicitly set in a test. - config = { 'fetch-retries': 0, ...config } - - if (!init && load) { - throw new Error('cant `load` without `init`') - } - - // Set log level as early as possible since - setLoglevel(t, config.loglevel) - const dir = t.testdir({ home: homeDir, prefix: prefixDir, cache: cacheDir, - global: globalPrefixDir, + global: setGlobalNodeModules(globalPrefixDir), other: otherDirs, }) + const dirs = { testdir: dir, prefix: path.join(dir, 'prefix'), @@ -119,52 +183,93 @@ const LoadMockNpm = async (t, { other: path.join(dir, 'other'), } - // Set cache to testdir via env var so it is available when load is run - // XXX: remove this for a solution where cache argv is passed in - mockGlobals(t, { + // Option objects can also be functions that are called with all the dir paths + // so they can be used to set configs that need to be based on paths + const withDirs = (v) => typeof v === 'function' ? v(dirs) : v + + const { argv, env, config } = Object.entries({ + // We want to fail fast when writing tests. Default this to 0 unless it was + // explicitly set in a test. + 'fetch-retries': 0, + cache: dirs.cache, + ...withDirs(_config), + }) + .reduce((acc, [key, value]) => { + // nerfdart configs passed in need to be set via env var instead of argv + if (key.startsWith('//')) { + acc.env[`process.env.npm_config_${key}`] = value + } else { + const values = [].concat(value) + acc.argv.push(...values.flatMap(v => [`--${key}`, v.toString()])) + } + acc.config[key] = value + return acc + }, { argv: [...rawArgv], env: {}, config: {} }) + + // process.cwd shouldnt be mocked unless we are actually initializing npm + // here, since it messes with other things like t.mock paths + const { 'process.cwd': processCwd, ...mockedGlobals } = { 'process.env.HOME': dirs.home, - 'process.env.npm_config_cache': dirs.cache, - ...(globals ? result(globals, { ...dirs }) : {}), - // Some configs don't work because they can't be set via npm.config.set until - // config is loaded. But some config items are needed before that. So this is - // an explicit set of configs that must be loaded as env vars. - // XXX(npm9): make this possible by passing in argv directly to npm/config - ...Object.entries(config) - .filter(([k]) => envConfigKeys.includes(k)) - .reduce((acc, [k, v]) => { - acc[`process.env.npm_config_${k.replace(/-/g, '_')}`] = - result(v, { ...dirs }).toString() - return acc - }, {}), + // global prefix and prefix cannot be (easily) set via argv + // so this is the easiest way to set them that also closely mimics the + // behavior a user would see since they will already be set while + // `npm.load()` is being run + 'process.env.PREFIX': dirs.globalPrefix, + 'process.cwd': () => dirs.prefix, + ...withDirs(globals), + } + + mockGlobals(t, mockedGlobals) + + const { npm, ...mockNpm } = await getMockNpm(t, { + init, + load, + mocks: withDirs(mocks), + npm: { argv, excludeNpmCwd: true, ...withDirs(npmOpts) }, + globals: { ...env, 'process.cwd': processCwd }, }) - const npm = init ? new Npm() : null + if (config.omit?.includes('prod')) { + // XXX: --omit=prod is not a valid config according to the definitions but + // it was being hacked in via flatOptions for older tests so this is to + // preserve that behavior and reduce churn in the snapshots. this should be + // removed or fixed in the future + npm.flatOptions.omit.push('prod') + } + t.teardown(() => { - npm && npm.unload() + if (npm) { + npm.unload() + } + // only set exitCode back if we're passing tests + if (t.passing()) { + process.exitCode = defExitCode + } }) - if (load) { - await npm.load() - for (const [k, v] of Object.entries(result(config, { npm, ...dirs }))) { - if (typeof v === 'object' && v.value && v.where) { - npm.config.set(k, v.value, v.where) - } else { - npm.config.set(k, v) - } + const mockCommand = {} + if (command) { + const cmd = await npm.cmd(command) + const usage = await cmd.usage + mockCommand.cmd = cmd + mockCommand[command] = { + usage, + exec: (args) => npm.exec(command, args), + completion: (args) => cmd.completion(args), + } + if (exec) { + await mockCommand[command].exec(exec) + // assign string output to the command now that we have it + // for easier testing + mockCommand[command].output = mockNpm.joinedOutput() } - // Set global loglevel *again* since it possibly got reset during load - // XXX: remove with npmlog - setLoglevel(t, config.loglevel, false) - npm.prefix = dirs.prefix - npm.cache = dirs.cache - npm.globalPrefix = dirs.globalPrefix } return { - ...rest, - ...dirs, - Npm, npm, + ...mockNpm, + ...dirs, + ...mockCommand, debugFile: async () => { const readFiles = npm.logFiles.map(f => fs.readFile(f)) const logFiles = await Promise.all(readFiles) @@ -180,80 +285,6 @@ const LoadMockNpm = async (t, { } } -const realConfig = require('../../lib/utils/config') - -// Basic npm fixture that you can give a config object that acts like -// npm.config You still need a separate flatOptions. Tests should migrate to -// using the real npm mock above -class MockNpm { - constructor (base = {}, t) { - this._mockOutputs = [] - this.isMockNpm = true - this.base = base - - const config = base.config || {} - - for (const attr in base) { - if (attr !== 'config') { - this[attr] = base[attr] - } - } - - this.flatOptions = base.flatOptions || {} - this.config = { - // for now just set `find` to what config.find should return - // this works cause `find` is not an existing config entry - find: (k) => ({ ...realConfig.defaults, ...config })[k], - // for now isDefault is going to just return false if a value was defined - isDefault: (k) => !Object.prototype.hasOwnProperty.call(config, k), - get: (k) => ({ ...realConfig.defaults, ...config })[k], - set: (k, v) => { - config[k] = v - // mock how real npm derives silent - if (k === 'loglevel') { - this.flatOptions.silent = v === 'silent' - this.silent = v === 'silent' - } - }, - list: [{ ...realConfig.defaults, ...config }], - validate: () => {}, - } - - if (t && config.loglevel) { - setLoglevel(t, config.loglevel) - } - - if (config.loglevel) { - this.config.set('loglevel', config.loglevel) - } - } - - get global () { - return this.config.get('global') || this.config.get('location') === 'global' - } - - output (...msg) { - if (this.base.output) { - return this.base.output(msg) - } - this._mockOutputs.push(msg) - } - - // with the older fake mock npm there is no - // difference between output and outputBuffer - // since it just collects the output and never - // calls the exit handler, so we just mock the - // method the same as output. - outputBuffer (...msg) { - this.output(...msg) - } -} - -const FakeMockNpm = (base = {}, t) => { - return new MockNpm(base, t) -} - -module.exports = { - fake: FakeMockNpm, - load: LoadMockNpm, -} +module.exports = setupMockNpm +module.exports.load = setupMockNpm +module.exports.setGlobalNodeModules = setGlobalNodeModules diff --git a/test/lib/arborist-cmd.js b/test/lib/arborist-cmd.js index f3c1d2573d33f..64d260552f849 100644 --- a/test/lib/arborist-cmd.js +++ b/test/lib/arborist-cmd.js @@ -1,115 +1,142 @@ const { resolve } = require('path') const t = require('tap') -const ArboristCmd = require('../../lib/arborist-cmd.js') -const configMock = { - validate: () => {}, - get: (key) => { - if (key === 'location') { - return 'project' - } - }, - isDefault: () => {}, -} +const { load: loadMockNpm } = require('../fixtures/mock-npm') -t.test('arborist-cmd', async t => { - const path = t.testdir({ - 'package.json': JSON.stringify({ - name: 'simple-workspaces-list', - version: '1.1.1', - workspaces: [ - 'a', - 'b', - 'group/*', - ], - }), - node_modules: { - abbrev: { - 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.1' }), +const mockArboristCmd = async (t, exec, workspace, { mocks = {}, ...opts } = {}) => { + const ArboristCmd = t.mock('../../lib/arborist-cmd.js', mocks) + + const config = (typeof workspace === 'function') + ? (dirs) => ({ workspace: workspace(dirs) }) + : { workspace } + + const mock = await loadMockNpm(t, { + config, + prefixDir: { + 'package.json': JSON.stringify({ + name: 'simple-workspaces-list', + version: '1.1.1', + workspaces: [ + 'a', + 'b', + 'group/*', + ], + }), + node_modules: { + abbrev: { + 'package.json': JSON.stringify({ name: 'abbrev', version: '1.1.1' }), + }, + a: t.fixture('symlink', '../a'), + b: t.fixture('symlink', '../b'), }, - a: t.fixture('symlink', '../a'), - b: t.fixture('symlink', '../b'), - }, - a: { - 'package.json': JSON.stringify({ name: 'a', version: '1.0.0' }), - }, - b: { - 'package.json': JSON.stringify({ name: 'b', version: '1.0.0' }), - }, - group: { - c: { - 'package.json': JSON.stringify({ - name: 'c', - version: '1.0.0', - dependencies: { - abbrev: '^1.1.1', - }, - }), + a: { + 'package.json': JSON.stringify({ name: 'a', version: '1.0.0' }), + }, + b: { + 'package.json': JSON.stringify({ name: 'b', version: '1.0.0' }), }, - d: { - 'package.json': JSON.stringify({ name: 'd', version: '1.0.0' }), + group: { + c: { + 'package.json': JSON.stringify({ + name: 'c', + version: '1.0.0', + dependencies: { + abbrev: '^1.1.1', + }, + }), + }, + d: { + 'package.json': JSON.stringify({ name: 'd', version: '1.0.0' }), + }, }, }, + ...opts, }) - class TestCmd extends ArboristCmd {} - - const cmd = new TestCmd({ localPrefix: path, config: configMock }) - - // check filtering for a single workspace name - cmd.exec = async function (args) { - t.same(this.workspaceNames, ['a'], 'should set array with single ws name') - t.same(args, ['foo'], 'should get received args') + let execArg + class TestCmd extends ArboristCmd { + async exec (arg) { + execArg = arg + } } - await cmd.execWorkspaces(['foo'], ['a']) - // check filtering single workspace by path - cmd.exec = async function (args) { - t.same(this.workspaceNames, ['a'], - 'should set array with single ws name from path') + const cmd = new TestCmd(mock.npm) + if (exec) { + await cmd.execWorkspaces(exec) } - await cmd.execWorkspaces([], ['./a']) - // check filtering single workspace by full path - cmd.exec = function (args) { - t.same(this.workspaceNames, ['a'], - 'should set array with single ws name from full path') - } - await cmd.execWorkspaces([], [resolve(path, './a')]) + return { ...mock, cmd, getArg: () => execArg } +} - // filtering multiple workspaces by name - cmd.exec = async function (args) { - t.same(this.workspaceNames, ['a', 'c'], - 'should set array with multiple listed ws names') - } - await cmd.execWorkspaces([], ['a', 'c']) +t.test('arborist-cmd', async t => { + await t.test('single name', async t => { + const { cmd, getArg } = await mockArboristCmd(t, ['foo'], 'a') - // filtering multiple workspaces by path names - cmd.exec = async function (args) { - t.same(this.workspaceNames, ['a', 'c'], - 'should set array with multiple ws names from paths') - } - await cmd.execWorkspaces([], ['./a', 'group/c']) + t.same(cmd.workspaceNames, ['a'], 'should set array with single ws name') + t.same(getArg(), ['foo'], 'should get received args') + }) - // filtering multiple workspaces by parent path name - cmd.exec = async function (args) { - t.same(this.workspaceNames, ['c', 'd'], - 'should set array with multiple ws names from a parent folder name') - } - await cmd.execWorkspaces([], ['./group']) + await t.test('single path', async t => { + const { cmd } = await mockArboristCmd(t, [], './a') + + t.same(cmd.workspaceNames, ['a'], 'should set array with single ws name') + }) + + await t.test('single full path', async t => { + const { cmd } = await mockArboristCmd(t, [], ({ prefix }) => resolve(prefix, 'a')) + + t.same(cmd.workspaceNames, ['a'], 'should set array with single ws name') + }) + + await t.test('multiple names', async t => { + const { cmd } = await mockArboristCmd(t, [], ['a', 'c']) + + t.same(cmd.workspaceNames, ['a', 'c'], 'should set array with single ws name') + }) + + await t.test('multiple paths', async t => { + const { cmd } = await mockArboristCmd(t, [], ['./a', 'group/c']) + + t.same(cmd.workspaceNames, ['a', 'c'], 'should set array with single ws name') + }) + + await t.test('parent path', async t => { + const { cmd } = await mockArboristCmd(t, [], './group') + + t.same(cmd.workspaceNames, ['c', 'd'], 'should set array with single ws name') + }) + + await t.test('parent path', async t => { + const { cmd } = await mockArboristCmd(t, [], './group') + + t.same(cmd.workspaceNames, ['c', 'd'], 'should set array with single ws name') + }) + + await t.test('prefix inside cwd', async t => { + const { npm, cmd, prefix } = await mockArboristCmd(t, null, ['a', 'c'], { + globals: (dirs) => ({ + 'process.cwd': () => dirs.testdir, + }), + }) + + npm.localPrefix = prefix + await cmd.execWorkspaces([]) + + t.same(cmd.workspaceNames, ['a', 'c'], 'should set array with single ws name') + }) }) t.test('handle getWorkspaces raising an error', async t => { - const ArboristCmd = t.mock('../../lib/arborist-cmd.js', { - '../../lib/workspaces/get-workspaces.js': async () => { - throw new Error('oopsie') + const { cmd } = await mockArboristCmd(t, null, 'a', { + mocks: { + '../../lib/workspaces/get-workspaces.js': async () => { + throw new Error('oopsie') + }, }, }) - class TestCmd extends ArboristCmd {} - const cmd = new TestCmd({ localPrefix: t.testdir(), config: configMock }) await t.rejects( - cmd.execWorkspaces(['foo'], ['a']), + cmd.execWorkspaces(['foo']), { message: 'oopsie' } ) }) diff --git a/test/lib/cli.js b/test/lib/cli.js index 42a22a20b3964..b77e8fb7bbc45 100644 --- a/test/lib/cli.js +++ b/test/lib/cli.js @@ -1,5 +1,4 @@ const t = require('tap') - const { load: loadMockNpm } = require('../fixtures/mock-npm.js') const cliMock = async (t, opts) => { @@ -42,24 +41,18 @@ t.test('print the version, and treat npm_g as npm -g', async t => { t.strictSame(process.argv, ['node', 'npm', '-g', '-v'], 'system process.argv was rewritten') t.strictSame(logsBy('cli'), [['node npm']]) t.strictSame(logsBy('title'), [['npm']]) - t.strictSame(logsBy('argv'), [['"--global" "--version"']]) + t.match(logsBy('argv'), [['"--global" "--version"']]) t.strictSame(logs.info, [ ['using', 'npm@%s', Npm.version], ['using', 'node@%s', process.version], ]) + t.equal(outputs.length, 1) t.strictSame(outputs, [[Npm.version]]) t.strictSame(exitHandlerCalled(), []) }) t.test('calling with --versions calls npm version with no args', async t => { const { logsBy, cli, outputs, exitHandlerCalled } = await cliMock(t, { - mocks: { - '../../lib/commands/version.js': class Version { - async exec (args) { - t.strictSame(args, []) - } - }, - }, globals: { 'process.argv': ['node', 'npm', 'install', 'or', 'whatever', '--versions'], }, @@ -69,18 +62,14 @@ t.test('calling with --versions calls npm version with no args', async t => { t.equal(process.title, 'npm install or whatever') t.strictSame(logsBy('cli'), [['node npm']]) t.strictSame(logsBy('title'), [['npm install or whatever']]) - t.strictSame(logsBy('argv'), [['"install" "or" "whatever" "--versions"']]) - t.strictSame(outputs, []) + t.match(logsBy('argv'), [['"install" "or" "whatever" "--versions"']]) + t.equal(outputs.length, 1) + t.match(outputs[0][0], { npm: String, node: String, v8: String }) t.strictSame(exitHandlerCalled(), []) }) t.test('logged argv is sanitized', async t => { const { logsBy, cli } = await cliMock(t, { - mocks: { - '../../lib/commands/version.js': class Version { - async exec () {} - }, - }, globals: { 'process.argv': [ 'node', @@ -96,16 +85,11 @@ t.test('logged argv is sanitized', async t => { t.equal(process.title, 'npm version') t.strictSame(logsBy('cli'), [['node npm']]) t.strictSame(logsBy('title'), [['npm version']]) - t.strictSame(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org/password"']]) + t.match(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org/password"']]) }) t.test('logged argv is sanitized with equals', async t => { const { logsBy, cli } = await cliMock(t, { - mocks: { - '../../lib/commands/version.js': class Version { - async exec () {} - }, - }, globals: { 'process.argv': [ 'node', @@ -117,7 +101,7 @@ t.test('logged argv is sanitized with equals', async t => { }) await cli(process) - t.strictSame(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org"']]) + t.match(logsBy('argv'), [['"version" "--registry" "https://u:***@npmjs.org"']]) }) t.test('print usage if no params provided', async t => { diff --git a/test/lib/commands/config.js b/test/lib/commands/config.js index 35872e722e17e..f2bdcc7231ddf 100644 --- a/test/lib/commands/config.js +++ b/test/lib/commands/config.js @@ -26,16 +26,10 @@ t.test('config ignores workspaces', async t => { await t.rejects( sandbox.run('config', ['--workspaces']), { - code: 'EUSAGE', + code: 'ENOWORKSPACES', }, 'rejects with usage' ) - - t.match( - sandbox.logs.warn, - [['config', 'This command does not support workspaces.']], - 'logged the warning' - ) }) t.test('config list', async t => { diff --git a/test/lib/fixtures/mock-globals.js b/test/lib/fixtures/mock-globals.js index 02566e575af5e..ef3d5637032b7 100644 --- a/test/lib/fixtures/mock-globals.js +++ b/test/lib/fixtures/mock-globals.js @@ -1,6 +1,7 @@ const t = require('tap') const mockGlobals = require('../../fixtures/mock-globals') +/* eslint-disable no-console */ const originals = { platform: process.platform, error: console.error, @@ -28,6 +29,7 @@ t.test('console', async t => { t.equal(console.error, originals.error) }) +/* eslint-enable no-console */ t.test('platform', async (t) => { t.equal(process.platform, originals.platform) @@ -299,11 +301,11 @@ t.test('multiple mocks and resets', async (t) => { await t.test('platforms', async (t) => { const resets = platforms.map((p) => { - const { teardown, reset } = mockGlobals(t, { 'process.platform': p }) + const { teardown: nestedTeardown, reset } = mockGlobals(t, { 'process.platform': p }) t.equal(process.platform, p) return [ reset['process.platform'], - teardown, + nestedTeardown, ] }) diff --git a/test/lib/load-all-commands.js b/test/lib/load-all-commands.js index aaf6a69c27cd6..dd55560369310 100644 --- a/test/lib/load-all-commands.js +++ b/test/lib/load-all-commands.js @@ -7,28 +7,41 @@ const util = require('util') const { load: loadMockNpm } = require('../fixtures/mock-npm.js') const { allCommands } = require('../../lib/utils/cmd-list.js') +const isAsyncFn = (v) => typeof v === 'function' && /^\[AsyncFunction:/.test(util.inspect(v)) + t.test('load each command', async t => { for (const cmd of allCommands) { t.test(cmd, async t => { - const { npm, outputs } = await loadMockNpm(t, { + const { npm, outputs, cmd: impl } = await loadMockNpm(t, { + command: cmd, config: { usage: true }, }) - const impl = await npm.cmd(cmd) + const ctor = impl.constructor + if (impl.completion) { t.type(impl.completion, 'function', 'completion, if present, is a function') } - t.type(impl.exec, 'function', 'implementation has an exec function') - t.type(impl.execWorkspaces, 'function', 'implementation has an execWorkspaces function') - t.equal(util.inspect(impl.exec), '[AsyncFunction: exec]', 'exec function is async') - t.equal( - util.inspect(impl.execWorkspaces), - '[AsyncFunction: execWorkspaces]', - 'execWorkspaces function is async' - ) + + // exec fn + t.ok(isAsyncFn(impl.exec), 'exec is async') + t.ok(impl.exec.length <= 1, 'exec fn has 0 or 1 args') + + // workspaces + t.type(ctor.ignoreImplicitWorkspace, 'boolean', 'ctor has ignoreImplictWorkspace boolean') + t.type(ctor.workspaces, 'boolean', 'ctor has workspaces boolean') + if (ctor.workspaces) { + t.ok(isAsyncFn(impl.execWorkspaces), 'execWorkspaces is async') + t.ok(impl.exec.length <= 1, 'execWorkspaces fn has 0 or 1 args') + } else { + t.notOk(impl.execWorkspaces, 'has no execWorkspaces fn') + } + + // name/desc t.ok(impl.description, 'implementation has a description') t.ok(impl.name, 'implementation has a name') t.equal(cmd, impl.name, 'command list and name are the same') - t.ok(impl.ignoreImplicitWorkspace !== undefined, 'implementation has ignoreImplictWorkspace') + + // usage t.match(impl.usage, cmd, 'usage contains the command') await npm.exec(cmd, []) t.match(outputs[0][0], impl.usage, 'usage is what is output') diff --git a/test/lib/npm.js b/test/lib/npm.js index f850ff6aff8a4..2dd692a404170 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -1,39 +1,10 @@ const t = require('tap') const { resolve, dirname, join } = require('path') const fs = require('fs') - const { load: loadMockNpm } = require('../fixtures/mock-npm.js') const mockGlobals = require('../fixtures/mock-globals') const { commands } = require('../../lib/utils/cmd-list.js') -// delete this so that we don't have configs from the fact that it -// is being run by 'npm test' -const event = process.env.npm_lifecycle_event - -for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e))) { - if (env === 'npm_command') { - // should only be running this in the 'test' or 'run-script' command! - // if the lifecycle event is 'test', then it'll be either 'test' or 'run', - // otherwise it should always be run-script. Of course, it'll be missing - // if this test is just run directly, which is also acceptable. - if (event === 'test') { - t.ok( - ['test', 'run-script'].some(i => i === process.env[env]), - 'should match "npm test" or "npm run test"' - ) - } else { - t.match(process.env[env], /^(run-script|exec)$/) - } - } - delete process.env[env] -} - -t.afterEach(async (t) => { - for (const env of Object.keys(process.env).filter(e => /^npm_/.test(e))) { - delete process.env[env] - } -}) - t.test('not yet loaded', async t => { const { npm, logs } = await loadMockNpm(t, { load: false }) t.match(npm, { @@ -160,8 +131,8 @@ t.test('npm.load', async t => { prefixDir: { bin: t.fixture('symlink', dirname(process.execPath)), }, - globals: ({ prefix }) => ({ - 'process.env.PATH': resolve(prefix, 'bin'), + globals: (dirs) => ({ + 'process.env.PATH': resolve(dirs.prefix, 'bin'), 'process.argv': [ node, process.argv[1], @@ -299,9 +270,6 @@ t.test('npm.load', async t => { }, }) - // verify that calling the command with a short name still sets - // the npm.command property to the full canonical name of the cmd. - npm.command = null await npm.exec('run', []) t.equal(npm.command, 'run-script', 'npm.command set to canonical name') @@ -357,9 +325,7 @@ t.test('npm.load', async t => { ], }, }) - // verify that calling the command with a short name still sets - // the npm.command property to the full canonical name of the cmd. - npm.command = null + await t.rejects( npm.exec('run', []), /Workspaces not supported for global packages/ @@ -441,9 +407,9 @@ t.test('debug log', async t => { t.test('can load with bad dir', async t => { const { npm, testdir } = await loadMockNpm(t, { load: false, - config: { - 'logs-dir': (c) => join(c.testdir, 'my_logs_dir'), - }, + config: (dirs) => ({ + 'logs-dir': join(dirs.testdir, 'my_logs_dir'), + }), }) const logsDir = join(testdir, 'my_logs_dir') diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 29753c3039f36..7810f126e4d9c 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -1,5 +1,6 @@ const t = require('tap') -const path = require('path') +const { resolve } = require('path') +const fs = require('fs/promises') const { load: _loadMockNpm } = require('../../fixtures/mock-npm.js') const mockGlobals = require('../../fixtures/mock-globals.js') const { cleanCwd, cleanDate } = require('../../fixtures/clean-snapshot.js') @@ -8,6 +9,9 @@ t.formatSnapshot = (p) => { if (Array.isArray(p.files) && !p.files.length) { delete p.files } + if (p?.json === undefined) { + delete p.json + } return p } t.cleanSnapshot = p => cleanDate(cleanCwd(p)) @@ -22,35 +26,26 @@ mockGlobals(t, { }, }) -const loadMockNpm = async (t, { load, command, prefixDir, config } = {}) => { - const { npm, ...rest } = await _loadMockNpm(t, { - load, - prefixDir, - config, +const loadMockNpm = async (t, { errorMocks, ...opts } = {}) => { + const mockError = t.mock('../../../lib/utils/error-message.js', errorMocks) + const res = await _loadMockNpm(t, { + ...opts, mocks: { + ...opts.mocks, '../../package.json': { version: '123.456.789-npm', }, }, }) - if (command !== undefined) { - npm.command = command - } return { - npm, - ...rest, + ...res, + errorMessage: (er) => mockError(er, res.npm), } } -const errorMessage = (er, { mocks, logMocks, npm } = {}) => - t.mock('../../../lib/utils/error-message.js', { ...mocks, ...logMocks })(er, npm) - t.test('just simple messages', async t => { - const npm = await loadMockNpm(t, { + const { errorMessage } = await loadMockNpm(t, { command: 'audit', - config: { - 'node-version': '99.99.99', - }, }) const codes = [ 'ENOAUDIT', @@ -77,8 +72,7 @@ t.test('just simple messages', async t => { 'E403', 'ERR_SOCKET_TIMEOUT', ] - t.plan(codes.length) - codes.forEach(async code => { + for (const code of codes) { const path = '/some/path' const pkgid = 'some@package' const file = '/some/file' @@ -90,12 +84,12 @@ t.test('just simple messages', async t => { file, stack, }) - t.matchSnapshot(errorMessage(er, npm)) - }) + t.matchSnapshot(errorMessage(er)) + } }) t.test('replace message/stack sensistive info', async t => { - const npm = await loadMockNpm(t, { command: 'audit' }) + const { errorMessage } = await loadMockNpm(t, { command: 'audit' }) const path = '/some/path' const pkgid = 'some@package' const file = '/some/file' @@ -108,11 +102,11 @@ t.test('replace message/stack sensistive info', async t => { file, stack, }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) }) t.test('bad engine without config loaded', async t => { - const npm = await loadMockNpm(t, { load: false }) + const { errorMessage } = await loadMockNpm(t, { load: false }) const path = '/some/path' const pkgid = 'some@package' const file = '/some/file' @@ -124,11 +118,11 @@ t.test('bad engine without config loaded', async t => { file, stack, }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) }) t.test('enoent without a file', async t => { - const npm = await loadMockNpm(t) + const { errorMessage } = await loadMockNpm(t) const path = '/some/path' const pkgid = 'some@package' const stack = 'dummy stack trace' @@ -138,11 +132,11 @@ t.test('enoent without a file', async t => { pkgid, stack, }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) }) t.test('enolock without a command', async t => { - const npm = await loadMockNpm(t, { command: null }) + const { errorMessage } = await loadMockNpm(t, { command: null }) const path = '/some/path' const pkgid = 'some@package' const file = '/some/file' @@ -154,41 +148,43 @@ t.test('enolock without a command', async t => { file, stack, }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) }) t.test('default message', async t => { - const npm = await loadMockNpm(t) - t.matchSnapshot(errorMessage(new Error('error object'), npm)) - t.matchSnapshot(errorMessage('error string', npm)) + const { errorMessage } = await loadMockNpm(t) + t.matchSnapshot(errorMessage(new Error('error object'))) + t.matchSnapshot(errorMessage('error string')) t.matchSnapshot(errorMessage(Object.assign(new Error('cmd err'), { cmd: 'some command', signal: 'SIGYOLO', args: ['a', 'r', 'g', 's'], stdout: 'stdout', stderr: 'stderr', - }), npm)) + }))) }) t.test('args are cleaned', async t => { - const npm = await loadMockNpm(t) + const { errorMessage } = await loadMockNpm(t) t.matchSnapshot(errorMessage(Object.assign(new Error('cmd err'), { cmd: 'some command', signal: 'SIGYOLO', args: ['a', 'r', 'g', 's', 'https://evil:password@npmjs.org'], stdout: 'stdout', stderr: 'stderr', - }), npm)) + }))) }) t.test('eacces/eperm', async t => { const runTest = (windows, loaded, cachePath, cacheDest) => async t => { - if (windows) { - mockGlobals(t, { 'process.platform': 'win32' }) - } - const npm = await loadMockNpm(t, { windows, load: loaded }) - const path = `${cachePath ? npm.cache : '/not/cache/dir'}/path` - const dest = `${cacheDest ? npm.cache : '/not/cache/dir'}/dest` + const { errorMessage, logs, cache } = await loadMockNpm(t, { + windows, + load: loaded, + globals: windows ? { 'process.platform': 'win32' } : [], + }) + + const path = `${cachePath ? cache : '/not/cache/dir'}/path` + const dest = `${cacheDest ? cache : '/not/cache/dir'}/dest` const er = Object.assign(new Error('whoopsie'), { code: 'EACCES', path, @@ -196,8 +192,8 @@ t.test('eacces/eperm', async t => { stack: 'dummy stack trace', }) - t.matchSnapshot(errorMessage(er, npm)) - t.matchSnapshot(npm.logs.verbose) + t.matchSnapshot(errorMessage(er)) + t.matchSnapshot(logs.verbose) } for (const windows of [true, false]) { @@ -217,50 +213,14 @@ t.test('json parse', t => { t.test('merge conflict in package.json', async t => { const prefixDir = { - 'package.json': ` -{ - "array": [ -<<<<<<< HEAD - 100, - { - "foo": "baz" - }, -||||||| merged common ancestors - 1, -======= - 111, - 1, - 2, - 3, - { - "foo": "bar" - }, ->>>>>>> a - 1 - ], - "a": { - "b": { -<<<<<<< HEAD - "c": { - "x": "bbbb" - } -||||||| merged common ancestors - "c": { - "x": "aaaa" - } -======= - "c": "xxxx" ->>>>>>> a + 'package.json': await fs.readFile( + resolve(__dirname, '../../fixtures/merge-conflict.json'), 'utf-8'), } - } -} -`, - } - const npm = await loadMockNpm(t, { prefixDir }) + const { errorMessage, npm } = await loadMockNpm(t, { prefixDir }) t.matchSnapshot(errorMessage(Object.assign(new Error('conflicted'), { code: 'EJSONPARSE', - path: path.resolve(npm.prefix, 'package.json'), - }), npm)) + path: resolve(npm.prefix, 'package.json'), + }))) t.end() }) @@ -268,11 +228,11 @@ t.test('json parse', t => { const prefixDir = { 'package.json': 'not even slightly json', } - const npm = await loadMockNpm(t, { prefixDir }) + const { errorMessage, npm } = await loadMockNpm(t, { prefixDir }) t.matchSnapshot(errorMessage(Object.assign(new Error('not json'), { code: 'EJSONPARSE', - path: path.resolve(npm.prefix, 'package.json'), - }), npm)) + path: resolve(npm.prefix, 'package.json'), + }))) t.end() }) @@ -280,11 +240,11 @@ t.test('json parse', t => { const prefixDir = { 'blerg.json': 'not even slightly json', } - const npm = await loadMockNpm(t, { prefixDir }) + const { npm, errorMessage } = await loadMockNpm(t, { prefixDir }) t.matchSnapshot(errorMessage(Object.assign(new Error('not json'), { code: 'EJSONPARSE', - path: path.resolve(npm.prefix, 'blerg.json'), - }), npm)) + path: resolve(npm.prefix, 'blerg.json'), + }))) t.end() }) @@ -292,26 +252,26 @@ t.test('json parse', t => { }) t.test('eotp/e401', async t => { - const npm = await loadMockNpm(t) + const { errorMessage } = await loadMockNpm(t) t.test('401, no auth headers', t => { t.matchSnapshot(errorMessage(Object.assign(new Error('nope'), { code: 'E401', - }), npm)) + }))) t.end() }) t.test('401, no message', t => { t.matchSnapshot(errorMessage({ code: 'E401', - }, npm)) + })) t.end() }) t.test('one-time pass challenge code', t => { t.matchSnapshot(errorMessage(Object.assign(new Error('nope'), { code: 'EOTP', - }), npm)) + }))) t.end() }) @@ -319,7 +279,7 @@ t.test('eotp/e401', async t => { const message = 'one-time pass' t.matchSnapshot(errorMessage(Object.assign(new Error(message), { code: 'E401', - }), npm)) + }))) t.end() }) @@ -339,7 +299,7 @@ t.test('eotp/e401', async t => { }, code: 'E401', }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) } @@ -347,11 +307,11 @@ t.test('eotp/e401', async t => { }) t.test('404', async t => { - const npm = await loadMockNpm(t) + const { errorMessage } = await loadMockNpm(t) t.test('no package id', t => { const er = Object.assign(new Error('404 not found'), { code: 'E404' }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) t.test('you should publish it', t => { @@ -359,7 +319,7 @@ t.test('404', async t => { pkgid: 'yolo', code: 'E404', }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) t.test('name with warning', t => { @@ -367,7 +327,7 @@ t.test('404', async t => { pkgid: new Array(215).fill('x').join(''), code: 'E404', }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) t.test('name with error', t => { @@ -375,7 +335,7 @@ t.test('404', async t => { pkgid: 'node_modules', code: 'E404', }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) t.test('cleans sensitive info from package id', t => { @@ -383,13 +343,13 @@ t.test('404', async t => { pkgid: 'http://evil:password@npmjs.org/not-found', code: 'E404', }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) }) t.test('bad platform', async t => { - const npm = await loadMockNpm(t) + const { errorMessage } = await loadMockNpm(t) t.test('string os/arch', t => { const er = Object.assign(new Error('a bad plat'), { @@ -404,7 +364,7 @@ t.test('bad platform', async t => { }, code: 'EBADPLATFORM', }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) t.test('array os/arch', t => { @@ -420,23 +380,16 @@ t.test('bad platform', async t => { }, code: 'EBADPLATFORM', }) - t.matchSnapshot(errorMessage(er, npm)) + t.matchSnapshot(errorMessage(er)) t.end() }) }) t.test('explain ERESOLVE errors', async t => { - const { npm, ...rest } = await loadMockNpm(t) const EXPLAIN_CALLED = [] - const er = Object.assign(new Error('could not resolve'), { - code: 'ERESOLVE', - }) - - t.matchSnapshot(errorMessage(er, { - npm, - ...rest, - mocks: { + const { errorMessage } = await loadMockNpm(t, { + errorMocks: { '../../../lib/utils/explain-eresolve.js': { report: (...args) => { EXPLAIN_CALLED.push(args) @@ -444,6 +397,12 @@ t.test('explain ERESOLVE errors', async t => { }, }, }, - })) + }) + + const er = Object.assign(new Error('could not resolve'), { + code: 'ERESOLVE', + }) + + t.matchSnapshot(errorMessage(er)) t.match(EXPLAIN_CALLED, [[er, false]]) }) diff --git a/test/lib/utils/exit-handler.js b/test/lib/utils/exit-handler.js index d22ec4dd141a8..ef06ae9641b49 100644 --- a/test/lib/utils/exit-handler.js +++ b/test/lib/utils/exit-handler.js @@ -2,7 +2,7 @@ const t = require('tap') const os = require('os') const fs = require('fs') const fsMiniPass = require('fs-minipass') -const { join } = require('path') +const { join, resolve } = require('path') const EventEmitter = require('events') const { format } = require('../../../lib/utils/log-file') const { load: loadMockNpm } = require('../../fixtures/mock-npm') @@ -61,10 +61,10 @@ const mockExitHandler = async (t, { init, load, testdir, config, mocks, files } }, ...mocks, }, - config: { + config: (dirs) => ({ loglevel: 'notice', - ...config, - }, + ...(typeof config === 'function' ? config(dirs) : config), + }), globals: { 'console.error': (err) => errors.push(err), }, @@ -75,6 +75,13 @@ const mockExitHandler = async (t, { init, load, testdir, config, mocks, files } summary: [['ERR SUMMARY', err.message]], detail: [['ERR DETAIL', err.message]], ...(files ? { files } : {}), + json: { + error: { + code: err.code, + summary: err.message, + detail: err.message, + }, + }, }), os: { type: () => 'Foo', @@ -101,8 +108,8 @@ const mockExitHandler = async (t, { init, load, testdir, config, mocks, files } // to t.plan() every test to make sure we get process.exit called. Also // introduce a small artificial delay so the logs are consistently finished // by the time the exit handler forces process.exit - exitHandler: (...args) => new Promise(resolve => setTimeout(() => { - process.once('exit', resolve) + exitHandler: (...args) => new Promise(res => setTimeout(() => { + process.once('exit', res) exitHandler(...args) }, 50)), } @@ -352,10 +359,10 @@ t.test('timers fail to write', async (t) => { }) const { exitHandler, logs } = await mockExitHandler(t, { - config: { - 'logs-dir': 'LOGS_DIR', + config: (dirs) => ({ + 'logs-dir': resolve(dirs.prefix, 'LOGS_DIR'), timing: true, - }, + }), mocks: { // note, this is relative to test/fixtures/mock-npm.js not this file '../../lib/utils/timers.js': mockTimers, @@ -381,9 +388,9 @@ t.test('log files fail to write', async (t) => { }) const { exitHandler, logs } = await mockExitHandler(t, { - config: { - 'logs-dir': 'LOGS_DIR', - }, + config: (dirs) => ({ + 'logs-dir': resolve(dirs.prefix, 'LOGS_DIR'), + }), mocks: { // note, this is relative to test/fixtures/mock-npm.js not this file '../../lib/utils/log-file.js': mockLogFile, @@ -417,9 +424,9 @@ t.test('files from error message', async (t) => { t.test('files from error message with error', async (t) => { const { exitHandler, logs } = await mockExitHandler(t, { - config: { - 'logs-dir': 'LOGS_DIR', - }, + config: (dirs) => ({ + 'logs-dir': resolve(dirs.prefix, 'LOGS_DIR'), + }), files: [ ['error-file.txt', '# error file content'], ], @@ -587,10 +594,7 @@ t.test('exits uncleanly when only emitting exit event', async (t) => { t.test('do no fancy handling for shellouts', async t => { const { exitHandler, npm, logs } = await mockExitHandler(t) - const exec = await npm.cmd('exec') - - npm.command = 'exec' - npm.commandInstance = exec + await npm.cmd('exec') const loudNoises = () => logs.filter(([level]) => ['warn', 'error'].includes(level)) diff --git a/test/lib/utils/update-notifier.js b/test/lib/utils/update-notifier.js index fa4a04bad9839..41f131cf43040 100644 --- a/test/lib/utils/update-notifier.js +++ b/test/lib/utils/update-notifier.js @@ -17,7 +17,6 @@ let PACOTE_ERROR = null const pacote = { manifest: async (spec, opts) => { if (!spec.match(/^npm@/)) { - console.error(new Error('should only fetch manifest for npm')) process.exit(1) } MANIFEST_REQUEST.push(spec) @@ -53,22 +52,15 @@ const fs = { ...require('fs'), stat: (path, cb) => { if (basename(path) !== '_update-notifier-last-checked') { - console.error( - new Error('should only write to notifier last checked file') - ) process.exit(1) } process.nextTick(() => cb(STAT_ERROR, { mtime: new Date(STAT_MTIME) })) }, writeFile: (path, content, cb) => { if (content !== '') { - console.error(new Error('should not be writing content')) process.exit(1) } if (basename(path) !== '_update-notifier-last-checked') { - console.error( - new Error('should only write to notifier last checked file') - ) process.exit(1) } process.nextTick(() => cb(WRITE_ERROR)) diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js index e1d47ffcd3736..1ddf267839195 100644 --- a/workspaces/config/lib/index.js +++ b/workspaces/config/lib/index.js @@ -17,6 +17,14 @@ const { mkdir, } = require('fs/promises') +const fileExists = (...p) => stat(resolve(...p)) + .then((st) => st.isFile()) + .catch(() => false) + +const dirExists = (...p) => stat(resolve(...p)) + .then((st) => st.isDirectory()) + .catch(() => false) + const hasOwnProperty = (obj, key) => Object.prototype.hasOwnProperty.call(obj, key) @@ -90,6 +98,7 @@ class Config { platform = process.platform, execPath = process.execPath, cwd = process.cwd(), + excludeNpmCwd = false, }) { // turn the definitions into nopt's weirdo syntax this.definitions = definitions @@ -117,10 +126,12 @@ class Config { this.execPath = execPath this.platform = platform this.cwd = cwd + this.excludeNpmCwd = excludeNpmCwd // set when we load configs this.globalPrefix = null this.localPrefix = null + this.localPackage = null // defaults to env.HOME, but will always be *something* this.home = null @@ -311,15 +322,11 @@ class Config { // default the globalconfig file to that location, instead of the default // global prefix. It's weird that `npm get globalconfig --prefix=/foo` // returns `/foo/etc/npmrc`, but better to not change it at this point. - settableGetter(data, 'globalconfig', () => - resolve(this[_get]('prefix'), 'etc/npmrc')) + settableGetter(data, 'globalconfig', () => resolve(this[_get]('prefix'), 'etc/npmrc')) } loadHome () { - if (this.env.HOME) { - return this.home = this.env.HOME - } - this.home = homedir() + this.home = this.env.HOME || homedir() } loadGlobalPrefix () { @@ -330,7 +337,7 @@ class Config { if (this.env.PREFIX) { this.globalPrefix = this.env.PREFIX } else if (this.platform === 'win32') { - // c:\node\node.exe --> prefix=c:\node\ + // c:\node\node.exe --> prefix=c:\node\ this.globalPrefix = dirname(this.execPath) } else { // /usr/local/bin/node --> prefix=/usr/local @@ -599,6 +606,12 @@ class Config { // we return to make sure localPrefix is set await this.loadLocalPrefix() + // if we have not detected a local package json yet, try now that we + // have a local prefix + if (this.localPackage == null) { + this.localPackage = await fileExists(this.localPrefix, 'package.json') + } + if (this[_get]('global') === true || this[_get]('location') === 'global') { this.data.get('project').source = '(global mode enabled, ignored)' this.sources.set(this.data.get('project').source, 'project') @@ -630,16 +643,17 @@ class Config { const isGlobal = this[_get]('global') || this[_get]('location') === 'global' for (const p of walkUp(this.cwd)) { - const hasNodeModules = await stat(resolve(p, 'node_modules')) - .then((st) => st.isDirectory()) - .catch(() => false) + // HACK: this is an option set in tests to stop the local prefix from being set + // on tests that are created inside the npm repo + if (this.excludeNpmCwd && p === this.npmPath) { + break + } - const hasPackageJson = await stat(resolve(p, 'package.json')) - .then((st) => st.isFile()) - .catch(() => false) + const hasPackageJson = await fileExists(p, 'package.json') - if (!this.localPrefix && (hasNodeModules || hasPackageJson)) { + if (!this.localPrefix && (hasPackageJson || await dirExists(p, 'node_modules'))) { this.localPrefix = p + this.localPackage = hasPackageJson // if workspaces are disabled, or we're in global mode, return now if (cliWorkspaces === false || isGlobal) { @@ -663,11 +677,7 @@ class Config { for (const w of workspaces.values()) { if (w === this.localPrefix) { // see if there's a .npmrc file in the workspace, if so log a warning - const hasNpmrc = await stat(resolve(this.localPrefix, '.npmrc')) - .then((st) => st.isFile()) - .catch(() => false) - - if (hasNpmrc) { + if (await fileExists(this.localPrefix, '.npmrc')) { log.warn(`ignoring workspace config at ${this.localPrefix}/.npmrc`) } @@ -675,6 +685,7 @@ class Config { const { data } = this.data.get('default') data.workspace = [this.localPrefix] this.localPrefix = p + this.localPackage = hasPackageJson log.info(`found workspace root at ${this.localPrefix}`) // we found a root, so we return now return diff --git a/workspaces/config/test/index.js b/workspaces/config/test/index.js index 8dbee05880619..d7d55c2366237 100644 --- a/workspaces/config/test/index.js +++ b/workspaces/config/test/index.js @@ -164,7 +164,7 @@ loglevel = yolo npmPath: `${path}/npm`, env: {}, argv: [process.execPath, __filename, '--userconfig', `${path}/npm/npmrc`], - cwd: `${path}/project`, + cwd: join(`${path}/project`), shorthands, definitions, }) @@ -179,7 +179,7 @@ loglevel = yolo npmPath: `${path}/npm`, env: {}, argv: [process.execPath, __filename, '--global'], - cwd: `${path}/project`, + cwd: join(`${path}/project`), shorthands, definitions, }) @@ -195,7 +195,7 @@ loglevel = yolo npmPath: `${path}/npm`, env: {}, argv: [process.execPath, __filename, '--location', 'global'], - cwd: `${path}/project`, + cwd: join(`${path}/project`), shorthands, definitions, }) @@ -238,7 +238,7 @@ loglevel = yolo npmPath: `${path}/npm`, env, argv, - cwd: `${path}/project`, + cwd: join(`${path}/project`), shorthands, definitions, @@ -410,7 +410,7 @@ loglevel = yolo npmPath: `${path}/npm`, env, argv, - cwd: `${path}/project`, + cwd: join(`${path}/project`), shorthands, definitions, @@ -443,7 +443,7 @@ loglevel = yolo npmPath: `${path}/npm`, env, argv: [process.execPath, __filename, '--userconfig', `${path}/project/.npmrc`], - cwd: `${path}/project`, + cwd: join(`${path}/project`), shorthands, definitions, @@ -487,7 +487,7 @@ loglevel = yolo npmPath: path, env, argv, - cwd: `${path}/project-no-config`, + cwd: join(`${path}/project-no-config`), // should prepend DESTDIR to /global DESTDIR: path, @@ -887,7 +887,7 @@ t.test('finding the local prefix', t => { }) t.test('has node_modules', async t => { const c = new Config({ - cwd: `${path}/hasNM/x/y/z`, + cwd: join(`${path}/hasNM/x/y/z`), shorthands, definitions, npmPath: path, @@ -897,7 +897,7 @@ t.test('finding the local prefix', t => { }) t.test('has package.json', async t => { const c = new Config({ - cwd: `${path}/hasPJ/x/y/z`, + cwd: join(`${path}/hasPJ/x/y/z`), shorthands, definitions, npmPath: path, @@ -907,13 +907,13 @@ t.test('finding the local prefix', t => { }) t.test('nada, just use cwd', async t => { const c = new Config({ - cwd: '/this/path/does/not/exist/x/y/z', + cwd: join('/this/path/does/not/exist/x/y/z'), shorthands, definitions, npmPath: path, }) await c.load() - t.equal(c.localPrefix, '/this/path/does/not/exist/x/y/z') + t.equal(c.localPrefix, join('/this/path/does/not/exist/x/y/z')) }) t.end() }) @@ -1131,7 +1131,7 @@ t.test('workspaces', async (t) => { npmPath: cwd, env: {}, argv: [process.execPath, __filename], - cwd: `${path}/workspaces/one`, + cwd: join(`${path}/workspaces/one`), shorthands, definitions, }) @@ -1152,7 +1152,7 @@ t.test('workspaces', async (t) => { npmPath: process.cwd(), env: {}, argv: [process.execPath, __filename, '--workspace', '../two'], - cwd: `${path}/workspaces/one`, + cwd: join(`${path}/workspaces/one`), shorthands, definitions, }) @@ -1173,7 +1173,7 @@ t.test('workspaces', async (t) => { npmPath: process.cwd(), env: {}, argv: [process.execPath, __filename], - cwd: `${path}/workspaces/three`, + cwd: join(`${path}/workspaces/three`), shorthands, definitions, }) @@ -1195,7 +1195,7 @@ t.test('workspaces', async (t) => { npmPath: process.cwd(), env: {}, argv: [process.execPath, __filename, '--prefix', './'], - cwd: `${path}/workspaces/one`, + cwd: join(`${path}/workspaces/one`), shorthands, definitions, }) @@ -1215,7 +1215,7 @@ t.test('workspaces', async (t) => { npmPath: process.cwd(), env: {}, argv: [process.execPath, __filename, '--no-workspaces'], - cwd: `${path}/workspaces/one`, + cwd: join(`${path}/workspaces/one`), shorthands, definitions, }) @@ -1235,7 +1235,7 @@ t.test('workspaces', async (t) => { npmPath: process.cwd(), env: {}, argv: [process.execPath, __filename, '--global'], - cwd: `${path}/workspaces/one`, + cwd: join(`${path}/workspaces/one`), shorthands, definitions, }) @@ -1255,7 +1255,7 @@ t.test('workspaces', async (t) => { npmPath: process.cwd(), env: {}, argv: [process.execPath, __filename, '--location=global'], - cwd: `${path}/workspaces/one`, + cwd: join(`${path}/workspaces/one`), shorthands, definitions, }) @@ -1266,6 +1266,27 @@ t.test('workspaces', async (t) => { t.equal(logs.length, 0, 'got no log messages') }) + t.test('excludeNpmCwd skips auto detect', async (t) => { + const cwd = process.cwd() + t.teardown(() => process.chdir(cwd)) + process.chdir(`${path}/workspaces/one`) + + const config = new Config({ + npmPath: process.cwd(), + env: {}, + argv: [process.execPath, __filename], + cwd: join(`${path}/workspaces/one`), + shorthands, + definitions, + excludeNpmCwd: true, + }) + + await config.load() + t.equal(config.localPrefix, join(path, 'workspaces', 'one'), 'localPrefix is the root') + t.same(config.get('workspace'), [], 'did not set workspace') + t.equal(logs.length, 0, 'got no log messages') + }) + t.test('does not error for invalid package.json', async (t) => { const invalidPkg = join(path, 'workspaces', 'package.json') const cwd = process.cwd() @@ -1281,7 +1302,7 @@ t.test('workspaces', async (t) => { npmPath: cwd, env: {}, argv: [process.execPath, __filename], - cwd: `${path}/workspaces/one`, + cwd: join(`${path}/workspaces/one`), shorthands, definitions, })