fix: the spinner #8322

mbtools · 2025-05-24T15:55:45Z

TL;DR

I fixed the spinner, yay! If you like it, buy me a ☕ or 🍕 .

Background

In 10.7.0, a spinner was introduced to the npm cli, a small progress indicator showing that npm is still busy and not dead.

I'm not a JavaScript developer, but I love a good challenge: Debugging and finding a fix for annoying bugs that no one had time to or wanted to fix is as good as sx (well, almost).

Problem

With the spinner came a regression that disrupted the user experience when npm asked for input during login or adding a user: The spinner would show up after the second prompt and drive users nuts.

For example, the username prompt for npm adduser would be ok but subsequent prompts for password and email showed the spinner although users were asked to enter something. Very confusing.

Root Cause

The was a race condition between turning off the spinner and showing a new prompt.

I tried setting timers, waiting for the next tick, and a flag indicating if there's an active prompt. None worked.

Solution

This PR improves the cli prompt experience by ensuring the progress spinner does not appear between consecutive user prompts (such as username and password).

It introduces a prompt counter in display.ts to track active prompts. The spinner now only resumes when all prompts are finished, preventing it from showing up between multi-step user input. This change eliminates distracting spinner flicker during sequential prompts, making CLI interactions smoother and more user-friendly.

Also included:

fixes unnecessary newlines after prompts (only required for silent prompts like password)
adjusts the email prompt from "Email: (this IS public) " to "Email (this will be public):" (no need to shout)

Demo

Here's a demo of what it looks after this PR 😃 (uses Verdaccio in case you wonder)

References

Ref #7432
Closes #7425
Closes #7583

Enjoy, Marc

wraithgar · 2025-05-27T15:54:34Z

lib/utils/display.js

-          .catch(rej)
-          // Any call to procLog.input.read will render a prompt to the user, so we always
-          // add a single newline of output to stdout to move the cursor to the next line
-          .finally(() => output.standard('')))


We still need this for all other input prompts, including the initial password prompt.

do we? for example, with npm init, when I hit enter, it jumps to a newline (win, powershell).

wraithgar · 2025-05-27T16:04:50Z

lib/utils/read-user-info.js

@@ -24,6 +24,9 @@ function readOTP (msg = otpPrompt, otp, isRetry) {

 function readPassword (msg = passwordPrompt, password, isRetry) {
  if (isRetry && password) {
+    // because the prompt is silent and we need to move the cursor
+    // to the next line so the next output is on a new line.
+    process.stdout.write('\n')


We never write to stdout directly in npm. All input and output goes through their respective functions in proc-log which are event-driven and intercepted by npm in its display layer.

There already was an effective newline in lib/utils/display.js that was covering this use case whenever we intercepted an "input" event from proc-log. This is needed because the newline that a user enters is not acted on in and of itself, it's consumed by readline as part of the end of input. If that isn't working I don't understand how this will. You can also see in snapshots where that newline is now missing from all other input prompts.

const read = (...args) => input.read(() => _read(...args))

input.read is what emits input.KEYS.read and is then intercepted by npm, writing that newline via output.standard('').

I think you may have been on the right track with the counter for active inputs. I don't understand right now how we could have two active inputs, it may be a race condition. However ALL of the functionality in there (not just the progress spinner) would need to be in the consideration for pausing until all input prompts are finished.

for me (win), only the silent password prompt consumes the newline at end of input (which is common for making sure the password won't be visible). the other prompts, work well without any of this. i understand that process.stdout isn't proper but didn't know how to do it otherwise (just for the pwd prompt). maybe there's some platform difference here?

i didn't see how two prompts could be active but the counter says otherwise ;-).

i tested with cmd, pwsh, and bash and it works well as is. each input is on it's own line. I don't get the point of the additional newline.

This is a place where we are definitely going to have to lean on the "Chesterton's Fence" principle here and not remove something unless we can explain why it's there. This would require testing in linux, osx and windows in multiple versions of each, and likely a deeper dive into the history of this code to see if it's not a legacy from a previous input method.

Please keep that output line in the display layer.

Going back to the process.stdout itself, that is going to have to be an emitted newline. It should also be something we can see if we can work into the display layer itself. This is not the only place where we will potentially be inputting silently, and if we did that in the future we would almost certainly forget to add this new line. Perhaps we can add a flag on the input meta to let the display layer know it was a silent input?

The finally was added as part of #7432 which introduced the spinner. I think there's a good chance that it was put into the inputHandler because of the side-effect that the spinner shows up on the second and subsequent prompts. Now that this PR makes sure that the spinner no longer gets in the way, the finally is not necessary in general but only for silent prompts. maybe @lukekarrys remembers?

I will work on moving the newline for silent prompts to the display layer.

Thanks for doing that history search.

iirc, the newline was written to move the cursor so no output would be overwritten if the user does a ctrl+c during one of the prompts.

Here's an example from [email protected] and this PR where I ctrl+c during the Username prompt:

╰❯ npm adduser --auth-type=legacy npm notice Log in on https://registry.npmjs.org/ Username: npm error canceled npm error A complete log of this run can be found in: /Users/lukekarrys/.cache/npm/_logs/2025-05-27T22_52_17_565Z-debug-0.log

╰❯ node . adduser --auth-type=legacy npm notice Log in on https://registry.npmjs.org/ npm error canceled npm error A complete log of this run can be found in: /Users/lukekarrys/.cache/npm/_logs/2025-05-27T22_52_23_380Z-debug-0.log

You can see in the second example from this PR that the Username: line get overwritten by the first line of the error. I believe this might be the only reason for the .finally() block. A small thing in comparison to the usability of multi-prompt commands I think, but maybe this context is helpful in deciding if it should stay or go.

THANK YOU. We need to add the ctrl-c consideration to the comments so we know why it's there. I think it needs to stay.

The newline on password prompts can be a separate concern still and mbtools is working on move that into the display layer.

lib/utils/auth.js

mbtools and others added 8 commits May 24, 2025 05:15

fix: prevent spinner from appearing between consecutive prompts

030d216

Update prompt, stop cursor blinking

ef7f0ae

Add exithandler

676a21a

Fix display test

8a44dfa

Add cursor hidden flag and global exithandler

d311426

Move cursor blink logic to separate PR

1110958

Merge branch 'latest' into spinner

9d8c130

Adjust snapshot

2cfa70c

mbtools marked this pull request as ready for review May 25, 2025 18:02

mbtools requested a review from a team as a code owner May 25, 2025 18:02

wraithgar reviewed May 27, 2025

View reviewed changes

lib/utils/auth.js Outdated Show resolved Hide resolved

mbtools and others added 3 commits May 27, 2025 18:24

Revert email prompt change

702b932

Add handling of silent prompts to display; add tests

6a78cfc

Simplify

fd2ab0a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: the spinner #8322

fix: the spinner #8322

mbtools commented May 24, 2025 •

edited

Loading

Uh oh!

wraithgar May 27, 2025

Uh oh!

mbtools May 27, 2025 •

edited

Loading

Uh oh!

wraithgar May 27, 2025

Uh oh!

mbtools May 27, 2025

Uh oh!

mbtools May 27, 2025

Uh oh!

wraithgar May 27, 2025

Uh oh!

mbtools May 27, 2025

Uh oh!

wraithgar May 27, 2025

Uh oh!

lukekarrys May 27, 2025 •

edited

Loading

Uh oh!

wraithgar May 28, 2025

Uh oh!

Uh oh!

Uh oh!

fix: the spinner #8322

Are you sure you want to change the base?

fix: the spinner #8322

Conversation

mbtools commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Problem

Root Cause

Solution

Demo

References

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbtools May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukekarrys May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mbtools commented May 24, 2025 •

edited

Loading

mbtools May 27, 2025 •

edited

Loading

lukekarrys May 27, 2025 •

edited

Loading