From 6f6414829fd82704233fbb56375b167495a0aaf5 Mon Sep 17 00:00:00 2001
From: Luke Karrys <luke@lukekarrys.com>
Date: Fri, 10 May 2024 21:08:46 -0700
Subject: [PATCH] fix: require stdout to be a TTY for progress (#7507)

---
 tap-snapshots/test/lib/docs.js.test.cjs           | 2 +-
 workspaces/config/lib/definitions/definitions.js  | 7 +++++--
 workspaces/config/test/definitions/definitions.js | 1 +
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs
index 85657b9e75dc4..5815c272f2035 100644
--- 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
index 57ab171611838..2978aa4b55dac 100644
--- 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
index 913e14e5c78f2..1b77bd0e811bf 100644
--- 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,
   })