rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Add proposal for new CLI command: `yarn upgrade-transitive`

Open magicmark opened this issue 7 years ago • 27 comments

I'm still scoping the possible implementation of this. This is early stage, but I would appreciate any early feedback on this concept!

(https://github.com/yarnpkg/yarn/issues/2543)

magicmark avatar Mar 20 '17 03:03 magicmark

I love the idea to have control over what dependencies Yarn installs transitively. Not sure if upgrade-interactive will be a good place because average projects that I come around have 500+ dependencies, the UI might be unusable.

What do you think of a separate command for this feature? Maybe like this

yarn update-transitive react/babel-runtime/[email protected] 
# updates a specific dependency to a version
or
yarn update-transitive */babel-runtime/[email protected] 
# updates all core-js under babel-runtime to a version
or
yarn update-transitive */react-core 
# updates all core-js to latest version within package.json limits

Then it would allow a fine control if you want to patch (or roll back) a deep dependency or many of them

bestander avatar Mar 23 '17 17:03 bestander

@bestander That sounds exactly like the level of control I keep finding myself wishing for. Would love to see something like that ❤️

dfreeman avatar Mar 23 '17 17:03 dfreeman

Should the * work like globs tho, where ** and * are different?

ljharb avatar Mar 23 '17 17:03 ljharb

Yeah, you are right, Jordan

On 23 March 2017 at 17:40, Jordan Harband [email protected] wrote:

Should the * work like globs tho, where ** and * are different?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/yarnpkg/rfcs/pull/54#issuecomment-288801854, or mute the thread https://github.com/notifications/unsubscribe-auth/ACBdWK1ekrKuQUFS_fBCclO-h_fCSLWBks5roq6PgaJpZM4MiAlG .

bestander avatar Mar 23 '17 17:03 bestander

Separate command sounds entirely reasonable to me.

I'll revise the RFC :)

magicmark avatar Mar 23 '17 18:03 magicmark

What would probably also be useful would be a --transitive flag on the yarn outdated command, to find out which transitive dependencies you'd want to update.

Vinnl avatar May 16 '17 08:05 Vinnl

Nice! I added an issue requesting a similar functionality a few months ago.

A command like yarn update-transitive would do the trick for me as well. Im crossing my fingers for this rfc!

My previous yarn issue: https://github.com/yarnpkg/yarn/issues/2558

presidenten avatar May 31 '17 14:05 presidenten

Ping, does anyone want to send a PR?

bestander avatar Jun 01 '17 11:06 bestander

@bestander I've updated the RFC. Still working on a code implementation (which will also help me answer some of the questions I still have regarding the implementation details in the RFC). Happy if anyone beats me to it, but I am working on it, apologies for the slowness!

magicmark avatar Jun 01 '17 15:06 magicmark

No worries, @magicmark! Thanks for checking in and the work you do

bestander avatar Jun 01 '17 15:06 bestander

Correct me if I wrong, but the issue this RFC aims to solve is similar to Selective versions resolutions. If yes I would go with resolutions mechanism as more flexible and obvious how to use. I urge @magicmark take a look and probably join the forces on landing united RFC which address all related issues.

sejoker avatar Jun 04 '17 13:06 sejoker

@sejoker Thank you for this info. Looks like it makes sense for the other RFC to contain most of the logic for this as it is more generalised.

There could still be value in this RFC just becoming an interface for adding lines to the package.resolutions, field, regenerating the lockfile and updating node_modules?

magicmark avatar Jun 04 '17 15:06 magicmark

@magicmark I'm the author of the #68 RFC, don't hesitate to make comment there to clarify points that could be helpful to make it coherent with your RFC! Also, I saw the bit about vocabulary, if you think using "transitive dependency" is clearer than "nested dependency", please comment about it there, we could improve the language thus :)

victornoel avatar Jun 04 '17 18:06 victornoel

As a non native english speaker I put my vote on nested dependency

presidenten avatar Jun 05 '17 13:06 presidenten

It makes sense to be consistent in implementation and vocabulary between this RFC and #68. Maybe this one could actually piggy-back on #68 implementation?

bestander avatar Jun 15 '17 02:06 bestander

@bestander sounds good to me. I've given my feedback in #68, I'll await @victornoel:

Actually, there was good reason to discuss that here (so no problem with the hijacking ;), because we could consider that doing: yarn upgrade-interactive '**/some-package' would allow to generate valid package.resolutions from the command line! So I will maybe include it in the RFC too, at least for discussing it!

If this is included in #68, we can drop this?

magicmark avatar Jun 15 '17 23:06 magicmark

@magicmark, we might want to discuss more how upgrade-interactive would work modifying the resolutions described in #68.

bestander avatar Jun 15 '17 23:06 bestander

@magicmark sorry for stalling on reviewing this RFC. I have read it through once more after going through #68 and #71.

It seems like your RFC covers and intends to solve another case that neither of those do. Here is an example.

We have a structure of packages

A@^1.1.0 -> B@^2.5.0 -> C@^3.1.0

And let's say they got locked in yarn.lock as

[email protected] -> [email protected] -> [email protected]

After some time all 3 got new versions, e.g. [email protected] -> [email protected] -> [email protected], released and you want to update B and C to latest within their semver ranges. How do you do this?

yarn upgrade A would only bump A to 1.2.1 and then grab [email protected] and [email protected] from yarn.lock if [email protected] did not update its dependency on B.

Removing B and C sections from the lockfile manually seems like the easiest solution to bump them.

