From 58fd1cb4bb2bd09cde8e286aef2e764fefec8b9d Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Thu, 25 Apr 2024 14:21:22 -0700 Subject: [PATCH] fixup! fix: hide banner for exec and explore --- lib/npm.js | 60 ++++++++++++++++++++++++++------------------ lib/utils/display.js | 17 +++++++------ test/lib/docs.js | 2 +- 3 files changed, 47 insertions(+), 32 deletions(-) diff --git a/lib/npm.js b/lib/npm.js index 2d8c4794e1195..d7561e0cd03ac 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -32,6 +32,7 @@ class Npm { updateNotification = null argv = [] + #commandName = null #command = null #runId = new Date().toISOString().replace(/[.:]/g, '_') #title = 'npm' @@ -94,11 +95,11 @@ class Npm { async load () { return time.start('npm:load', async () => { - const { exec } = await this.#load() + const { exec, command, args } = await this.#load() return { exec, - command: this.argv.shift(), - args: this.argv, + command, + args, } }) } @@ -165,7 +166,26 @@ class Npm { await time.start('npm:load:configload', () => this.config.load()) + // npm --versions + if (this.config.get('versions', 'cli')) { + this.argv = ['version'] + this.config.set('usage', false, 'cli') + } else { + this.argv = [...this.config.parsedArgv.remain] + } + + // Remove first argv since that is our command as typed + // Note that this might not be the actual name of the command + // due to aliases, etc. But we use the raw form of it later + // in user output so it must be preserved as is. + const commandArg = this.argv.shift() + + // This is the actual name of the command that will be run or + // undefined if deref could not find a match + const command = deref(commandArg) + await this.#display.load({ + command, loglevel: this.config.get('loglevel'), stdoutColor: this.color, stderrColor: this.logColor, @@ -174,9 +194,6 @@ class Npm { progress: this.flatOptions.progress, json: this.config.get('json'), heading: this.config.get('heading'), - // TODO: avoid passing all of npm in. this is a quick - // fix to allow display to know the currently running command - npm: this, }) process.env.COLOR = this.color ? '1' : '0' @@ -205,23 +222,24 @@ class Npm { // note: this MUST be shorter than the actual argv length, because it // uses the same memory, so node will truncate it if it's too long. + // We time this because setting process.title is slow sometimes but we + // have to do it for security reasons. But still helpful to know how slow it is. time.start('npm:load:setTitle', () => { - const { parsedArgv: { cooked, remain } } = this.config - this.argv = remain // Secrets are mostly in configs, so title is set using only the positional args // to keep those from being leaked. We still do a best effort replaceInfo. - this.#title = ['npm'].concat(replaceInfo(remain)).join(' ').trim() + this.#title = ['npm'].concat(replaceInfo(this.config.parsedArgv.remain)).join(' ').trim() process.title = this.#title - // The cooked argv is also logged separately for debugging purposes. It is - // cleaned as a best effort by replacing known secrets like basic auth - // password and strings that look like npm tokens. XXX: for this to be - // safer the config should create a sanitized version of the argv as it - // has the full context of what each option contains. - this.#argvClean = replaceInfo(cooked) - log.verbose('title', this.title) - log.verbose('argv', this.#argvClean.map(JSON.stringify).join(' ')) }) + // The cooked argv is also logged separately for debugging purposes. It is + // cleaned as a best effort by replacing known secrets like basic auth + // password and strings that look like npm tokens. XXX: for this to be + // safer the config should create a sanitized version of the argv as it + // has the full context of what each option contains. + this.#argvClean = replaceInfo(this.config.parsedArgv.cooked) + log.verbose('title', this.title) + log.verbose('argv', this.#argvClean.map(JSON.stringify).join(' ')) + // logFile.load returns a promise that resolves when old logs are done being cleaned. // We save this promise to an array so that we can await it in tests to ensure more // deterministic logging behavior. The process will also hang open if this were to @@ -247,13 +265,7 @@ class Npm { log.warn('using --force', 'Recommended protections disabled.') } - // npm --versions - if (this.config.get('versions', 'cli')) { - this.argv = ['version'] - this.config.set('usage', false, 'cli') - } - - return { exec: true } + return { exec: true, command: commandArg, args: this.argv } } get isShellout () { diff --git a/lib/utils/display.js b/lib/utils/display.js index 24aa38c18b271..299edc797aaf3 100644 --- a/lib/utils/display.js +++ b/lib/utils/display.js @@ -119,12 +119,12 @@ class Display { #progress // options + #command #levelIndex #timing #json #heading #silent - #npm // display streams #stdout @@ -160,6 +160,7 @@ class Display { } async load ({ + command, heading, json, loglevel, @@ -168,9 +169,8 @@ class Display { stdoutColor, timing, unicode, - npm, }) { - this.#npm = npm + this.#command = command // get createSupportsColor from chalk directly if this lands // https://github.com/chalk/chalk/pull/600 const [{ Chalk }, { createSupportsColor }] = await Promise.all([ @@ -275,10 +275,13 @@ class Display { return } - // HACK: if it looks like the banner and we are silent do not print it. - // There's no other way to do this right now :(\ - const isBanner = args.length === 1 && args[0].startsWith('\n> ') && args[0].endsWith('\n') - const hideBanner = this.#silent || ['exec', 'explore'].includes(this.#npm.command) + // 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 } diff --git a/test/lib/docs.js b/test/lib/docs.js index 7ace9b9995701..e074d556bb360 100644 --- a/test/lib/docs.js +++ b/test/lib/docs.js @@ -74,7 +74,7 @@ t.test('basic usage', async t => { // are generated in the following test const { npm } = await loadMockNpm(t, { mocks: { - '{LIB}/utils/cmd-list.js': { commands: [] }, + '{LIB}/utils/cmd-list.js': { ...cmdList, commands: [] }, }, config: { userconfig: '/some/config/file/.npmrc' }, globals: { process: { platform: 'posix' } },