Fix for #1214, linked scoped dependencies should not be overwritten (#…

…1970)

* Fix for #1214

Scoped dependencies are nested one folder deeper than traditional
dependencies, the possiblyExtraneous needed updating to work with the
actual dependencies, not the entire scoped folder.

* Fix typo/variable renaming

__tests__/commands/install/integration.js

@@ -695,3 +695,21 @@ test.concurrent('install a module with incompatible optional dependency should s
       assert.ok(!(await fs.exists(path.join(config.cwd, 'node_modules', 'dep-a'))));
     });
   });
+
+test.concurrent('install will not overwrite files in symlinked scoped directories', async (): Promise<void> => {
+  await runInstall({}, 'install-dont-overwrite-linked-scoped', async (config): Promise<void> => {
+    const dependencyPath = path.join(config.cwd, 'node_modules', '@fakescope', 'fake-dependency');
+    assert.equal(
+      'Symlinked scoped package test',
+      (await fs.readJson(path.join(dependencyPath, 'package.json'))).description,
+    );
+    assert.ok(!(await fs.exists(path.join(dependencyPath, 'index.js'))));
+  }, async (cwd) => {
+    const dirToLink = path.join(cwd, 'dir-to-link');
+    await fs.mkdirp(path.join(cwd, '.yarn-link', '@fakescope'));
+    await fs.symlink(dirToLink, path.join(cwd, '.yarn-link', '@fakescope', 'fake-dependency'));
+    await fs.mkdirp(path.join(cwd, 'node_modules', '@fakescope'));
+    await fs.symlink(dirToLink, path.join(cwd, 'node_modules', '@fakescope', 'fake-dependency'));
+  });
+});
+

__tests__/fixtures/install/install-dont-overwrite-linked-scoped/.npmrc

@@ -0,0 +1 @@
+yarn-offline-mirror=./mirror-for-offline

__tests__/fixtures/install/install-dont-overwrite-linked-scoped/dir-to-link/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "@fakescope/fake-dependency",
+  "description": "Symlinked scoped package test",
+  "version": "1.0.1",
+  "dependencies": {},
+  "license": "MIT"
+}

__tests__/fixtures/install/install-dont-overwrite-linked-scoped/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@fakescope/fake-dependency": "1.0.1"
+  }
+}

__tests__/fixtures/install/install-dont-overwrite-linked-scoped/yarn.lock

@@ -0,0 +1,5 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+"@fakescope/[email protected]":
+  version "1.0.1"
+  resolved "@fakescope-fake-dependency-1.0.1.tgz#477dafd486d856af0b3faf5a5f1c895001221609"

src/config.js

@@ -177,7 +177,21 @@ export default class Config {
 
     await fs.mkdirp(this.globalFolder);
     await fs.mkdirp(this.linkFolder);
-    this.linkedModules = await fs.readdir(this.linkFolder);
+
+    this.linkedModules = [];
+
+    const linkedModules = await fs.readdir(this.linkFolder);
+
+    for (const dir of linkedModules) {
+      const linkedPath = path.join(this.linkFolder, dir);
+
+      if (dir[0] === '@') { // it's a scope, not a package
+        const scopedLinked = await fs.readdir(linkedPath);
+        this.linkedModules.push(...scopedLinked.map((scopedDir) => path.join(dir, scopedDir)));
+      } else {
+        this.linkedModules.push(dir);
+      }
+    }
 
     for (const key of Object.keys(registries)) {
       const Registry = registries[key];
@@ -372,7 +386,7 @@ export default class Config {
 
   /**
    * Read normalized package info according yarn-metadata.json
-   * throw an error if package.json was not found 
+   * throw an error if package.json was not found
    */
 
   async readManifest(dir: string, priorityRegistry?: RegistryNames, isRoot?: boolean = false): Promise<Manifest> {
@@ -386,8 +400,8 @@ export default class Config {
   }
 
  /**
- * try get the manifest file by looking 
- * 1. mainfest fiel in cache 
+ * try get the manifest file by looking
+ * 1. mainfest file in cache
  * 2. manifest file in registry
  */
   maybeReadManifest(dir: string, priorityRegistry?: RegistryNames, isRoot?: boolean = false): Promise<?Manifest> {

src/package-linker.js

@@ -148,6 +148,9 @@ export default class PackageLinker {
       });
     }
 
+    // keep track of all scoped paths to remove empty scopes after copy
+    const scopedPaths = new Set();
+
     // register root & scoped packages as being possibly extraneous
     const possibleExtraneous: Set<string> = new Set();
     for (const folder of this.config.registryFolders) {
@@ -158,12 +161,14 @@ export default class PackageLinker {
         let filepath;
         for (const file of files) {
           filepath = path.join(loc, file);
-          possibleExtraneous.add(filepath);
           if (file[0] === '@') { // it's a scope, not a package
+            scopedPaths.add(filepath);
             const subfiles = await fs.readdir(filepath);
             for (const subfile of subfiles) {
               possibleExtraneous.add(path.join(filepath, subfile));
             }
+          } else {
+            possibleExtraneous.add(filepath);
           }
         }
       }
@@ -200,6 +205,14 @@ export default class PackageLinker {
       },
     });
 
+    // remove any empty scoped directories
+    for (const scopedPath of scopedPaths) {
+      const files = await fs.readdir(scopedPath);
+      if (files.length === 0) {
+        await fs.unlink(scopedPath);
+      }
+    }
+
     //
     if (this.config.binLinks) {
       const tickBin = this.reporter.progress(flatTree.length);