Is it worth having a command for this? Some ideas:

  • yarn unlock A@^1.2.0 - would remove A and all subdependencies from yarn.lock and on next install Yarn would fetch latest
  • yarn upgrade A --fresh/transitive - would update A and re-resolve all subdependencies ignoring all existing entries in yarn.lock. That might be tricky because the resolution logic is parallel and recursive

bestander avatar Jun 30 '17 07:06 bestander

As long as yarn.lock continues to open with DO NOT EDIT THIS FILE DIRECTLY (which I assume isn't changing, nor would I want it to), it seems like having a command to manage this would be beneficial.

I think my ideal would be something that captures the behavior you describe for yarn unlock and then immediately does an install — is there a use case I'm not imagining where you'd only want to do the unlock and then stay in a state where you have missing yarn.lock entries?

dfreeman avatar Jul 05 '17 15:07 dfreeman

yarn upgrade A --fresh/transitive - would update A and re-resolve all subdependencies ignoring all existing entries in yarn.lock. That might be tricky because the resolution logic is parallel and recursive

I'm not sure this would be possible, because the lockfile isn't a tree. Let's say two dependencies (A and B) both depend on C@^1.0.0, and are both locked to 1.0.0. Running upgrade A --deep would then proceed to upgrade both A/node_modules/C and B/node_modules/C to 1.0.1, even though we requested to only upgrade the first one 😞

arcanis avatar Jul 17 '17 12:07 arcanis

@arcanis I think that would be an acceptable trade-off. I imagine this command would only be run very rarely, and only when the dev really knows they want to update the deps - it's an advanced level command. So it's ok to make tradeoffs like that.

It'd probably be a good idea to get confirmation first:

$ yarn unlock A@^1.0.2
This will unlock A for the following dependant packages:
└─┬ [email protected]
  └── [email protected]
└─┬ [email protected]
  └─┬ [email protected]
    └── [email protected] 
Are you sure? [y]

jesstelford avatar Aug 30 '17 08:08 jesstelford

I am not sure just having a command-line flag here would be enough. I've been getting a lot of bug reports on one of my libraries where a dependency of the library was, at some point, broken. A fixed version has been released a long time ago, but people are still stuck on the old version because yarn will keep the old version of the transitive dependency in the package lock around... forever, it seems, unless you do something non-obvious like removing the library and re-installing it.

I think even when we have the proposed command-line option (if this rfc picks up stream again at some point) the fact that users have to find out about it and use it to upgrade is a liability. Is there a good reason not to do this by default when running yarn upgrade?

marijnh avatar Feb 23 '18 10:02 marijnh

So quite a lot has changed since the initial draft of this RFC - namely the introduction of the new Selective dependency resolutions API.

I think it makes sense to use this existing API to solve the problems of force upgrading a transitive dep past its usual semver range.

A new CLI command/flag wrapping this might still be a nice DUX thing. Ideally it would also bump an in-range version of a transitive dep without adding an override. This would unify the workflow for both scenarios under one command.

I'll let this marinate for a few days to get some feedback on this, and I'll do some investigation into some implementation details here.

magicmark avatar Mar 19 '18 22:03 magicmark

@bestander Updated the RFC and trimmed it down a bit. I think it's in a good place to be reviewed again.

(tl;dr this is mostly now a dumb wrapper around the package.resolutions field, with some optimizations planned as a follow-up.)

Thanks everyone who has looked over this so far! (And thanks to @kaylieEB @victornoel for the work on the resolutions field that this will use 😃 )

magicmark avatar May 03 '18 23:05 magicmark

I don't quite see how this is the same as the resolutions in the package.json. We find ourselves in this scenario quite often and it is frankly rather annoying:

We have multiple dependencies with the same dependency, for example:

  • my-app:
    • shared-dep@~2.0.0
    • depA:
      • shared-dep@~2.0.0
    • depB:
      • shared-dep@~2.0.0
    • depC:
      • shared-dep@^2.0.3

Now we sometimes end up with two versions of shared-dep (e.g. 2.0.0 and 2.0.3) in our yarn.lock file - and there is no way to get rid of this, as running e.g. yarn upgrade shared-dep will ignore the transitive dependencies. The only way we found to really get rid of this is to run yarn upgrade, but this will upgrade all dependencies - we'd like to sometimes only upgrade certain dependencies.

This feels really weird, since all of these can easily be satisfied by the same version (e.g. 2.0.4), and it does not feel clear to me at all why running yarn upgrade shared-dep would not also try to upgrade transitive dependencies.

IMHO, requiring to manually delete entries from the yarn.lock file is not really a solution for that. Also, I don't see how requiring us to add a manual resolution to the package.json is an adequate solution for that - since we'll need to then update this resolution for each further update of the dependency, which adds a lot of noise.

We would love to have any one of these options:

  • something like yarn unlock to remove an entry from the lockfile
  • Ensure that e.g. yarn upgrade depC would also upgrade this package's dependencies to the allowed ranges (or add a command option to activate that behavior)
  • Add a new yarn upgrade-transitive command to accomodate this

mydea avatar Oct 09 '18 08:10 mydea

Maybe relevant: yarn dedupe is already a command, I'm guessing because of the existence of the npm equivalent. Today when you run yarn dedupe it no-ops and emits a message claiming to be unnecessary, but the problem described here seems like more or less what npm dedupe is meant to solve.

dfreeman avatar Oct 09 '18 20:10 dfreeman

@mydea I'm not sure what you describe is really related to the issue here, but I'm seeing it as well and it is indeed fairly annoying. I've been using yarn-tools (through npx) to fix this "semi-manually"

yched avatar Oct 10 '18 05:10 yched