From 078fe543fbaac8a7b29a69a266685cb3253856fe Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Sat, 27 Apr 2024 15:11:18 -0700 Subject: [PATCH] feat: add spinner Closes #7425 --- lib/commands/init.js | 8 +- lib/utils/display.js | 289 ++++++++++++------ lib/utils/open-url-prompt.js | 10 +- lib/utils/read-user-info.js | 6 +- .../@npmcli/run-script/lib/run-script-pkg.js | 6 +- node_modules/@npmcli/run-script/package.json | 4 +- package-lock.json | 5 +- package.json | 2 +- .../lib/utils/open-url-prompt.js.test.cjs | 1 + test/lib/utils/read-user-info.js | 50 +-- workspaces/libnpmexec/lib/index.js | 26 +- 11 files changed, 260 insertions(+), 147 deletions(-) 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..7e192365b8c27 100644 --- a/lib/utils/display.js +++ b/lib/utils/display.js @@ -1,5 +1,5 @@ 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,6 +137,9 @@ 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 () { @@ -146,9 +149,9 @@ class Display { 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 () { @@ -171,6 +174,7 @@ class Display { 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([ @@ -201,18 +205,18 @@ 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 + this.#progress.write(stream, formatWithOptions({ colors, ...options }, ...args)) } // HANDLERS @@ -220,85 +224,115 @@ 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)) + if (args.length) { + this.#writeOutput(output.KEYS.standard, meta, ...args) + } } - 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.pause() + break + + case input.KEYS.end: + log.resume() + output.flush() + this.#progress.resume() + break + + case input.KEYS.read: { + input.start() + const [resolve, reject, promiseFn] = args + return promiseFn() + .then(resolve) + .catch(reject) + .finally(() => { + output.standard('') + input.end() + }) + } } - - 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 +378,107 @@ 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 + #client + #enabled = false + + #frameIndex = 0 + #lastUpdate = 0 + #interval + #initialTimeout + + constructor ({ stream }) { + this.#client = proggy.createClient({ normalize: true }) + this.#stream = stream + } - // PROGRESS + load ({ enabled, unicode }) { + this.#enabled = enabled + this.#spinner = unicode ? Progress.dots : Progress.lines + this.#delayRender(500) + } - #startProgress ({ progress, unicode }) { - if (!progress || this.#silent) { + off () { + this.#clear() + } + + pause () { + this.#clear({ clearLine: true }) + } + + #clear ({ clearLine } = {}) { + if (!this.#enabled) { 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 + clearTimeout(this.#initialTimeout) + this.#initialTimeout = null + clearInterval(this.#interval) + this.#interval = null + this.#frameIndex = 0 + this.#lastUpdate = 0 + this.#stream.cursorTo(0) + if (clearLine) { + this.#stream.clearLine(1) + } + } + + resume () { + this.#delayRender(10) + } + + write (stream, str) { + if (!this.#enabled || !this.#interval) { + return stream.write(str) + } + this.#stream.cursorTo(0) + if (str.startsWith('\n')) { + this.#stream.write(' ') + this.#stream.cursorTo(0) + } + stream.write(str) + this.#render() + } + + #delayRender (ms) { + this.#initialTimeout = setTimeout(() => { + this.#initialTimeout = null + this.#render() + }, ms) + this.#initialTimeout.unref() + } + + #render () { + if (!this.#enabled || this.#initialTimeout) { + return + } + 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.#stream.cursorTo(0) + this.#stream.write(this.#spinner.frames[this.#frameIndex]) } } 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/node_modules/@npmcli/run-script/lib/run-script-pkg.js b/node_modules/@npmcli/run-script/lib/run-script-pkg.js index a4f27b500718c..d2a41702dd282 100644 --- a/node_modules/@npmcli/run-script/lib/run-script-pkg.js +++ b/node_modules/@npmcli/run-script/lib/run-script-pkg.js @@ -4,6 +4,7 @@ const packageEnvs = require('./package-envs.js') const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp') const signalManager = require('./signal-manager.js') const isServerPackage = require('./is-server-package.js') +const { output, input } = require('proc-log') const runScriptPkg = async options => { const { @@ -44,6 +45,7 @@ const runScriptPkg = async options => { return { code: 0, signal: null } } + let inputEnd = () => {} if (stdio === 'inherit') { let banner if (pkg._id) { @@ -56,8 +58,8 @@ const runScriptPkg = async options => { banner += ` ${args.join(' ')}` } banner += '\n' - const { output } = require('proc-log') output.standard(banner) + inputEnd = input.start() } const [spawnShell, spawnArgs, spawnOpts] = makeSpawnArgs({ @@ -104,7 +106,7 @@ const runScriptPkg = async options => { } else { throw er } - }) + }).finally(inputEnd) } module.exports = runScriptPkg diff --git a/node_modules/@npmcli/run-script/package.json b/node_modules/@npmcli/run-script/package.json index dc780ad3ecbec..132584c4d27d6 100644 --- a/node_modules/@npmcli/run-script/package.json +++ b/node_modules/@npmcli/run-script/package.json @@ -16,7 +16,7 @@ }, "devDependencies": { "@npmcli/eslint-config": "^4.0.0", - "@npmcli/template-oss": "4.21.3", + "@npmcli/template-oss": "4.21.4", "spawk": "^1.8.1", "tap": "^16.0.1" }, @@ -42,7 +42,7 @@ }, "templateOSS": { "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.", - "version": "4.21.3", + "version": "4.21.4", "publish": "true" }, "tap": { diff --git a/package-lock.json b/package-lock.json index 1493bbe5ca16f..2681cc88d8732 100644 --- a/package-lock.json +++ b/package-lock.json @@ -95,7 +95,7 @@ "@npmcli/package-json": "^5.1.0", "@npmcli/promise-spawn": "^7.0.1", "@npmcli/redact": "^1.1.0", - "@npmcli/run-script": "^8.0.0", + "@npmcli/run-script": "github:npm/run-script#pull/202/head", "@sigstore/tuf": "^2.3.2", "abbrev": "^2.0.0", "archy": "~1.0.0", @@ -1803,8 +1803,7 @@ }, "node_modules/@npmcli/run-script": { "version": "8.0.0", - "resolved": "https://registry.npmjs.org/@npmcli/run-script/-/run-script-8.0.0.tgz", - "integrity": "sha512-5noc+eCQmX1W9nlFUe65n5MIteikd3vOA2sEPdXtlUv68KWyHNFZnT/LDRXu/E4nZ5yxjciP30pADr/GQ97W1w==", + "resolved": "git+ssh://git@github.com/npm/run-script.git#f8e34252d4056cc19d1fc049613b29731c11741e", "inBundle": true, "dependencies": { "@npmcli/node-gyp": "^3.0.0", diff --git a/package.json b/package.json index 76debeb79dca0..18d559a845e1e 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "@npmcli/package-json": "^5.1.0", "@npmcli/promise-spawn": "^7.0.1", "@npmcli/redact": "^1.1.0", - "@npmcli/run-script": "^8.0.0", + "@npmcli/run-script": "github:npm/run-script#pull/202/head", "@sigstore/tuf": "^2.3.2", "abbrev": "^2.0.0", "archy": "~1.0.0", diff --git a/tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs b/tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs index cf5feed44cc37..a0af353917772 100644 --- a/tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs +++ b/tap-snapshots/test/lib/utils/open-url-prompt.js.test.cjs @@ -8,6 +8,7 @@ exports[`test/lib/utils/open-url-prompt.js TAP does not error when opener can not find command > Outputs extra Browser unavailable message and url 1`] = ` npm home: https://www.npmjs.com + Browser unavailable. Please open the URL manually: https://www.npmjs.com ` diff --git a/test/lib/utils/read-user-info.js b/test/lib/utils/read-user-info.js index 854277783bb6b..91f798d2bef5d 100644 --- a/test/lib/utils/read-user-info.js +++ b/test/lib/utils/read-user-info.js @@ -1,39 +1,41 @@ 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, }, }, - 'npm-user-validate': npmUserValidate, + '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 + }, + }, }) 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') }