Use consistent versions on new dependencies

[ForbesLindesay]

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

I feel it's a bug, but it's all a matter of perspective I suppose.

What is the current behavior?

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

1. Publish a package "sub-dependency" version 1.0.0
2. Publish a package "dependency-a" with dependencies: `{"sub-dependency": "^1.0.0"}
3. yarn add dependency-a
4. Publish "sub-dependency" version 1.0.1
5. Publish a package "dependency-b" with dependencies {"sub-dependency": "^1.0.1"}
6. yarn add dependency-b

These steps result in dependency-a depending on sub-dependency 1.0.0 and dependency-b depending on sub-dependency 1.0.1, even though both are capable of picking up sub-dependency 1.0.1. This is a big problem with packages like react, where you need to constantly ensure that there is only one version in your project.

What is the expected behavior?

It should merge the two dependencies, as it would on a fresh install without a lock file. The current workaround is to delete the entire lockfile and re-install, but that also unpins all other versions in the project.

Please mention your node.js, yarn and operating system version.

yarn versions
yarn versions v1.3.2
{ yarn: '1.3.2',
  http_parser: '2.7.0',
  node: '8.10.0',
  v8: '6.2.414.50',
  uv: '1.19.1',
  zlib: '1.2.11',
  ares: '1.10.1-DEV',
  modules: '57',
  nghttp2: '1.25.0',
  openssl: '1.0.2n',
  icu: '60.1',
  unicode: '10.0',
  cldr: '32.0',
  tz: '2017c' }
✨  Done in 0.03s.

OS X

[rally25rs]

This might be a duplicate of #4686

[ForbesLindesay]

I think it is at least closely related, although that pr seems to argue that the new dependency can sometimes use the old version, whereas I'm saying that the new dependency should sometimes force (perhaps with a warning/user prompt) an upgrade of the existing dependency. In my example, using sub-dependency@1.0.0 for both packages would not be a valid resolution.

[rally25rs]

Ah yes, good point. I think this is one of those things where no matter which way you implement it, 50% of the community will want it to work the other way and re-open it as an issue. I'll add the needs-discussion tag to this...

@yarnpkg/core thoughts?

[arcanis]

Relying on dependencies being hoisted a certain way isn't safe. The only per-spec way of ensuring that the exact same version will be used across your app is to use peer dependencies (so for example as a real case, react should be a peer dependencies of applications exposing React plugins).

[Haroenv]

Can't you also use the resolutions field to be sure something is resolved as expected?

[arcanis]

The resolution field is also unsafe in this aspect since hoisting is an optimization, not a guarantee. Yarn would be allowed not to hoist them, leading to duplicate instances in memory.

[ForbesLindesay]

@arcanis There are thousands (if not millions) of packages that rely on some amount of hoisting out in the wild now. I believe that ship has sailed.

@rally25rs I agree that there are two potentially reasonable courses of action here. I would suggest a confirmation prompt as a reasonable compromise (you could also gather metrics about which option people choose). The current approach is getting to be very problematic though.

[bestander]

This question was raised is some form a few times, I agree with the idea that Yarn should give control over resolution tree considering hoisting.

I don't think that @ForbesLindesay's proposal should be the default behavior though.
But maybe a command to "shrink" tree where you can pass strategy (update all subdeps to max version vs find most common version) would be nice.

Resolution logic implementation is a bit hard to read through, it is a recursive call but I would help review a PR :)

[arcanis]

@arcanis There are thousands (if not millions) of packages that rely on some amount of hoisting out in the wild now. I believe that ship has sailed.

This issue is raised every once in a while, but it's never been that big of an issue that we have to act now or everything is broken. This ship will sail if we start making promises, though, and that's something I'd like to avoid.

Another issue is that perfect hoisting is a NP hard problem, so any implementation has to have some boundaries and heuristics to tweak it. Those used by npm and Yarn aren't the same (nor are they the same between versions, for what it's worth), and it is very likely we'll want to change them in the future in impredictable ways.

The distinction between dependencies and peerDependencies is very significant in term of semantic, and I don't believe mixing them will improve the ecosystem.

[arcanis]

That being said, the behavior detailed in the original post (reusing existing resolutions) could be an nice optimization, not arguing against that. Just saying that it should not be needed for your packages to work.

[rally25rs]

One issue I could imagine happening is that if we give people options as to how they want yarn to do resolutions and hoisting, then we're going to get a swarm of issues that are "package X wont install correctly" to which the answer will be "you have to use the --whatever flag for that package".

I'm typically against package managers trying to work magic and fix everyone else's packages and would rather see the burden put on package managers to do things correctly (which is unlikely to happen in all cases, but I can dream...)

---

This may be related to another use case that comes up constantly, and that is people transitioning to yarn from bower (since they kindly redirected people to use yarn). People quickly discover that yarn isn't a 1-to-1 match for the way bower ensured a "flattened" de-duped set of deps and want to know how to "flatten my web dependencies but not my nodejs devDependencies". I think the ability to have flattened dependencies here might fix the original issue because it would ensure there was only a single version of React.

I have long been thinking of adding a feature to pass a manifest name to yarn (instead of always defaulting to package.json) so that you could do something like install package.json devDeps to node_modules/, then install web-package.json with --flat to web_modules/

However that too would cause problems because web packages have install scripts that run in node.js which assumes node_modules directories, so installing most packages to anything other than node_modules will cause errors.

---

Maybe a potential feature would be to add another package.json section akin to resolutions that specifies which packages to install flat? So instead of flattening all packages like --flat does now, we would only do ones like react (or potentially all the deps and not the devDeps) and also it would prevent someone from having to remember to add that flag every time yarn is run. Would something like that fix the original issue here? 🤔

[ForbesLindesay]

One issue I could imagine happening is that if we give people options as to how they want yarn to do resolutions and hoisting, then we're going to get a swarm of issues that are "package X wont install correctly" to which the answer will be "you have to use the --whatever flag for that package".

My suggestion here is not that we make any changes to how hoisting happens in the case where there is no lock file. What I want to change is only what happens to the lock file when you add a dependency.

I think the ability to have flattened dependencies here might fix the original issue

It wouldn't for me. I would not be willing to accept a fully flattened tree if there are genuinely incompatible versions of sub-dependencies. I need the dependencies to merge when the versions are compatible only.

so that you could do something like install package.json devDeps to node_modules/, then install web-package.json with --flat to web_modules/

Yuck, please don't do this. node_modules being a misnomer is definitely not a good enough reason for such major changes. This would break so much stuff. I think it also misses the fact that sharing dependencies & code between node.js and frontend in a single project is a common use case.

Maybe a potential feature would be to add another package.json section akin to resolutions that specifies which packages to install flat?

Maybe. This could apply to react and @types/react and be a reasonable solution. I do think it's more about the frustration of having "add A", "add B" produce different results to having "A and B are in package.json, run install" .

This ship will sail if we start making promises, though, and that's something I'd like to avoid.

The promises made currently are "good enough" that people rely on them. If we could add something to "break" packages that use dependencies when they should use peer dependencies, maybe we could force people to move over.

There is another scenario that is not currently handled well though: Package A depends on Package B and consumers of Package A may or may not use Package B. If the consumers use Package B, they must use the same version. Peer dependencies are not well suited, because it forces everyone to list B as a dependency, even if they only need it because they are using A. Dependencies are wrong because even if the versions are compatible, you could end up with two versions of B.