Skip to content

Commit b00cedd

Browse files
owlstronautreggi
authored andcommitted
fix: ensure progress bars display consistently across all environments (#8411)
The progress configuration's default value and runtime behavior now use identical logic, preventing cases where progress bars were unexpectedly disabled in cloud IDEs and other environments. Fixes npm/statusboard#996
1 parent 42c19ea commit b00cedd

File tree

3 files changed

+60
-8
lines changed

3 files changed

+60
-8
lines changed

tap-snapshots/test/lib/docs.js.test.cjs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1290,7 +1290,8 @@ a semver. Like the \`rc\` in \`1.2.0-rc.8\`.
12901290
12911291
#### \`progress\`
12921292
1293-
* Default: \`true\` unless running in a known CI system
1293+
* Default: \`true\` when not in CI and both stderr and stdout are TTYs and not
1294+
in a dumb terminal
12941295
* Type: Boolean
12951296
12961297
When set to \`true\`, npm will display a progress bar during time intensive

workspaces/config/lib/definitions/definitions.js

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,9 +1571,9 @@ const definitions = {
15711571
},
15721572
}),
15731573
progress: new Definition('progress', {
1574-
default: !ciInfo.isCI,
1574+
default: !(ciInfo.isCI || !process.stderr.isTTY || !process.stdout.isTTY || process.env.TERM === 'dumb'),
15751575
defaultDescription: `
1576-
\`true\` unless running in a known CI system
1576+
\`true\` when not in CI and both stderr and stdout are TTYs and not in a dumb terminal
15771577
`,
15781578
type: Boolean,
15791579
description: `
@@ -1583,11 +1583,8 @@ const definitions = {
15831583
Set to \`false\` to suppress the progress bar.
15841584
`,
15851585
flatten (key, obj, flatOptions) {
1586-
flatOptions.progress = !obj.progress ? false
1587-
// progress is only written to stderr but we disable it unless stdout is a tty
1588-
// also. This prevents the progress from appearing when piping output to another
1589-
// command which doesn't break anything, but does look very odd to users.
1590-
: !!process.stderr.isTTY && !!process.stdout.isTTY && process.env.TERM !== 'dumb'
1586+
// Only show progress if explicitly enabled AND we have proper TTY environment
1587+
flatOptions.progress = !!obj.progress && !!process.stderr.isTTY && !!process.stdout.isTTY && process.env.TERM !== 'dumb'
15911588
},
15921589
}),
15931590
provenance: new Definition('provenance', {

workspaces/config/test/definitions/definitions.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,7 @@ t.test('progress', t => {
402402

403403
const flat = {}
404404

405+
// Test flatten function behavior
405406
mockDefs().progress.flatten('progress', {}, flat)
406407
t.strictSame(flat, { progress: false })
407408

@@ -417,6 +418,59 @@ t.test('progress', t => {
417418
mockDefs().progress.flatten('progress', { progress: true }, flat)
418419
t.strictSame(flat, { progress: false })
419420

421+
// Ensures consistency between default and flatOptions behavior
422+
t.test('default value consistency', t => {
423+
// Test case 1: Both TTYs, normal terminal, not CI
424+
setEnv({ tty: true, term: 'xterm' })
425+
const def1 = mockDefs({ 'ci-info': { isCI: false, name: null } }).progress
426+
t.equal(def1.default, true, 'default should be true when both TTYs, normal terminal, and not CI')
427+
428+
// Test case 2: No TTYs, not CI
429+
setEnv({ tty: false, term: 'xterm' })
430+
const def2 = mockDefs({ 'ci-info': { isCI: false, name: null } }).progress
431+
t.equal(def2.default, false, 'default should be false when no TTYs')
432+
433+
// Test case 3: Both TTYs but dumb terminal, not CI
434+
setEnv({ tty: true, term: 'dumb' })
435+
const def3 = mockDefs({ 'ci-info': { isCI: false, name: null } }).progress
436+
t.equal(def3.default, false, 'default should be false in dumb terminal')
437+
438+
// Test case 4: Mixed TTY states, not CI
439+
mockGlobals(t, {
440+
'process.stderr.isTTY': true,
441+
'process.stdout.isTTY': false,
442+
'process.env.TERM': 'xterm',
443+
})
444+
const def4 = mockDefs({ 'ci-info': { isCI: false, name: null } }).progress
445+
t.equal(def4.default, false, 'default should be false when only one TTY')
446+
447+
// Test case 5: Good TTY environment but in CI
448+
setEnv({ tty: true, term: 'xterm' })
449+
const def5 = mockDefs({ 'ci-info': { isCI: true, name: 'github-actions' } }).progress
450+
t.equal(def5.default, false, 'default should be false in CI even with good TTY environment')
451+
452+
t.end()
453+
})
454+
455+
// Test that flatten behavior is independent of CI detection
456+
t.test('flatten function ignores CI detection', t => {
457+
const flatObj = {}
458+
459+
// Test that CI doesn't affect flatten behavior when user explicitly enables
460+
setEnv({ tty: true, term: 'xterm' })
461+
const defsCI = mockDefs({ 'ci-info': { isCI: true, name: 'github-actions' } })
462+
defsCI.progress.flatten('progress', { progress: true }, flatObj)
463+
t.equal(flatObj.progress, true, 'flatten should enable progress in CI if user explicitly sets true and TTY is available')
464+
465+
// Test that non-CI doesn't guarantee flatten success if TTY is bad
466+
setEnv({ tty: false, term: 'xterm' })
467+
const defsNoCI = mockDefs({ 'ci-info': { isCI: false, name: null } })
468+
defsNoCI.progress.flatten('progress', { progress: true }, flatObj)
469+
t.equal(flatObj.progress, false, 'flatten should disable progress outside CI if TTY is not available')
470+
471+
t.end()
472+
})
473+
420474
t.end()
421475
})
422476

0 commit comments

Comments
 (0)