link bins of transitive deps to top level

__tests__/commands/install/bin-links.js

@@ -0,0 +1,128 @@
+/* @flow */
+
+import * as fs from '../../../src/util/fs.js';
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000;
+
+const request = require('request');
+const path = require('path');
+import {runInstall} from '../_helpers.js';
+
+async function linkAt(config, ...relativePath): Promise<string> {
+  const joinedPath = path.join(config.cwd, ...relativePath);
+  const stat = await fs.lstat(joinedPath);
+  if (stat.isSymbolicLink()) {
+    const linkPath = await fs.readlink(joinedPath);
+    return linkPath;
+  } else {
+    const contents = await fs.readFile(joinedPath);
+    return /node" +"\$basedir\/([^"]*\.js)"/.exec(contents)[1];
+  }
+}
+
+beforeEach(request.__resetAuthedRequests);
+afterEach(request.__resetAuthedRequests);
+
+test('install should hoist nested bin scripts', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-nested-bin', async (config) => {
+    const binScripts = await fs.walk(path.join(config.cwd, 'node_modules', '.bin'));
+    // need to double the amount as windows makes 2 entries for each dependency
+    // so for below, there would be an entry for eslint and eslint.cmd on win32
+    const amount = process.platform === 'win32' ? 20 : 10;
+    expect(binScripts).toHaveLength(amount);
+
+    expect(await linkAt(config, 'node_modules', '.bin', 'standard'))
+      .toEqual('../standard/bin/cmd.js');
+    expect(await linkAt(config, 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+  });
+});
+
+// dependency tree:
+//   eslint@3.7.0
+//   standard
+//   standard -> eslint@3.7.1
+// result should be:
+//   eslint 3.7.0 is linked in /.bin because it takes priority over the transitive 3.7.1
+//   eslint 3.7.1 is linked in standard/node_modules/.bin
+test('direct dependency bin takes priority over transitive bin', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-duplicate-bin', async (config) => {
+    expect(await linkAt(config, 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+    expect(await linkAt(config, 'node_modules', 'standard', 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+  });
+});
+
+test.concurrent('install should respect --no-bin-links flag', (): Promise<void> => {
+  return runInstall({binLinks: false}, 'install-nested-bin', async (config) => {
+    const binExists = await fs.exists(path.join(config.cwd, 'node_modules', '.bin'));
+    expect(binExists).toBeFalsy();
+  });
+});
+
+
+// Scenario: Transitive dependency having version that is overridden by newer version as the direct dependency.
+// Behavior: eslint@3.12.2 is symlinked in node_modeules/.bin
+//           and eslint@3.10.1 is symlinked to node_modules/sample-dep-eslint-3.10.1/node_modules/.bin
+test('newer transitive dep is overridden by older direct dep', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-bin-links-newer', async (config) => {
+    expect(await linkAt(config, 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+    expect(await linkAt(config, 'node_modules', 'sample-dep-eslint-3.10.1', 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+  });
+});
+
+// Scenario: Transitive dependency having version that is overridden by older version as the direct dependency.
+// Behavior: eslint@3.10.1 is symlinked in node_modeules/.bin
+//           and eslint@3.12.2 is symlinked to node_modules/sample-dep-eslint-3.12.2/node_modules/.bin
+test('newer transitive dep is overridden by older direct dep', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-bin-links-older', async (config) => {
+    expect(await linkAt(config, 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+    expect(await linkAt(config, 'node_modules', 'sample-dep-eslint-3.12.2', 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+  });
+});
+
+// Scenario: Transitive dependency having version that is overridden by newer version as the dev dependency.
+// Behavior: eslint@3.12.2 is symlinked in node_modeules/.bin
+//           and eslint@3.10.1 dependency for sample-dep-eslint-3.10.1 module is not linked.
+// SKIPPED because this seems like an NPM bug more than intentional design.
+//           Why would it matter if the direct dependency is a dev one or not when linking the transient dep?
+test.skip('transitive dep is overridden by dev dep', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-bin-links-dev', async (config) => {
+    expect(await linkAt(config, 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+    expect(await linkAt(config, 'node_modules', 'sample-dep-eslint-3.10.1', 'node_modules', '.bin', 'eslint'))
+      .not.toBeDefined();
+  });
+});
+
+// Scenario: Transitive dependency having version that is conflicting with another transitive dependency version.
+// Behavior: eslint@3.10.1 is symlinked in node_modeules/.bin
+//           and eslint@3.12.2 is symlinked to node_modules/sample-dep-eslint-3.12.2/node_modules/.bin.
+//           Here it seems like NPM add the modules in alphabatical order
+//           and transitive deps of first dependency is installed at top level.
+test('first transient dep is installed when same level and reference count', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-bin-links-conflicting', async (config) => {
+    expect(await linkAt(config, 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+    expect(await linkAt(config, 'node_modules', 'sample-dep-eslint-3.12.2', 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+  });
+});
+
+// Scenario: Transitive dependency having version that is conflicting with another dev transitive dependency version.
+// Behavior: eslint@3.10.1 is symlinked in node_modeules/.bin
+//           and eslint@3.12.2 is symlinked to node_modules/sample-dep-eslint-3.12.2/node_modules/.bin.
+//           Whether the dependencies are devDependencies or not does not seem to matter to NPM.
+test('first dep is installed when same level and reference count and one is a dev dep', (): Promise<void> => {
+  return runInstall({binLinks: true}, 'install-bin-links-conflicting-dev', async (config) => {
+    expect(await linkAt(config, 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+    expect(await linkAt(config, 'node_modules', 'sample-dep-eslint-3.12.2', 'node_modules', '.bin', 'eslint'))
+      .toEqual('../eslint/bin/eslint.js');
+  });
+});

__tests__/commands/install/integration.js

@@ -526,25 +526,6 @@ test.concurrent(
   },
 );
 
-// disabled to resolve https://github.com/yarnpkg/yarn/pull/1210
-test.skip('install should hoist nested bin scripts', (): Promise<void> => {
-  return runInstall({binLinks: true}, 'install-nested-bin', async (config) => {
-    const binScripts = await fs.walk(path.join(config.cwd, 'node_modules', '.bin'));
-    // need to double the amount as windows makes 2 entries for each dependency
-    // so for below, there would be an entry for eslint and eslint.cmd on win32
-    const amount = process.platform === 'win32' ? 20 : 10;
-    expect(binScripts).toHaveLength(amount);
-    expect(binScripts.findIndex((f) => f.basename === 'eslint')).toBeGreaterThanOrEqual(0);
-  });
-});
-
-test.concurrent('install should respect --no-bin-links flag', (): Promise<void> => {
-  return runInstall({binLinks: false}, 'install-nested-bin', async (config) => {
-    const binExists = await fs.exists(path.join(config.cwd, 'node_modules', '.bin'));
-    expect(binExists).toBeFalsy();
-  });
-});
-
 test.concurrent('install should update a dependency to yarn and mirror (PR import scenario 2)', (): Promise<void> => {
   // mime-types@2.0.0 is gets updated to mime-types@2.1.11 via
   // a change in package.json,

__tests__/fixtures/install/install-bin-links-conflicting-dev/package.json

@@ -0,0 +1,9 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "sample-dep-eslint-3.10.1": "file:sample-dep-eslint-3.10.1"
+  },
+  "devDependencies": {
+    "sample-dep-eslint-3.12.2": "file:sample-dep-eslint-3.12.2"
+  }
+}

__tests__/fixtures/install/install-bin-links-conflicting-dev/sample-dep-eslint-3.10.1/package.json

@@ -0,0 +1,6 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.10.1"
+  }
+}

__tests__/fixtures/install/install-bin-links-conflicting-dev/sample-dep-eslint-3.12.2/package.json

@@ -0,0 +1,6 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.12.2"
+  }
+}

__tests__/fixtures/install/install-bin-links-conflicting/package.json

@@ -0,0 +1,7 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "sample-dep-eslint-3.12.2": "file:sample-dep-eslint-3.12.2",
+    "sample-dep-eslint-3.10.1": "file:sample-dep-eslint-3.10.1"
+  }
+}

