Upgrade transient deps during upgrades. #4636

rally25rs · 2017-10-05T02:30:47Z

Summary

Due to the changes made to upgrade for v1, we inadvertently stopped upgrading transient dependencies.

This happend for 2 reasons:

because the existing lockfile is left in-place, so transient deps were already at a locked version
only direct dependencies that were actually out of date were upgraded (for performance reasons)

This change will now clear dependencies to be upgraded, recursively, from the lockfile. This lets transient deps get upgrade as well. In the case that all packages are being upgraded, the lockfile is just removed and rebuilt.

I also found a bug in upgrade in yarn v1.0 and fixed that while in here...

in Yarn v1.0 if you yarn upgrade some-package then it will possibly upgrade some-package if there is a new version, but the transient deps underneath some-package would not be upgraded (because of the entries in the lockfile, mentioned above). With this update those transient deps will be upgraded.

Test plan

Added 2 additional unit tests that check transient deps.

buildsize · 2017-10-05T02:47:44Z

This change will increase the build size from 9.92 MB to 9.93 MB, an increase of 8.17 KB (0%)

File name	Previous Size	New Size	Change
yarn-[version].noarch.rpm	858.23 KB	858.9 KB	678 bytes (0%)
yarn-[version].js	3.78 MB	3.78 MB	3.14 KB (0%)
yarn-legacy-[version].js	3.83 MB	3.83 MB	3.24 KB (0%)
yarn-v[version].tar.gz	863.97 KB	864.63 KB	674 bytes (0%)
yarn_[version]all.deb	653.25 KB	653.72 KB	478 bytes (0%)

bdwain · 2017-10-05T05:43:41Z

__tests__/commands/upgrade.js

@@ -31,6 +31,11 @@ const expectInstalledDevDependency = async (config, name, range, expectedVersion
  await _expectDependency('devDependencies', config, name, range, expectedVersion);
 };

