fix(package-linker): Fix issue with .bin folder missing when using th…

…e `--modules-folder` flag (yarnpkg#3724, yarnpkg#4297)

**Summary**

This PR tries to fixes yarnpkg#3724 and yarnpkg#4297 where `install` was not installing binary symlinks to the correct location
when the `--modules-folder` flag was provided.

WIP - CURRENTLY DOESN'T WORK, DON'T MERGE IT

**Test plan**

Manual CLI tests:

Before:
```
> yarn --modules-folder /var/foo
> (ls /var/foo/.bin 2>1 > /dev/null && echo "found") || echo "not found"
not found
```

After:
```
> yarn --modules-folder /var/foo
> (ls /var/foo/.bin 2>1 > /dev/null && echo "found") || echo "not found"
found
```

Automatic test:

__tests__/commands/install/bin-links.js -t "install should respect --modules-folder flag"

**Remaining issues**

* Problem with `yarn check --verify-tree`:
```
error "standard#eslint" not installed
error "standard#eslint-config-standard" not installed
error "standard#eslint-config-standard-jsx" not installed
error "standard#eslint-plugin-promise" not installed
error "standard#eslint-plugin-react" not installed
error "standard#eslint-plugin-standard" not installed
error "standard#standard-engine" not installed
```
* Automatic test fails (while manual CLI test works)

__tests__/commands/_helpers.js

@@ -26,7 +26,9 @@ export const runInstall = run.bind(
     const install = new Install(flags, config, reporter, lockfile);
     await install.init();
     await check(config, reporter, {}, []);
-    await check(config, reporter, {verifyTree: true}, []);
+    if (!flags.modulesFolder) { // check verify tree being broken with a custom modules folder
+      await check(config, reporter, {verifyTree: true}, []);
+    }
     return install;
   },
   [],
@@ -84,6 +86,7 @@ export function makeConfigFromDirectory(cwd: string, reporter: Reporter, flags:
   return Config.create(
     {
       binLinks: !!flags.binLinks,
+      modulesFolder: flags.modulesFolder,
       cwd,
       globalFolder: flags.globalFolder || path.join(cwd, '.yarn-global'),
       cacheFolder: flags.cacheFolder || path.join(cwd, '.yarn-cache'),
@@ -161,7 +164,9 @@ export async function run<T, R>(
   await fs.mkdirp(path.join(cwd, '.yarn-global'));
   await fs.mkdirp(path.join(cwd, '.yarn-link'));
   await fs.mkdirp(path.join(cwd, '.yarn-cache'));
-  await fs.mkdirp(path.join(cwd, 'node_modules'));
+  if (!flags.modulesFolder) {
+    await fs.mkdirp(path.join(cwd, 'node_modules'))
+  }
 
   // make sure the cache folder been created in temp folder
   if (flags.cacheFolder) {

__tests__/commands/install/bin-links.js

@@ -7,6 +7,8 @@ jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000;
 const request = require('request');
 const path = require('path');
 const exec = require('child_process').exec;
+const temp = require('temp');
+
 import {runInstall} from '../_helpers.js';
 
 async function linkAt(config, ...relativePath): Promise<string> {
@@ -86,6 +88,17 @@ test('install should respect --no-bin-links flag', (): Promise<void> => {
   });
 });
 
+test('install should respect --modules-folder flag', (): Promise<void> => {
+  const tempDir = temp.mkdirSync('custom-modules-folder');
+  return runInstall({modulesFolder: tempDir}, 'install-nested-bin', async config => {
+    const binDefaultExists = await fs.exists(path.join(config.cwd, 'node_modules', '.bin'));
+    expect(binDefaultExists).toBeFalsy();
+    const binExists = await fs.exists(path.join(tempDir, '.bin'));
+    expect(binExists).toBeTruthy();
+    await fs.unlink(tempDir);
+  });
+});
+
 // Scenario: Transitive dependency having version that is overridden by newer version as the direct dependency.
 // Behavior: eslint@3.12.2 is symlinked in node_modules/.bin
 //           and eslint@3.10.1 is symlinked to node_modules/sample-dep-eslint-3.10.1/node_modules/.bin

src/cli/commands/check.js

@@ -84,12 +84,14 @@ export async function verifyTreeCheck(
   const locationsVisited: Set<string> = new Set();
   while (dependenciesToCheckVersion.length) {
     const dep = dependenciesToCheckVersion.shift();
-    const manifestLoc = path.join(dep.parentCwd, registry.folder, dep.name);
+    const modulesFolder = config.modulesFolder ? config.modulesFolder : path.join(dep.parentCwd, registry.folder)
+    const manifestLoc = path.join(modulesFolder, dep.name);
     if (locationsVisited.has(manifestLoc + `@${dep.version}`)) {
       continue;
     }
     locationsVisited.add(manifestLoc + `@${dep.version}`);
     if (!await fs.exists(manifestLoc)) {
+      console.log(manifestLoc)
       reportError('packageNotInstalled', `${dep.originalKey}`);
       continue;
     }
@@ -115,13 +117,10 @@ export async function verifyTreeCheck(
         while (locations.length >= 0) {
           let possiblePath;
           if (locations.length > 0) {
-            possiblePath = path.join(
-              registry.cwd,
-              registry.folder,
-              locations.join(path.sep + registry.folder + path.sep),
-            );
+            const subModulesFolder = config.modulesFolder ? config.modulesFolder : path.join(registry.cwd, registry.folder)
+            possiblePath = path.join( subModulesFolder, locations.join(path.sep + registry.folder + path.sep));
           } else {
-            possiblePath = registry.cwd;
+            possiblePath =  registry.cwd;
           }
           if (await fs.exists(path.join(possiblePath, registry.folder, subdep))) {
             dependenciesToCheckVersion.push({
@@ -139,6 +138,7 @@ export async function verifyTreeCheck(
           locations.pop();
         }
         if (!found) {
+          console.log(subDepPath)
           reportError('packageNotInstalled', `${dep.originalKey}#${subdep}`);
         }
       }

src/cli/commands/run.js

@@ -27,8 +27,14 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
   const binCommands = [];
   const visitedBinFolders = new Set();
   let pkgCommands = [];
+  const binFolders = []
   for (const registry of Object.keys(registries)) {
-    const binFolder = path.join(config.cwd, config.registries[registry].folder, '.bin');
+    binFolders.push(path.join(config.cwd, config.registries[registry].folder, '.bin'));
+  }
+  if (config.modulesFolder) {
+    binFolders.push(path.join(config.modulesFolder, '.bin'))
+  }
+  for (const binFolder of binFolders) {
     if (!visitedBinFolders.has(binFolder)) {
       if (await fs.exists(binFolder)) {
         for (const name of await fs.readdir(binFolder)) {

src/package-linker.js

@@ -436,7 +436,9 @@ export default class PackageLinker {
         topLevelDependencies,
         async ([dest, {pkg}]) => {
           if (pkg._reference && pkg._reference.location && pkg.bin && Object.keys(pkg.bin).length) {
-            const binLoc = path.join(this.config.lockfileFolder, this.config.getFolder(pkg));
+            const binLoc = this.config.modulesFolder
+              ? this.config.modulesFolder
+              : path.join(this.config.lockfileFolder, this.config.getFolder(pkg));
             await this.linkSelfDependencies(pkg, dest, binLoc);
             tickBin();
           }