jquery-ujs package installs it's own nested version of jquery when dependency should be satisfied

[kernow]

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

Possible bug

What is the current behavior?
When installing the jquery-ujs package a nested version of jquery is installed which is different to the version specified in package.json

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

yarn init
yarn add jquery@3.2.1
yarn add jquery-ujs

package.json contents:

# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


jquery-ujs@^1.2.2:
  version "1.2.2"
  resolved "https://registry.yarnpkg.com/jquery-ujs/-/jquery-ujs-1.2.2.tgz#6a8ef1020e6b6dda385b90a4bddc128c21c56397"
  dependencies:
    jquery ">=1.8.0"

jquery@3.2.1:
  version "3.2.1"
  resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.2.1.tgz#5c4d9de652af6cd0a770154a631bba12b015c787"

jquery@>=1.8.0:
  version "3.3.1"
  resolved "https://registry.yarnpkg.com/jquery/-/jquery-3.3.1.tgz#958ce29e81c9790f31be7792df5d4d95fc57fbca"

What is the expected behavior?
The dependency of jquery-ujs is jquery >=1.8.0 so it should be satisfied by the jquery 3.2.1 already added

Please mention your node.js, yarn and operating system version.
yarn 1.5.1
node 9.9.0
osx 10.13.3

[rally25rs]

confirmed on v1.5.1, but I suspect this issue has been in yarn for a long time.

---

Seems to be somehow connected to: https://github.com/yarnpkg/yarn/blob/master/src/package-request.js#L207

const solvedRange = semver.validRange(range) ? info.version : range;

The jquery-ujs package has a dependency on jquery@>=1.8.0.
range is >=1.8.0
info.version is 3.3.1 (I think this is resolved from latest?)
solvedRange is 3.3.1

I don't really understand the above logic. if range is a valid semver then don't use it?

solvedRange ("3.3.1") gets passed to:

this.resolver.getHighestRangeVersionMatch(name, solvedRange, info);
// actual passed params are ('jquery', '3.3.1', info)
// but I believe they should be ('jquery', '>=1.8.0', info)

so it finds the highest already-installed version that matches exactly 3.3.1. Which is why the second version of jquery is installed.

---

@arcanis a git blame on that solvedRange line points to an old PR of yours #3729
Any thoughts on this?

[rally25rs]

Oh, looks like I've been down this road before 😆duplicate #4686
(@arcanis it looks like this open issue was already in-process by you, so we can move discussion there instead of in this issue.)

[arcanis]

Yep I remember the hoisting became less efficient at some point, it might very well be related.

That being said, another issue at play here is that jquery-ujs lists a dependency on jQuery when it should instead list a peer dependency. Dependencies provide no guarantee regarding optimizations (and using the same package than the top-level if possible is an optimization, albeit a simple one). Peer dependencies provide the guarantee that the instance they will get from a require call will always be the same one as the parent package gets.

I don't really understand the above logic. if range is a valid semver then don't use it?

You're right, that doesn't make a lot of sense :/ I think what I wanted to do was "if this is not a version but is a range, then use the one we already have if possible" (since the goal of the PR was to not do optimization when reading from the lockfile). As such the condition should probably be:

const solvedRange = semver.valid(range) && !semver.validRange(range) ? range : info.version;