+const expectInstalledTransientDependency = async (config, name, range, expectedVersion) => {


i think transient is not the right word here. It should be transitive. same goes for other usages of transient in the PR

whoopsies 😆 thanks for noticing that.

bdwain · 2017-10-05T05:49:54Z

I'm running this locally and it seems to ignore my .yarnrc file in my project. it Points to an internal registry, but yarn upgrade --verbose logs calls to registry.yarnpkg.com. I'm not sure if it's related to running locally or not, but yarn install did not have the same issue.

overall though it looks like it works. Thanks!

rally25rs · 2017-10-05T12:05:30Z

@bdwain is this .yarnrc registry thing a new error in this PR, or did it not work in v1.0 either? I'll try to set something up here to reproduce the error...

rally25rs · 2017-10-05T12:38:12Z

Hmm, OK so yeah I see the same error as @bdwain if I have a registry "http://localhost:4873/" set in my .yarnrc

With Yarn v1.0:

$ yarn upgrade --verbose
...
verbose 0.401 Performing "GET" request to "http://localhost:4873/strip-ansi".
verbose 0.402 Performing "GET" request to "http://localhost:4873/array-union".
verbose 0.485 Request "http://localhost:4873/array-union" finished with status code 200.
verbose 0.49 Request "http://localhost:4873/strip-ansi" finished with status code 200.
[1/4] 🔍  Resolving packages...
...

With this PR:

$ yarn upgrade --verbose
...
verbose 0.752 Performing "GET" request to "https://registry.yarnpkg.com/strip-ansi".
verbose 0.753 Performing "GET" request to "https://registry.yarnpkg.com/array-union".
verbose 0.878 Request "https://registry.yarnpkg.com/array-union" finished with status code 200.
verbose 0.88 Request "https://registry.yarnpkg.com/strip-ansi" finished with status code 200.
...

that's very weird. I have no clue what I would have changed that would have affected that. Well, I'll start digging in...

edit:

This is broken in master, not by my PR changes. I merged master into this branch before PR'ing it to resolve some conflicts. I'll raise the concern in the development Slack channel ☎️

edit2:

Tracked the registry error that @bdwain reported to #4238

arcanis · 2017-10-05T15:05:18Z

This change will now clear dependencies to be upgraded, recursively, from the lockfile.

I'm not convinced this is the right behavior. Let's say with have:

[email protected], depends on B@~1.0.0
[email protected], depends on B@~1.0.0
[email protected]
[email protected]
Root depends on A@~1.0.0 and B@~1.0.0
Lockfile is [email protected] and [email protected]

If I run yarn update A, I don't expect B to also be updated to 1.0.1.

rally25rs · 2017-10-05T15:16:18Z

@arcanis hmm, interesting case. Maybe direct dependencies get excluded from that recursion?

rally25rs · 2017-10-08T18:44:31Z

@arcanis I added a test for the case you mentioned. (also fixed merge conflicts and added some verbose logging to the upgrade process)

arcanis · 2017-10-10T15:59:55Z

Thanks @rally25rs! I just merged #4654 which should ship with fixes to a few commands that include upgrade and upgrade-interactive, do you mind rebasing and checking if it makes your tests pass as well?

voxpelli · 2017-10-10T16:43:05Z

Tried this PR on a larger project of mine and it correctly produced the same yarn.lock that was produced by removing the yarn.lock and starting over from scratch. 👌 Thanks!

rally25rs · 2017-10-10T17:37:25Z

@voxpelli Thank you for taking the time to try this out!

@arcanis Merged master and resolved conflicts.

arcanis · 2017-10-11T16:26:07Z

Thank you both!

bdwain · 2017-10-13T03:21:24Z

thanks again!

* [yarnpkg#4476] Upgrade transient deps during upgrades. * Rename 'transient' to 'transitive' * dont upgrade direct deps unless requested, add verbose upgrade logging * upgrade-interactive reuse lockfile cleaning from upgrade.js

vkrol · 2017-11-05T10:37:02Z

@rally25rs if I run the upgrade command with the latest flag (for example yarn upgrade package-name --latest) then the transitive dependencies won’t be upgraded. I think that it is the wrong behavior. What do you think about it?

rally25rs added 2 commits October 4, 2017 22:03

[yarnpkg#4476] Upgrade transient deps during upgrades.

fa45786

merged master to branch upgrade-all-4476

ea32f70

rally25rs mentioned this pull request Oct 5, 2017

Yarn Upgrade not changing yarn.lock since v1.0 #4476

Closed

bdwain reviewed Oct 5, 2017

View reviewed changes

Rename 'transient' to 'transitive'

9eb036f

arcanis self-assigned this Oct 5, 2017

rally25rs added 2 commits October 7, 2017 23:53

merged master to branch upgrade-all-4476

b6569fd

dont upgrade direct deps unless requested, add verbose upgrade logging

f5b0092

upgrade-interactive reuse lockfile cleaning from upgrade.js

5062fcb

voxpelli mentioned this pull request Oct 10, 2017

Yarn upgrade creates duplicate dependency resolution #3967

Closed

merge master to branch upgrade-all-4476

9e49ce7

arcanis approved these changes Oct 11, 2017

View reviewed changes

arcanis merged commit 5e564c6 into yarnpkg:master Oct 11, 2017

renovate bot mentioned this pull request Feb 6, 2018

chore(deps): update jest monorepo packages to ^22.0.0 #5190

Closed

bestander mentioned this pull request Feb 11, 2018

yarn upgrade does not respect scope flag correctly #5350

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Upgrade transient deps during upgrades. #4636

Upgrade transient deps during upgrades. #4636

Uh oh!

rally25rs commented Oct 5, 2017

Uh oh!

buildsize bot commented Oct 5, 2017 •

edited

Loading

Uh oh!

bdwain Oct 5, 2017 •

edited

Loading

Uh oh!

rally25rs Oct 5, 2017

Uh oh!

bdwain commented Oct 5, 2017 •

edited

Loading

Uh oh!

rally25rs commented Oct 5, 2017

Uh oh!

rally25rs commented Oct 5, 2017 •

edited

Loading

Uh oh!

arcanis commented Oct 5, 2017

Uh oh!

rally25rs commented Oct 5, 2017

Uh oh!

rally25rs commented Oct 8, 2017

Uh oh!

arcanis commented Oct 10, 2017

Uh oh!

voxpelli commented Oct 10, 2017

Uh oh!

rally25rs commented Oct 10, 2017

Uh oh!

arcanis commented Oct 11, 2017

Uh oh!

bdwain commented Oct 13, 2017

Uh oh!

vkrol commented Nov 5, 2017

Uh oh!

Uh oh!

Upgrade transient deps during upgrades. #4636

Upgrade transient deps during upgrades. #4636

Uh oh!

Conversation

rally25rs commented Oct 5, 2017

Uh oh!

buildsize bot commented Oct 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdwain Oct 5, 2017 • 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!

rally25rs Oct 5, 2017

Choose a reason for hiding this comment

Uh oh!

bdwain commented Oct 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rally25rs commented Oct 5, 2017

Uh oh!

rally25rs commented Oct 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arcanis commented Oct 5, 2017

Uh oh!

rally25rs commented Oct 5, 2017

Uh oh!

rally25rs commented Oct 8, 2017

Uh oh!

arcanis commented Oct 10, 2017

Uh oh!

voxpelli commented Oct 10, 2017

Uh oh!

rally25rs commented Oct 10, 2017

Uh oh!

arcanis commented Oct 11, 2017

Uh oh!

bdwain commented Oct 13, 2017

Uh oh!

vkrol commented Nov 5, 2017

Uh oh!

Uh oh!

buildsize bot commented Oct 5, 2017 •

edited

Loading

bdwain Oct 5, 2017 •

edited

Loading

bdwain commented Oct 5, 2017 •

edited

Loading

rally25rs commented Oct 5, 2017 •

edited

Loading