Yarn: "bin" symlinks not created for sub-dependencies

[rally25rs]

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

What is the current behavior?
Yarn does not create node_modules/.bin symlinks for nested dependencies that have a bin specified in their package.json

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

1. Add grunt-eslint to your package.json
2. yarn install

"grunt-eslint" depends on "eslint", which has this line in its package.json:

  "bin": {
    "eslint": "./bin/eslint.js"
  },

After install, no symlink is made in node_modules/.bin

What is the expected behavior?

If you npm install instead, the symlink is made.

$ ls -l node_modules/.bin/eslint

lrwxr-xr-x  1 user  computer\Domain Users  23 Mar  8 09:48 node_modules/.bin/eslint -> ../eslint/bin/eslint.js

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

yarn v0.21.3
node v6.3
OSX

[Daniel15]

If you want to use the eslint command, shouldn't your project have a direct dependency on eslint? Relying on transitive dependencies can be risky - For example, what if grunt-eslint were to remove its dependency on eslint? I know that's not likely in this case, but it could be more reasonable in other cases.

[rally25rs]

I actually only noticed this bug because SublimeLinter-eslint plugin for SublimeText3 tries to use the local symlinked node_modules/.bin/eslint, so suddenly when I changed from npm install to yarn install, the linting in my editor stopped working.

---

The real-life situation is a little more complex; I have something like 7 web UI projects are all basically clones of each other (or all based on a common framework of dependencies anyway).

All these projects share 1 "common" dependency module that sets them up (like a boilerplate template that has all the UI dependencies and grunt build stuff all set up already). So when I update a version of a library or a build step, I can change it in 1 code repo, not 7.

So all these projects just have 1 dependency that is to a private repo that I set up. That private repo has the 2-dozen actual dependencies, like eslint (this was originally done through peerDependencies before NPM stopped auto-installing those back around npm v3).

The grunt-eslint -> eslint relation just publicly demonstrates the same issue.

[rally25rs]

Did a little more digging and I think this issue should have been fixed by: #1210 (Nested Executables Fix).

Strangely there seemed to be a test that is now marked "skip":

// disabled to resolve https://github.com/yarnpkg/yarn/pull/1210
test.skip('install should hoist nested bin scripts', (): Promise<void> => {

[edmorley]

The test is skipped since #1210 was reverted by #1867.

[mateodelnorte]

+1 on this issue. This bug breaks compatibility with npm and will cause issues if child dependencies rely on those binaries being installed.

➜  nycnode git:(master) ✗ rm -rf node_modules && npm i && ls node_modules/.bin
_mocha            cross-env         meta-npm-clean    meta-npm-run      meta-yarn-install mocha             which
concurrent        loop              meta-npm-install  meta-yarn         meta-yarn-link    semver
concurrently      meta-npm          meta-npm-link     meta-yarn-clean   mkdirp            tree-kill

➜  nycnode git:(master) ✗ rm -rf node_modules && yarn && ls node_modules/.bin
meta-npm          meta-npm-install  meta-npm-run      meta-yarn-clean   meta-yarn-link
meta-npm-clean    meta-npm-link     meta-yarn         meta-yarn-install



md5-a102b5385c46d172368e0c9777d1ff47



npm i -g meta # install meta globally
npm i meta-npm # install npm plugin
meta npm install

nycnode:
nycnode ✓

nycnode-denormalizer:
npm WARN [email protected] requires a peer of [email protected] but none was installed.
npm WARN [email protected] No repository field.
nycnode-denormalizer ✓

nycnode-meetup-ingestor:
npm WARN deprecated [email protected]: Jade has been renamed to pug, please install the latest version of pug instead of jade
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN deprecated [email protected]: graceful-fs v3.0.0 and before will fail on node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible. Use 'npm ls graceful-fs' to find it in the tree.

...



md5-5ea01e052cf66e24bc51f9760ff70b78



npm i -g meta # install meta globally
npm i meta-yarn # install yarn plugin
meta yarn install

nycnode:
yarn install v0.21.3
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
✨  Done in 0.89s.
nycnode ✓

nycnode-denormalizer:
/bin/sh: /Users/mateodelnorte/development/tmpp/nycnode/node_modules/.bin/cross-env: No such file or directory
nycnode-denormalizer exited with error: Error: Command failed: "/Users/mateodelnorte/development/tmpp/nycnode/node_modules/.bin/cross-env" yarn install

nycnode-meetup-ingestor:
/bin/sh: /Users/mateodelnorte/development/tmpp/nycnode/node_modules/.bin/cross-env: No such file or directory
nycnode-meetup-ingestor exited with error: Error: Command failed: "/Users/mateodelnorte/development/tmpp/nycnode/node_modules/.bin/cross-env" yarn install

[palmerj3]

Can we have a compromise here? As long as transitive deps that specify bins put the bins in ../node_modules/.bin then resolution will occur. The top level app would still not have access to the transitive dep bins but deps that actually rely on those bins would.

I would say if an app absolutely relies on a transitive dep bin being in the top level ./node_modules/.bin directory then a simple postinstall script to create that symlink would work fine.

Thoughts?

[mateatslc]

@rally25rs I have a setup/situation similar to what you've described: one centralized private package for dev tooling used by several other projects. I've solved it by republishing the symlinks in that common package through the bin property in the package.json like this:

{
  ...
  "dependencies": {
    "ava": "0.18.2",
    "eslint": "3.18.0",
    ...
    "nyc": "10.2.0",
    "rimraf": "2.6.1",
    "webpack": "2.3.2",
    "webpack-dev-server": "2.4.2"
  },
  "bin": {
    "ava": "./node_modules/.bin/ava",
    "eslint": "./node_modules/.bin/eslint",
    ...
    "nyc": "./node_modules/.bin/nyc",
    "rimraf": "./node_modules/.bin/rimraf",
    "webpack": "./node_modules/.bin/webpack",
    "webpack-dev-server": "./node_modules/.bin/webpack-dev-server"
  },
  ...
}

With that in place, consumers of the package get correct symlinks ready to go.

[mateodelnorte]

@mateatslc that won't work for me, as my dependency commands can be optionally installed.

As @edmorley pointed out, this was already deemed a bug in #1210 and #1867. A fix was already committed, but was then rolled back because it caused other breakages. We don't need to be debating the merits of a fix.

The bigger issue for me is what is says about yarn that this can go five months without being fixed.
:-

[palmerj3]

@mateodelnorte is there a reason you can't have a postinstall script that creates symlinks in ./node_modules/.bin? Genuinely curious if that won't work for some reason for your use case.

[mateodelnorte]

It might, but it doesn't seem reasonable that every one of my plugin packages should have logic in package.json to check to see whether cross-env's bin has already been symlinked into the current ./node_modules/.bin just because yarn breaks compat with npm.

I understand the question at hand that probably led to this not yet being implemented: In the event that there are different versions of cross-env, for example, used by dependencies, which version gets installed in ./node_modules/.bin?

But if yarn kicks the can on answering the question then it no longer is 1:1 compat with npm. So, now if I need 1:1 compat with npm, I need to do extra work in every one of my plugin modules to make it compatible with yarn and npm. The right thing to do seems that yarn should install the same version of dependency binaries into ./node_modules/.bin that npm would end up installing. Or, if yarn wants to make some compromise for performance benefits - just pick the first encountered.

[spoike]

One example npm package is react-scripts that has a transitive dependency on eslint.

It is just weird that specifying tasks with eslint works when doing npm install but not with yarn.

[djMax]

If the "transitive" bin script specification works cross platform, that seems like a fair compromise to me. If my package depends on A which depends on B and I need a script in B from MY code, I do indeed depend on that script being provided by A, even if it doesn't implement it directly, so that feels "correct" for A to provide the script.

Does NPM also work with this now-conflicting binary script?

[mateodelnorte]

npm works as you described. The crux of this issue is that yarn does not, breaking compat.

[rally25rs]

Notes copied from #3272

---

If you have something like:

"dependencies": {
  "eslint": "3.13.1",
  "standard": "1.2.3"
}

and assume that the standard package also depends on a different version of eslint [email protected]

There is nothing in the current implementation of this PR that would guarantee that node_modules/.bin/eslint was a link to the 3.13.1 version of eslint, not 3.7.1

I believe result should be:

node_modules/eslint -- version 3.13.1
node_modules/.bin/eslint -> ../eslint/bin/index.js -- version 3.13.1
node_modules/standard/node_modules/eslint -- version 3.7.1
node_modules/standard/node_modules/.bin/eslint -> ../eslint/bin/index.js -- version 3.7.1

---

I think we also need to figure out what the "correct" behavior is if we have a dependency graph like:

myPkg -> pkgA -> eslint@1
myPkg -> pkgB -> eslint@2

Which eslint is in node_modules/.bin? First one resolved? last one resolved? (I'm assuming questions like this are why this bug has been open for months).

---

Implementation Discussion

My recommendation for implementing this is that:

• the transient dependencies are scanned for bins, and the "greatest version" gets tagged to be linked back to top-level node_modules/.bin (I assume there is a version comparison function somewhere in Yarn?).
• Then the immediate dependencies are scanned for bins, and those overwrite the ones from transients (immediate dependencies always take priority).
• (preferably) This would be built up in-memory then links made all at once instead of overwriting them on the filesystem over and over.

I'm pretty sure that can be implemented entirely from package-linker so no other classes would have to change. (though I did have the thought to make a "bin-hoister" that would traverse the packages again, but I think that's overkill)