From 7e349f45363bb8dbe1cc803f8b48befc01aae7fd Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Mon, 29 Apr 2024 16:33:59 -0700 Subject: [PATCH] feat: add spinner (#7432) Closes #7425 This is a little hard to test because everything should continue to work without progress, as evidenced by the lack of churn in the tests and snapshots here. The only existing tests that changed are the addition of newlines after prompts which is new behavior as part of this PR. I resorted to manually running some commands to get an idea of how the various states worked together: **`node . exec -- npm@4`** This should show progress at the start and then hide it as it prompts the user to install `npm@4`. `Ctrl+C` should exit from the prompt and the error should display on the next line (this is a current bug). **`node . audit signatures --loglevel=http`** This should show a lot of http log messages while always keeping the spinner on the last line of output. The spinner also should not jump between frames regardless of how quickly the log messages show up. **`node . pack`** This should immediately show the banners and output from `prepack` and not show the spinner until the actual packing is happening. **`node . login`** The spinner should never while the prompt is being displayed. **`node . view npm`** The spinner should appear while data is being fetched and then disappear for the rest of the command as output is being displayed. The end output on completion should not have any spinner frames rendered on new lines in the output. The old progress bar achieved this by calling `npmlog.disableProgress()` but now it is able to hide the spinner appropriately even while outputting individual lines to stdout. --- DEPENDENCIES.md | 2 - lib/commands/init.js | 8 +- lib/utils/display.js | 304 ++++++++++++------ lib/utils/open-url-prompt.js | 10 +- lib/utils/read-user-info.js | 6 +- package-lock.json | 3 - package.json | 2 - .../test/lib/commands/init.js.test.cjs | 1 + test/fixtures/mock-logs.js | 2 + test/lib/commands/token.js | 10 +- test/lib/utils/display.js | 29 +- test/lib/utils/read-user-info.js | 54 ++-- workspaces/libnpmexec/lib/index.js | 26 +- workspaces/libnpmexec/test/prompt.js | 19 +- 14 files changed, 309 insertions(+), 167 deletions(-) diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index 730d8a35ca730..62bed0488e891 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -132,7 +132,6 @@ graph LR; npm-->pacote; npm-->parse-conflict-json; npm-->proc-log; - npm-->proggy; npm-->read; npm-->semver; npm-->ssri; @@ -527,7 +526,6 @@ graph LR; npm-->pacote; npm-->parse-conflict-json; npm-->proc-log; - npm-->proggy; npm-->qrcode-terminal; npm-->read; npm-->remark-gfm; diff --git a/lib/commands/init.js b/lib/commands/init.js index 1847e19a9560f..205352e86e6ed 100644 --- a/lib/commands/init.js +++ b/lib/commands/init.js @@ -6,7 +6,7 @@ const npa = require('npm-package-arg') const libexec = require('libnpmexec') const mapWorkspaces = require('@npmcli/map-workspaces') const PackageJson = require('@npmcli/package-json') -const { log, output } = require('proc-log') +const { log, output, input } = require('proc-log') const updateWorkspaces = require('../utils/update-workspaces.js') const BaseCommand = require('../base-cmd.js') @@ -148,8 +148,6 @@ class Init extends BaseCommand { } async template (path = process.cwd()) { - log.pause() - const initFile = this.npm.config.get('init-module') if (!this.npm.config.get('yes') && !this.npm.config.get('force')) { output.standard([ @@ -167,7 +165,7 @@ class Init extends BaseCommand { } try { - const data = await initJson(path, initFile, this.npm.config) + const data = await input.read(() => initJson(path, initFile, this.npm.config)) log.silly('package data', data) return data } catch (er) { @@ -176,8 +174,6 @@ class Init extends BaseCommand { } else { throw er } - } finally { - log.resume() } } diff --git a/lib/utils/display.js b/lib/utils/display.js index 299edc797aaf3..4939bc4169bda 100644 --- a/lib/utils/display.js +++ b/lib/utils/display.js @@ -1,5 +1,4 @@ -const proggy = require('proggy') -const { log, output, META } = require('proc-log') +const { log, output, input, META } = require('proc-log') const { explain } = require('./explain-eresolve.js') const { formatWithOptions } = require('./format') @@ -137,18 +136,17 @@ class Display { // Handlers are set immediately so they can buffer all events process.on('log', this.#logHandler) process.on('output', this.#outputHandler) + process.on('input', this.#inputHandler) + this.#progress = new Progress({ stream: stderr }) } off () { process.off('log', this.#logHandler) this.#logState.buffer.length = 0 - process.off('output', this.#outputHandler) this.#outputState.buffer.length = 0 - - if (this.#progress) { - this.#progress.stop() - } + process.off('input', this.#inputHandler) + this.#progress.off() } get chalk () { @@ -170,7 +168,6 @@ class Display { timing, unicode, }) { - this.#command = command // get createSupportsColor from chalk directly if this lands // https://github.com/chalk/chalk/pull/600 const [{ Chalk }, { createSupportsColor }] = await Promise.all([ @@ -181,17 +178,14 @@ class Display { // what it knows about the environment to get color support since we already // determined in our definitions that we want to show colors. const level = Math.max(createSupportsColor(null).level, 1) - this.#noColorChalk = new Chalk({ level: 0 }) - this.#stdoutColor = stdoutColor this.#stdoutChalk = stdoutColor ? new Chalk({ level }) : this.#noColorChalk - this.#stderrColor = stderrColor this.#stderrChalk = stderrColor ? new Chalk({ level }) : this.#noColorChalk - this.#logColors = COLOR_PALETTE({ chalk: this.#stderrChalk }) + this.#command = command this.#levelIndex = LEVEL_OPTIONS[loglevel].index this.#timing = timing this.#json = json @@ -201,18 +195,19 @@ class Display { // Emit resume event on the logs which will flush output log.resume() output.flush() - this.#startProgress({ progress, unicode }) + this.#progress.load({ + unicode, + enabled: !!progress && !this.#silent, + }) } // STREAM WRITES // Write formatted and (non-)colorized output to streams - #stdoutWrite (options, ...args) { - this.#stdout.write(formatWithOptions({ colors: this.#stdoutColor, ...options }, ...args)) - } - - #stderrWrite (options, ...args) { - this.#stderr.write(formatWithOptions({ colors: this.#stderrColor, ...options }, ...args)) + #write (stream, options, ...args) { + const colors = stream === this.#stdout ? this.#stdoutColor : this.#stderrColor + const value = formatWithOptions({ colors, ...options }, ...args) + this.#progress.write(() => stream.write(value)) } // HANDLERS @@ -220,85 +215,112 @@ class Display { // Arrow function assigned to a private class field so it can be passed // directly as a listener and still reference "this" #logHandler = withMeta((level, meta, ...args) => { - if (level === log.KEYS.resume) { - this.#logState.buffering = false - this.#logState.buffer.forEach((item) => this.#tryWriteLog(...item)) - this.#logState.buffer.length = 0 - return - } - - if (level === log.KEYS.pause) { - this.#logState.buffering = true - return - } - - if (this.#logState.buffering) { - this.#logState.buffer.push([level, meta, ...args]) - return + switch (level) { + case log.KEYS.resume: + this.#logState.buffering = false + this.#logState.buffer.forEach((item) => this.#tryWriteLog(...item)) + this.#logState.buffer.length = 0 + break + + case log.KEYS.pause: + this.#logState.buffering = true + break + + default: + if (this.#logState.buffering) { + this.#logState.buffer.push([level, meta, ...args]) + } else { + this.#tryWriteLog(level, meta, ...args) + } + break } - - this.#tryWriteLog(level, meta, ...args) }) // Arrow function assigned to a private class field so it can be passed // directly as a listener and still reference "this" #outputHandler = withMeta((level, meta, ...args) => { - if (level === output.KEYS.flush) { - this.#outputState.buffering = false - - if (meta.jsonError && this.#json) { - const json = {} - for (const item of this.#outputState.buffer) { - // index 2 skips the level and meta - Object.assign(json, tryJsonParse(item[2])) + switch (level) { + case output.KEYS.flush: + this.#outputState.buffering = false + if (meta.jsonError && this.#json) { + const json = {} + for (const item of this.#outputState.buffer) { + // index 2 skips the level and meta + Object.assign(json, tryJsonParse(item[2])) + } + this.#writeOutput( + output.KEYS.standard, + meta, + JSON.stringify({ ...json, error: meta.jsonError }, null, 2) + ) + } else { + this.#outputState.buffer.forEach((item) => this.#writeOutput(...item)) } - this.#writeOutput( - output.KEYS.standard, - meta, - JSON.stringify({ ...json, error: meta.jsonError }, null, 2) - ) - } else { - this.#outputState.buffer.forEach((item) => this.#writeOutput(...item)) - } - - this.#outputState.buffer.length = 0 - return - } - - if (level === output.KEYS.buffer) { - this.#outputState.buffer.push([output.KEYS.standard, meta, ...args]) - return - } - - if (this.#outputState.buffering) { - this.#outputState.buffer.push([level, meta, ...args]) - return + this.#outputState.buffer.length = 0 + break + + case output.KEYS.buffer: + this.#outputState.buffer.push([output.KEYS.standard, meta, ...args]) + break + + default: + if (this.#outputState.buffering) { + this.#outputState.buffer.push([level, meta, ...args]) + } else { + // HACK: if it looks like the banner and we are in a state where we hide the + // banner then dont write any output. This hack can be replaced with proc-log.META + const isBanner = args.length === 1 && + typeof args[0] === 'string' && + args[0].startsWith('\n> ') && + args[0].endsWith('\n') + const hideBanner = this.#silent || ['exec', 'explore'].includes(this.#command) + if (!(isBanner && hideBanner)) { + this.#writeOutput(level, meta, ...args) + } + } + break } + }) - // HACK: if it looks like the banner and we are in a state where we hide the - // banner then dont write any output. This hack can be replaced with proc-log.META - const isBanner = args.length === 1 && - typeof args[0] === 'string' && - args[0].startsWith('\n> ') && - args[0].endsWith('\n') - const hideBanner = this.#silent || ['exec', 'explore'].includes(this.#command) - if (isBanner && hideBanner) { - return + #inputHandler = withMeta((level, meta, ...args) => { + switch (level) { + case input.KEYS.start: + log.pause() + this.#outputState.buffering = true + this.#progress.off() + break + + case input.KEYS.end: + log.resume() + output.flush() + this.#progress.resume() + break + + case input.KEYS.read: { + // The convention when calling input.read is to pass in a single fn that returns + // the promise to await. resolve and reject are provided by proc-log + const [res, rej, p] = args + return input.start(() => p() + .then(res) + .catch(rej) + // Any call to procLog.input.read will render a prompt to the user, so we always + // add a single newline of output to stdout to move the cursor to the next line + .finally(() => output.standard(''))) + } } - - this.#writeOutput(level, meta, ...args) }) // OUTPUT #writeOutput (level, meta, ...args) { - if (level === output.KEYS.standard) { - this.#stdoutWrite({}, ...args) - return - } - - if (level === output.KEYS.error) { - this.#stderrWrite({}, ...args) + switch (level) { + case output.KEYS.standard: + this.#write(this.#stdout, {}, ...args) + break + + case output.KEYS.error: + this.#write(this.#stderr, {}, ...args) + break } } @@ -344,22 +366,118 @@ class Display { this.#logColors[level](level), title ? this.#logColors.title(title) : null, ] - this.#stderrWrite({ prefix }, ...args) - } else if (this.#progress) { - // TODO: make this display a single log line of filtered messages + this.#write(this.#stderr, { prefix }, ...args) + } + } +} + +class Progress { + // Taken from https://github.com/sindresorhus/cli-spinners + // MIT License + // Copyright (c) Sindre Sorhus (https://sindresorhus.com) + static dots = { duration: 80, frames: ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'] } + static lines = { duration: 130, frames: ['-', '\\', '|', '/'] } + + #stream + #spinner + #enabled = false + + #frameIndex = 0 + #lastUpdate = 0 + #interval + #timeout + + // We are rendering is enabled option is set and we are not waiting for the render timeout + get #rendering () { + return this.#enabled && !this.#timeout + } + + // We are spinning if enabled option is set and the render interval has been set + get #spinning () { + return this.#enabled && this.#interval + } + + constructor ({ stream }) { + this.#stream = stream + } + + load ({ enabled, unicode }) { + this.#enabled = enabled + this.#spinner = unicode ? Progress.dots : Progress.lines + // Dont render the spinner for short durations + this.#render(200) + } + + off () { + if (!this.#enabled) { + return + } + clearTimeout(this.#timeout) + this.#timeout = null + clearInterval(this.#interval) + this.#interval = null + this.#frameIndex = 0 + this.#lastUpdate = 0 + this.#clearSpinner() + } + + resume () { + this.#render() + } + + // If we are currenting rendering the spinner we clear it + // before writing our line and then re-render the spinner after. + // If not then all we need to do is write the line + write (write) { + if (this.#spinning) { + this.#clearSpinner() + } + write() + if (this.#spinning) { + this.#render() } } - // PROGRESS + #render (ms) { + if (ms) { + this.#timeout = setTimeout(() => { + this.#timeout = null + this.#renderSpinner() + }, ms) + // Make sure this timeout does not keep the process open + this.#timeout.unref() + } else { + this.#renderSpinner() + } + } - #startProgress ({ progress, unicode }) { - if (!progress || this.#silent) { + #renderSpinner () { + if (!this.#rendering) { return } - this.#progress = proggy.createClient({ normalize: true }) - // TODO: implement proggy trackers in arborist/doctor - // TODO: listen to progress events here and build progress UI - // TODO: see deprecated gauge package for what unicode chars were used + // We always attempt to render immediately but we only request to move to the next + // frame if it has been longer than our spinner frame duration since our last update + this.#renderFrame(Date.now() - this.#lastUpdate >= this.#spinner.duration) + clearInterval(this.#interval) + this.#interval = setInterval(() => this.#renderFrame(true), this.#spinner.duration) + } + + #renderFrame (next) { + if (next) { + this.#lastUpdate = Date.now() + this.#frameIndex++ + if (this.#frameIndex >= this.#spinner.frames.length) { + this.#frameIndex = 0 + } + } + this.#clearSpinner() + this.#stream.write(this.#spinner.frames[this.#frameIndex]) + } + + #clearSpinner () { + // Move to the start of the line and clear the rest of the line + this.#stream.cursorTo(0) + this.#stream.clearLine(1) } } diff --git a/lib/utils/open-url-prompt.js b/lib/utils/open-url-prompt.js index 261cf370da6bd..6f4d453a959d5 100644 --- a/lib/utils/open-url-prompt.js +++ b/lib/utils/open-url-prompt.js @@ -1,5 +1,5 @@ const readline = require('readline') -const { output } = require('proc-log') +const { input, output } = require('proc-log') const open = require('./open-url.js') function print (npm, title, url) { @@ -34,7 +34,7 @@ const promptOpen = async (npm, url, title, prompt, emitter) => { output: process.stdout, }) - const tryOpen = await new Promise(resolve => { + const tryOpen = await input.read(() => new Promise(resolve => { rl.on('SIGINT', () => { rl.close() resolve('SIGINT') @@ -47,14 +47,10 @@ const promptOpen = async (npm, url, title, prompt, emitter) => { if (emitter && emitter.addListener) { emitter.addListener('abort', () => { rl.close() - - // clear the prompt line - output.standard('') - resolve(false) }) } - }) + })) if (tryOpen === 'SIGINT') { throw new Error('canceled') diff --git a/lib/utils/read-user-info.js b/lib/utils/read-user-info.js index b2cd7374c17c3..4e8def4bdf1de 100644 --- a/lib/utils/read-user-info.js +++ b/lib/utils/read-user-info.js @@ -1,6 +1,6 @@ -const { read } = require('read') +const { read: _read } = require('read') const userValidate = require('npm-user-validate') -const { log } = require('proc-log') +const { log, input } = require('proc-log') exports.otp = readOTP exports.password = readPassword @@ -16,6 +16,8 @@ const passwordPrompt = 'npm password: ' const usernamePrompt = 'npm username: ' const emailPrompt = 'email (this IS public): ' +const read = (...args) => input.read(() => _read(...args)) + function readOTP (msg = otpPrompt, otp, isRetry) { if (isRetry && otp && /^[\d ]+$|^[A-Fa-f0-9]{64,64}$/.test(otp)) { return otp.replace(/\s+/g, '') diff --git a/package-lock.json b/package-lock.json index fe21cfb258713..547afaa35bd48 100644 --- a/package-lock.json +++ b/package-lock.json @@ -63,7 +63,6 @@ "pacote", "parse-conflict-json", "proc-log", - "proggy", "qrcode-terminal", "read", "semver", @@ -142,7 +141,6 @@ "pacote": "^18.0.2", "parse-conflict-json": "^3.0.1", "proc-log": "^4.2.0", - "proggy": "^2.0.0", "qrcode-terminal": "^0.12.0", "read": "^3.0.1", "semver": "^7.6.0", @@ -9879,7 +9877,6 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/proggy/-/proggy-2.0.0.tgz", "integrity": "sha512-69agxLtnI8xBs9gUGqEnK26UfiexpHy+KUpBQWabiytQjnn5wFY8rklAi7GRfABIuPNnQ/ik48+LGLkYYJcy4A==", - "inBundle": true, "engines": { "node": "^14.17.0 || ^16.13.0 || >=18.0.0" } diff --git a/package.json b/package.json index 570fad3703d37..4a52b246687f3 100644 --- a/package.json +++ b/package.json @@ -106,7 +106,6 @@ "pacote": "^18.0.2", "parse-conflict-json": "^3.0.1", "proc-log": "^4.2.0", - "proggy": "^2.0.0", "qrcode-terminal": "^0.12.0", "read": "^3.0.1", "semver": "^7.6.0", @@ -177,7 +176,6 @@ "pacote", "parse-conflict-json", "proc-log", - "proggy", "qrcode-terminal", "read", "semver", diff --git a/tap-snapshots/test/lib/commands/init.js.test.cjs b/tap-snapshots/test/lib/commands/init.js.test.cjs index 821193a55e1a9..eae04d77d2e82 100644 --- a/tap-snapshots/test/lib/commands/init.js.test.cjs +++ b/tap-snapshots/test/lib/commands/init.js.test.cjs @@ -20,5 +20,6 @@ Press ^C at any time to quit. exports[`test/lib/commands/init.js TAP workspaces no args -- yes > should print helper info 1`] = ` + added 1 package in {TIME} ` diff --git a/test/fixtures/mock-logs.js b/test/fixtures/mock-logs.js index 346944d7405b0..ce4c189219467 100644 --- a/test/fixtures/mock-logs.js +++ b/test/fixtures/mock-logs.js @@ -41,6 +41,8 @@ module.exports = () => { const streams = { stderr: { + cursorTo: () => {}, + clearLine: () => {}, write: (str) => { str = trimTrailingNewline(str) diff --git a/test/lib/commands/token.js b/test/lib/commands/token.js index 1290a5ee9cb17..f60a938b5b34b 100644 --- a/test/lib/commands/token.js +++ b/test/lib/commands/token.js @@ -265,6 +265,7 @@ t.test('token create', async t => { registry.createToken({ password, cidr }) await npm.exec('token', ['create']) t.strictSame(outputs, [ + '', 'Created publish token n3wt0k3n', 'with IP whitelist: 10.0.0.0/8,192.168.1.0/24', ]) @@ -291,6 +292,7 @@ t.test('token create read only', async t => { registry.createToken({ readonly: true, password }) await npm.exec('token', ['create']) t.strictSame(outputs, [ + '', 'Created read only token n3wt0k3n', ]) }) @@ -347,10 +349,10 @@ t.test('token create parseable output', async t => { }, { replace: true }) registry.createToken({ password, cidr }) await npm.exec('token', ['create']) - t.equal(outputs[0], 'token\tn3wt0k3n') - t.ok(outputs[1].startsWith('created\t')) - t.equal(outputs[2], 'readonly\tfalse') - t.equal(outputs[3], 'cidr_whitelist\t10.0.0.0/8,192.168.1.0/24') + t.equal(outputs[1], 'token\tn3wt0k3n') + t.ok(outputs[2].startsWith('created\t')) + t.equal(outputs[3], 'readonly\tfalse') + t.equal(outputs[4], 'cidr_whitelist\t10.0.0.0/8,192.168.1.0/24') }) t.test('token create ipv6 cidr', async t => { diff --git a/test/lib/utils/display.js b/test/lib/utils/display.js index b05690ebd8179..33f9360e5728c 100644 --- a/test/lib/utils/display.js +++ b/test/lib/utils/display.js @@ -1,11 +1,12 @@ const t = require('tap') +const timers = require('node:timers/promises') const tmock = require('../../fixtures/tmock') const mockLogs = require('../../fixtures/mock-logs') const mockGlobals = require('@npmcli/mock-globals') const { inspect } = require('util') const mockDisplay = async (t, { mocks, load } = {}) => { - const { log, output } = require('proc-log') + const procLog = require('proc-log') const logs = mockLogs() @@ -25,9 +26,8 @@ const mockDisplay = async (t, { mocks, load } = {}) => { t.teardown(() => display.off()) return { + ...procLog, display, - output, - log, displayLoad, ...logs.logs, } @@ -72,16 +72,31 @@ t.test('can buffer output when paused', async t => { }) t.test('can do progress', async (t) => { - const { log, logs } = await mockDisplay(t, { + const { log, logs, outputs, outputErrors, output, input } = await mockDisplay(t, { load: { progress: true, - loglevel: 'error', }, }) - log.silly('', 'this would go to progress') + // wait for initial timer interval to load + await timers.setTimeout(200) + + log.error('', 'before input') + output.standard('before input') + + const end = input.start() + log.error('', 'during input') + output.standard('during input') + end() + + // wait long enough for all spinner frames to render + await timers.setTimeout(800) + log.error('', 'after input') + output.standard('after input') - t.strictSame(logs, [], 'no logs were shown normally') + t.strictSame([...new Set(outputErrors)].sort(), ['-', '/', '\\', '|']) + t.strictSame(logs, ['error before input', 'error during input', 'error after input']) + t.strictSame(outputs, ['before input', 'during input', 'after input']) }) t.test('handles log throwing', async (t) => { diff --git a/test/lib/utils/read-user-info.js b/test/lib/utils/read-user-info.js index 854277783bb6b..35628f7f2faac 100644 --- a/test/lib/utils/read-user-info.js +++ b/test/lib/utils/read-user-info.js @@ -1,39 +1,45 @@ const t = require('tap') +const procLog = require('proc-log') const tmock = require('../../fixtures/tmock') let readOpts = null let readResult = null -const read = { read: async (opts) => { - readOpts = opts - return readResult -} } - -const npmUserValidate = { - username: (username) => { - if (username === 'invalid') { - return new Error('invalid username') - } - - return null - }, - email: (email) => { - if (email.startsWith('invalid')) { - return new Error('invalid email') - } - - return null - }, -} - let logMsg = null + const readUserInfo = tmock(t, '{LIB}/utils/read-user-info.js', { - read, + read: { + read: async (opts) => { + readOpts = opts + return readResult + }, + }, 'proc-log': { + ...procLog, log: { + ...procLog.log, warn: (msg) => logMsg = msg, }, + input: { + ...procLog.input, + read: (fn) => fn(), + }, + }, + 'npm-user-validate': { + username: (username) => { + if (username === 'invalid') { + return new Error('invalid username') + } + + return null + }, + email: (email) => { + if (email.startsWith('invalid')) { + return new Error('invalid email') + } + + return null + }, }, - 'npm-user-validate': npmUserValidate, }) t.beforeEach(() => { diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 944f34b01c237..28cba79a7f227 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/workspaces/libnpmexec/lib/index.js @@ -4,18 +4,16 @@ const { mkdir } = require('fs/promises') const Arborist = require('@npmcli/arborist') const ciInfo = require('ci-info') const crypto = require('crypto') -const { log } = require('proc-log') +const { log, input } = require('proc-log') const npa = require('npm-package-arg') const pacote = require('pacote') const { read } = require('read') const semver = require('semver') - const { fileExists, localFileExists } = require('./file-exists.js') const getBinFromManifest = require('./get-bin-from-manifest.js') const noTTY = require('./no-tty.js') const runScript = require('./run-script.js') const isWindows = require('./is-windows.js') - const { dirname, resolve } = require('path') const binPaths = [] @@ -242,26 +240,24 @@ const exec = async (opts) => { if (add.length) { if (!yes) { - const missingPackages = add.map(a => `${a.replace(/@$/, '')}`) + const addList = add.map(a => `${a.replace(/@$/, '')}`) + // set -n to always say no if (yes === false) { // Error message lists missing package(s) when process is canceled /* eslint-disable-next-line max-len */ - throw new Error(`npx canceled due to missing packages and no YES option: ${JSON.stringify(missingPackages)}`) + throw new Error(`npx canceled due to missing packages and no YES option: ${JSON.stringify(addList)}`) } if (noTTY() || ciInfo.isCI) { - log.warn('exec', `The following package${ - add.length === 1 ? ' was' : 's were' - } not found and will be installed: ${ - add.map((pkg) => pkg.replace(/@$/, '')).join(', ') - }`) + /* eslint-disable-next-line max-len */ + log.warn('exec', `The following package${add.length === 1 ? ' was' : 's were'} not found and will be installed: ${addList.join(', ')}`) } else { - const addList = missingPackages.join('\n') + '\n' - const prompt = `Need to install the following packages:\n${ - addList - }Ok to proceed? ` - const confirm = await read({ prompt, default: 'y' }) + const confirm = await input.read(() => read({ + /* eslint-disable-next-line max-len */ + prompt: `Need to install the following packages:\n${addList.join('\n')}\nOk to proceed? `, + default: 'y', + })) if (confirm.trim().toLowerCase().charAt(0) !== 'y') { throw new Error('canceled') } diff --git a/workspaces/libnpmexec/test/prompt.js b/workspaces/libnpmexec/test/prompt.js index 5edcff9a3d9cb..8c8c520d497b6 100644 --- a/workspaces/libnpmexec/test/prompt.js +++ b/workspaces/libnpmexec/test/prompt.js @@ -1,4 +1,4 @@ -const { log } = require('proc-log') +const procLog = require('proc-log') const { resolve } = require('path') const t = require('tap') const fs = require('fs/promises') @@ -15,6 +15,13 @@ t.test('prompt, accepts', async t => { 'ci-info': { isCI: false }, '../../lib/no-tty.js': () => false, read: { read: async () => 'y' }, + 'proc-log': { + ...procLog, + input: { + ...procLog.input, + read: (fn) => fn(), + }, + }, }, }) @@ -39,6 +46,13 @@ t.test('prompt, refuses', async t => { mocks: { 'ci-info': { isCI: false }, read: { read: async () => 'n' }, + 'proc-log': { + ...procLog, + input: { + ...procLog.input, + read: (fn) => fn(), + }, + }, '../../lib/no-tty.js': () => false, }, }) @@ -146,8 +160,9 @@ t.test('no prompt if CI, multiple packages', async t => { mocks: { 'ci-info': { isCI: true }, 'proc-log': { + ...procLog, log: { - ...log, + ...procLog.log, warn (title, msg) { t.equal(title, 'exec', 'should warn exec title') // this message is nondeterministic as it queries manifests so we just