__tests__/fixtures/install/install-bin-links-conflicting/sample-dep-eslint-3.10.1/package.json

@@ -0,0 +1,6 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.10.1"
+  }
+}

__tests__/fixtures/install/install-bin-links-conflicting/sample-dep-eslint-3.12.2/package.json

@@ -0,0 +1,6 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.12.2"
+  }
+}

__tests__/fixtures/install/install-bin-links-dev/package.json

@@ -0,0 +1,9 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "sample-dep-eslint-3.10.1": "file:sample-dep-eslint-3.10.1"
+  },
+  "devDependencies": {
+    "eslint": "3.12.2"
+  }
+}

__tests__/fixtures/install/install-bin-links-dev/sample-dep-eslint-3.10.1/package.json

@@ -0,0 +1,6 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.10.1"
+  }
+}

__tests__/fixtures/install/install-bin-links-newer/package.json

@@ -0,0 +1,7 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.12.2",
+    "sample-dep-eslint-3.10.1": "file:sample-dep-eslint-3.10.1"
+  }
+}

__tests__/fixtures/install/install-bin-links-newer/sample-dep-eslint-3.10.1/package.json

@@ -0,0 +1,6 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.10.1"
+  }
+}

__tests__/fixtures/install/install-bin-links-older/package.json

@@ -0,0 +1,7 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.10.1",
+    "sample-dep-eslint-3.12.2": "file:sample-dep-eslint-3.12.2"
+  }
+}

__tests__/fixtures/install/install-bin-links-older/sample-dep-eslint-3.12.2/package.json

@@ -0,0 +1,6 @@
+{
+  "version": "0.0.1",
+  "dependencies": {
+    "eslint": "3.12.2"
+  }
+}

__tests__/fixtures/install/install-duplicate-bin/package.json

@@ -0,0 +1,6 @@
+{
+  "dependencies": {
+    "eslint": "3.7.0",
+    "standard": "8.4.0"
+  }
+}