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

Yarn incorrectly upgrades all when a package is specified. #5043

Closed
mcqj opened this issue Dec 5, 2017 · 6 comments
Closed

Yarn incorrectly upgrades all when a package is specified. #5043

mcqj opened this issue Dec 5, 2017 · 6 comments
Assignees
Labels

Comments

@mcqj
Copy link

mcqj commented Dec 5, 2017

Using 1.3.2

Do you want to request a feature or report a bug?
Report a bug.

What is the current behavior?
Yarn incorrectly upgrades all when a package is specified, depending on CLI options.
yarn upgrade --latest -D
results in

If the current behavior is a bug, please provide the steps to reproduce.

yarn upgrade --latest -D <package-name>

results in all packages in package.json being upgraded to latest.

What is the expected behavior?
Only the specified package should be upgraded.
Interestingly, reversing the options leads to the expected behaviour

yarn upgrade -D --latest <package-name>

works as expected

Please mention your node.js, yarn and operating system version.
macOS High Sierra 10.13.1
node 8.9.1
yarn 1.3.2

While there is a workaround (reversing options), it is a quite pernicious bug because undoing the changes made can require quite a bit of work. While backwards compatibility would suggest that one wouldn't change behaviour, it is probably not a good idea to have 'Upgrade All' as the default behaviour when no package name is specified. It would probably be better to require a specific request for All e.g. a '--all' option.

@ghost ghost assigned rally25rs Dec 5, 2017
@ghost ghost added the triaged label Dec 5, 2017
@rally25rs
Copy link
Contributor

I don't think -D is a valid command line option, and I think commander.js drops parameters that come after one that it doesn't know about.

I'm pretty sure this is a duplicate of #2600

@mcqj
Copy link
Author

mcqj commented Dec 5, 2017

I believe that -D is a synonym for --dev. If it is an invalid option, the correct behaviour from Yarn would be to print an error message and not proceed to carry out an operation. However, when -D is placed first, the command behaves as expected and updates the package to the latest version and updates the package.json file with the new 'DevDependency'.

Be that as it may '--dev' is definitely a valid option as per the documentation and if I substitute '--dev' for '-D' in the bug described above, the behaviour is exactly the same.

So, I don't think its a duplicate of #2600

@rally25rs
Copy link
Contributor

-D/--dev is an option for the add command, but not for upgrade. #2600 is basically that when an unknown option is hit, Yarn should warn or error, not continue on with potentially bad results, which is why I marked it as a duplicate.

@rally25rs
Copy link
Contributor

rally25rs commented Dec 5, 2017

Quick followup; I'm not saying that this isn't a bug. The issue is that commander.js literally doesn't pass the package name to Yarn.

Quick pain NodeJS example:

var program = require('commander');

program
  .version('0.1.0')
  .option('-D', 'Development')
  .parse(process.argv);

console.log(program.args);

If you run node test.js -D foo bar then program.args is ['foo', 'bar'].

Now if you don't have a -D option:

var program = require('commander');

program
  .version('0.1.0')
  .option('-X', 'Unused option')
  .parse(process.argv);

console.log(program.args);

If you run node test.js -D foo bar then program.args is ['bar']. One of the parameters after the unknown flag gets dropped. In your example this is the word after the -D which is your package name.

Yarn is being run with no package name (due to commander.js losing that argument) and so all packages are upgraded.

Fixing issue 2600 would resolve this issue. Sorry if my quickly closing this issue made it come across as dismissive, I certainly didn't mean for it to be.

@mcqj
Copy link
Author

mcqj commented Dec 5, 2017

Ah - gotcha. It didn't come across as dismissive, just thought you misunderstood my report.

What added to my initial assumption was that when I issue the command

yarn upgrade --latest <package-name>

for a package that is only in devDependencies, I get a warning:-

warning <package-name> is already in "devDependencies". Please remove existing entry first before adding it to "dependencies".

This is what led me to believe that I needed to use the '--dev' option on the upgrade command. Should one just ignore that warning?

BTW I can now see why it is essentially a duplicate.

@rally25rs
Copy link
Contributor

for a package that is only in devDependencies, I get a warning:-

Ah! Now that is possibly a bug. I think @kaylieEB might have been working on something similar?

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

No branches or pull requests

2 participants