Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: enable interactivity by default when possible #5562

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paul-soporan
Copy link
Member

@paul-soporan paul-soporan commented Jul 3, 2023

What's the problem this PR addresses?

Interactivity can lead to a more user-friendly experience and we have quite a lot of interactive commands, but some of them (add and up) only enable it when explicitly requested via --interactive or when preferInteractive is set to true, which most users forget to do since it isn't the default.

In addition, when preferInteractive is set to true, these commands are interactive even outside TTYs, which is unintended (can be checked by running yarn add lodash in our repository).
Extracted to #6419.

How did you fix it?

Made interactivity the default, by defaulting preferInteractive to true, except on CI.

Also made it so that Yarn is never interactive outside of TTY environments since that can never work.

Now there are a few questions remaining:

  • should Yarn be interactive on CI when --interactive is passed? Is there a use case for it? Or should the CI check override everything else just like the TTY one
  • how can we make the OTP prompt in npmHttpUtils respect all of this? Do we forward stdout as a parameter everywhere or is there a better option? 🤔

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

Copy link
Member

@arcanis arcanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced about this one...

On one hand I agree the interactive mode is really nice and not enough people are aware of it, and since additional exposure would make sense.

On the other hand, those commands are currently "fire and forget": I frequently run them in a terminal, and before the command even starts I jump into another tab to do something else. With interactive mode the default, I'd go back and the command wouldn't have done anything. Frustrating.

There's also the problem of scripts, and generally all things expecting the command to run unimpaired (for example it's easy to imagine a create-react-app-like tool out there calling yarn add with stdio: 'inherit'.

And finally, there's the concern of making the command harder to use. In general we try to make the commands do the "right thing" by default, so that users don't have to remember to use specific flags except in rare occasions. Enabling the interactive mode kinda goes against that, as we now leave it up to the user to decide what they want to do. In essence, we're forcing them to use options, albeit these ones are selected with keys rather than the CLI.

Rather than making it the default, I'd first start by adding a note to the tips about it, and perhaps adding a line to yarn add when multiple suggestions existed, and letting the user know they can run the command again with -i to know more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants