feat(optional): ignore failed optional deps (#27)

Fixes: #3

index.js

@@ -29,12 +29,16 @@ class Installer {
     // Misc
     this.log = npmlog
     this.pkg = null
+    this.tree = null
+    this.failedDeps = new Set()
+    this.purgedDeps = new Set()
   }
 
   run () {
     return this.prepare()
     .then(() => this.runScript('preinstall', this.pkg, this.prefix))
     .then(() => this.extractTree(this.logicalTree))
+    .then(() => this.garbageCollect(this.logicalTree))
     .then(() => this.runScript('install', this.pkg, this.prefix))
     .then(() => this.runScript('postinstall', this.pkg, this.prefix))
     .then(() => this.runScript('prepublish', this.pkg, this.prefix))
@@ -137,11 +141,64 @@ class Installer {
           this.pkgCount++
           return this
         })
+        .catch(e => {
+          if (child.optional) {
+            this.pkgCount++
+            this.failedDeps.add(child)
+            return rimraf(childPath)
+          } else {
+            throw e
+          }
+        })
       })
       return child.pending
     }, { concurrency: 50 })
   }
 
+  // A cute little mark-and-sweep collector!
+  garbageCollect (tree) {
+    if (!this.failedDeps.size) { return }
+
+    const liveDeps = new Set()
+    const installer = this
+    const seen = new Set()
+    const failed = this.failedDeps
+    const purged = this.purgedDeps
+    mark(tree)
+    seen.clear()
+    return sweep(tree)
+
+    function mark (tree) {
+      for (let dep of tree.dependencies.values()) {
+        if (seen.has(dep)) { continue }
+        seen.add(dep)
+        if (!failed.has(dep)) {
+          liveDeps.add(dep)
+          mark(dep)
+        }
+      }
+    }
+
+    function sweep (tree) {
+      return BB.map(tree.dependencies.values(), dep => {
+        if (seen.has(dep)) { return }
+        seen.add(dep)
+        return sweep(dep).then(() => {
+          if (!liveDeps.has(dep) && !purged.has(dep)) {
+            const depPath = path.join(
+              installer.prefix,
+              'node_modules',
+              dep.address.replace(/:/g, '/node_modules/')
+            )
+            installer.pkgCount--
+            purged.add(dep)
+            return rimraf(depPath)
+          }
+        })
+      }, { concurrency: 100 })
+    }
+  }
+
   runScript (stage, pkg, pkgPath) {
     if (!this.config.lifecycleOpts.ignoreScripts && pkg.scripts && pkg.scripts[stage]) {
       // TODO(mikesherov): remove pkg._id when npm-lifecycle no longer relies on it
@@ -151,7 +208,9 @@ class Installer {
     return BB.resolve()
   }
 }
+
 module.exports = Installer
+
 module.exports._readJson = readJson
 
 function readJson (jsonPath, name, ignoreMissing) {

package.json

@@ -52,7 +52,7 @@
     "graceful-fs": "^4.1.11",
     "lock-verify": "^1.1.0",
     "npm-lifecycle": "^1.0.3",
-    "npm-logical-tree": "^1.0.0",
+    "npm-logical-tree": "^1.1.0",
     "npm-package-arg": "^5.1.2",
     "npmlog": "^4.1.2",
     "pacote": "^6.0.4",

test/specs/index.js

@@ -120,6 +120,8 @@ test('handles dependency list with only shallow subdeps', t => {
       }
     }),
     'package-lock.json': File({
+      name: pkgName,
+      verson: pkgVersion,
       dependencies: {
         a: {
           version: '1.1.1'
@@ -262,6 +264,112 @@ test('prioritizes npm-shrinkwrap over package-lock if both present', t => {
   })
 })
 
+test('removes failed optional dependencies', t => {
+  const fixture = new Tacks(Dir({
+    'package.json': File({
+      name: pkgName,
+      version: pkgVersion,
+      dependencies: {
+        a: '^1'
+      },
+      optionalDependencies: {
+        b: '^2'
+      }
+    }),
+    'package-lock.json': File({
+      lockfileVersion: 1,
+      requires: true,
+      dependencies: {
+        a: {
+          version: '1.0.0',
+          requires: {
+            b: '2.0.0',
+            d: '4.0.0'
+          }
+        },
+        b: {
+          version: '2.0.0',
+          optional: true,
+          requires: {
+            c: '3.0.0',
+            d: '4.0.0'
+          }
+        },
+        c: {
+          version: '3.0.0',
+          optional: true
+        },
+        d: {
+          version: '4.0.0'
+        }
+      }
+    })
+  }))
+  fixture.create(prefix)
+
+  extract = (name, child, childPath, opts) => {
+    let files
+    if (child.name === 'a') {
+      files = new Tacks(Dir({
+        'package.json': File({
+          name: 'a',
+          version: '1.0.0',
+          dependencies: {
+            b: '^2',
+            d: '^4'
+          }
+        })
+      }))
+    } else if (child.name === 'b') {
+      files = new Tacks(Dir({
+        'package.json': File({
+          name: 'b',
+          version: '2.0.0',
+          dependencies: {
+            c: '^3',
+            d: '^4'
+          },
+          scripts: {
+            install: 'exit 1'
+          }
+        })
+      }))
+    } else if (child.name === 'c') {
+      files = new Tacks(Dir({
+        'package.json': File({
+          name: 'c',
+          version: '3.0.0'
+        })
+      }))
+    } else if (child.name === 'd') {
+      files = new Tacks(Dir({
+        'package.json': File({
+          name: 'd',
+          version: '4.0.0'
+        })
+      }))
+    }
+    files.create(childPath)
+  }
+
+  const originalConsoleLog = console.log
+  console.log = () => {}
+  return new Installer({prefix}).run().then(details => {
+    console.log = originalConsoleLog
+    t.ok(true, 'installer succeeded even with optDep failure')
+    t.equal(details.pkgCount, 2, 'only successful deps counted')
+    const modP = path.join(prefix, 'node_modules')
+    t.ok(fs.statSync(path.join(modP, 'a')), 'dep a is there')
+    t.ok(fs.statSync(path.join(modP, 'd')), 'transitive dep d is there')
+    t.throws(() => {
+      fs.statSync(path.join(prefix, 'node_modules', 'b'))
+    }, 'failed optional dep b not in node_modules')
+    t.throws(() => {
+      fs.statSync(path.join(prefix, 'node_modules', 'c'))
+    }, 'isolated dependency d of failed dep removed')
+  })
+})
+
 test('runs lifecycle hooks of packages with env variables', t => {
   const originalConsoleLog = console.log
   console.log = () => {}