fix: require stdout to be a TTY for progress (#7507)

npm · May 11, 2024 · 6f64148 · 6f64148
1 parent 8add914
commit 6f64148
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 3 deletions.
diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs
@@ -1262,7 +1262,7 @@ a semver. Like the \`rc\` in \`1.2.0-rc.8\`.
 * Type: Boolean
 
 When set to \`true\`, npm will display a progress bar during time intensive
-operations, if \`process.stderr\` is a TTY.
+operations, if \`process.stderr\` and \`process.stdout\` are a TTY.
 
 Set to \`false\` to suppress the progress bar.
 

diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js
@@ -1549,13 +1549,16 @@ const definitions = {
     type: Boolean,
     description: `
       When set to \`true\`, npm will display a progress bar during time
-      intensive operations, if \`process.stderr\` is a TTY.
+      intensive operations, if \`process.stderr\` and \`process.stdout\` are a TTY.
 
       Set to \`false\` to suppress the progress bar.
     `,
     flatten (key, obj, flatOptions) {
       flatOptions.progress = !obj.progress ? false
-        : !!process.stderr.isTTY && process.env.TERM !== 'dumb'
+        // progress is only written to stderr but we disable it unless stdout is a tty
+        // also. This prevents the progress from appearing when piping output to another
+        // command which doesn't break anything, but does look very odd to users.
+        : !!process.stderr.isTTY && !!process.stdout.isTTY && process.env.TERM !== 'dumb'
     },
   }),
   provenance: new Definition('provenance', {

diff --git a/workspaces/config/test/definitions/definitions.js b/workspaces/config/test/definitions/definitions.js
@@ -396,6 +396,7 @@ t.test('color', t => {
 t.test('progress', t => {
   const setEnv = ({ tty, term } = {}) => mockGlobals(t, {
     'process.stderr.isTTY': tty,
+    'process.stdout.isTTY': tty,
     'process.env.TERM': term,
   })