feat(lifecycle): run scripts in dep order

Fixes: #16

index.js

@@ -9,6 +9,7 @@ const getPrefix = require('./lib/get-prefix.js')
 const lifecycle = require('npm-lifecycle')
 const lockVerify = require('lock-verify')
 const path = require('path')
+const logi = require('./lib/logical-tree.js')
 const rimraf = BB.promisify(require('rimraf'))
 
 const readFileAsync = BB.promisify(fs.readFile)
@@ -19,22 +20,38 @@ class Installer {
 
     // Stats
     this.startTime = Date.now()
-    this.runTime = null
+    this.runTime = 0
     this.pkgCount = 0
 
     // Misc
     this.pkg = null
-    this.scriptQ = []
   }
 
   run () {
+    return this.prepare()
+    .then(() => this.runScript('preinstall', this.pkg, this.prefix))
+    .then(() => this.extractTree(this.logicalTree))
+    .then(() => this.runScript('install', this.pkg, this.prefix))
+    .then(() => this.runScript('postinstall', this.pkg, this.prefix))
+    .then(() => {
+      extract.stopWorkers()
+      this.runTime = Date.now() - this.startTime
+      return this
+    }, e => {
+      extract.stopWorkers()
+      throw e
+    })
+  }
+
+  prepare () {
     extract.startWorkers()
 
     return (
       this.opts.prefix
       ? BB.resolve(this.opts.prefix)
       : getPrefix(process.cwd())
-    ).then(prefix => {
+    )
+    .then(prefix => {
       this.prefix = prefix
       return BB.join(
         readJson(prefix, 'package.json'),
@@ -45,32 +62,17 @@ class Installer {
           this.pkg = pkg
         }
       )
-    }).then(() => {
-      return config(this.prefix, process.argv, this.pkg)
-    }).then(conf => {
+    })
+    .then(() => config(this.prefix, process.argv, this.pkg))
+    .then(conf => {
       this.config = conf
       return BB.join(
         this.checkLock(),
         rimraf(path.join(this.prefix, 'node_modules'))
       )
     }).then(() => {
-      return this.runScript('preinstall', this.pkg, this.prefix)
-    }).then(() => {
-      return this.extractDeps(
-        path.join(this.prefix, 'node_modules'),
-        this.pkg._shrinkwrap.dependencies
-      )
-    }).then(() => {
-      return this.runScript('install', this.pkg, this.prefix)
-    }).then(() => {
-      return this.runScript('postinstall', this.pkg, this.prefix)
-    }).then(() => {
-      extract.stopWorkers()
-      this.runTime = Date.now() - this.startTime
-      return this
-    }, e => {
-      extract.stopWorkers()
-      throw e
+      // This needs to happen -after- we've done checkLock()
+      this.logicalTree = logi(this.pkg, this.pkg._shrinkwrap)
     })
   }
 
@@ -97,36 +99,31 @@ class Installer {
     })
   }
 
-  extractDeps (modPath, deps) {
-    return BB.map(Object.keys(deps || {}), name => {
-      const child = deps[name]
-      const childPath = path.join(modPath, name)
-      return extract.child(name, child, childPath, this.config).then(() => {
-        return readJson(childPath, 'package.json')
-      }).tap(pkg => {
+  extractTree (tree) {
+    const deps = tree.dependencies.values()
+    return BB.map(deps, child => {
+      if (child.pending) { return hasCycle(child) || child.pending }
+      if (child.dev && this.config.config.production) { return }
+      const childPath = path.join(
+        this.prefix,
+        'node_modules',
+        child.address.replace(/:/g, '/node_modules/')
+      )
+      child.pending = BB.resolve()
+      .then(() => extract.child(child.name, child, childPath, this.config))
+      .then(() => readJson(childPath, 'package.json'))
+      .then(pkg => {
         return this.runScript('preinstall', pkg, childPath)
-      }).then(pkg => {
-        return this.extractDeps(
-          path.join(childPath, 'node_modules'), child.dependencies
-        ).then(dependencies => {
-          return {
-            name,
-            package: pkg,
-            child,
-            childPath,
-            dependencies: dependencies.reduce((acc, dep) => {
-              acc[dep.name] = dep
-              return acc
-            }, {})
-          }
+        .then(() => this.extractTree(child))
+        .then(() => this.runScript('install', pkg, childPath))
+        .then(() => this.runScript('postinstall', pkg, childPath))
+        .then(() => {
+          this.pkgCount++
+          return this
         })
-      }).tap(full => {
-        this.pkgCount++
-        return this.runScript('install', full.package, childPath)
-      }).tap(full => {
-        return this.runScript('postinstall', full.package, childPath)
       })
-    }, {concurrency: 50})
+      return child.pending
+    }, { concurrency: 50 })
   }
 
   runScript (stage, pkg, pkgPath) {
@@ -154,3 +151,14 @@ function readJson (jsonPath, name, ignoreMissing) {
 function stripBOM (str) {
   return str.replace(/^\uFEFF/, '')
 }
+
+function hasCycle (child, seen) {
+  seen = seen || new Set()
+  if (seen.has(child.address)) {
+    return true
+  } else {
+    seen.add(child.address)
+    const deps = Array.from(child.dependencies.values())
+    return deps.some(dep => hasCycle(dep, seen))
+  }
+}

lib/logical-tree.js

@@ -0,0 +1,90 @@
+'use strict'
+
+class LogicalTree {
+  constructor (name, lockNode, address) {
+    this.name = name
+    this.version = lockNode.version
+    this.address = address
+    this.optional = !!lockNode.optional
+    this.dev = !!lockNode.dev
+    this.bundled = !!lockNode.bundled
+    this.resolved = lockNode.resolved
+    this.integrity = lockNode.integrity
+    this.dependencies = new Map()
+    this.pending = null
+  }
+
+  addDep (dep) {
+    this.dependencies.set(dep.name, dep)
+  }
+
+  getDep (name) {
+    return this.dependencies.get(name)
+  }
+}
+
+module.exports = logicalTree
+function logicalTree (pkg, pkgLock, opts) {
+  const tree = new LogicalTree(pkg.name, pkg, null)
+  const allDeps = new Map()
+  Array.from(
+    new Set(Object.keys(pkg.devDependencies || {})
+    .concat(Object.keys(pkg.optionalDependencies || {}))
+    .concat(Object.keys(pkg.dependencies || {})))
+  ).forEach(name => {
+    let dep = allDeps.get(name)
+    if (!dep) {
+      dep = new LogicalTree(name, (pkgLock.dependencies || {})[name] || {}, name)
+    }
+    addChild(dep, tree, allDeps, pkgLock)
+  })
+  return tree
+}
+
+function addChild (dep, tree, allDeps, pkgLock) {
+  tree.addDep(dep)
+  allDeps.set(dep.address, dep)
+  const addr = dep.address
+  const lockNode = atAddr(pkgLock, addr)
+  Object.keys(lockNode.requires || {}).forEach(name => {
+    const tdepAddr = reqAddr(pkgLock, name, lockNode.requires[name], addr)
+    let tdep = allDeps.get(tdepAddr)
+    if (!tdep) {
+      tdep = new LogicalTree(name, atAddr(pkgLock, tdepAddr), tdepAddr)
+      addChild(tdep, dep, allDeps, pkgLock)
+    } else {
+      dep.addDep(tdep)
+    }
+  })
+}
+
+module.exports._reqAddr = reqAddr
+function reqAddr (pkgLock, name, version, fromAddr) {
+  const lockNode = atAddr(pkgLock, fromAddr)
+  const child = (lockNode.dependencies || {})[name]
+  if (child && child.version === version) {
+    return `${fromAddr}:${name}`
+  } else {
+    const parts = fromAddr.split(':')
+    while (parts.length) {
+      parts.pop()
+      const parent = atAddr(pkgLock, parts.join(':'))
+      if (parent) {
+        const child = (parent.dependencies || {})[name]
+        if (child && child.version === version) {
+          return `${parts.join(':')}${parts.length ? ':' : ''}${name}`
+        }
+      }
+    }
+    throw new Error('req not found')
+  }
+}
+
+module.exports._atAddr = atAddr
+function atAddr (pkgLock, addr) {
+  if (!addr.length) { return pkgLock }
+  const parts = addr.split(':')
+  return parts.reduce((acc, next) => {
+    return acc && (acc.dependencies || {})[next]
+  }, pkgLock)
+}

test/specs/index.js

@@ -115,11 +115,16 @@ test('handles dependency list with only shallow subdeps', t => {
   const prefix = fixtureHelper.write(pkgName, {
     'package.json': {
       name: pkgName,
-      version: pkgVersion
+      version: pkgVersion,
+      dependencies: {
+        'a': '^1'
+      }
     },
     'package-lock.json': {
       dependencies: {
-        a: {}
+        a: {
+          version: '1.1.1'
+        }
       },
       lockfileVersion: 1
     }
@@ -150,13 +155,22 @@ test('handles dependency list with only deep subdeps', t => {
   const prefix = fixtureHelper.write(pkgName, {
     'package.json': {
       name: pkgName,
-      version: pkgVersion
+      version: pkgVersion,
+      dependencies: {
+        a: '^1'
+      }
     },
     'package-lock.json': {
       dependencies: {
         a: {
+          version: '1.1.1',
+          requires: {
+            b: '2.2.2'
+          },
           dependencies: {
-            b: {}
+            b: {
+              version: '2.2.2'
+            }
           }
         }
       },
@@ -206,11 +220,14 @@ test('runs lifecycle hooks of packages with env variables', t => {
         preinstall: writeEnvScript,
         install: writeEnvScript,
         postinstall: writeEnvScript
+      },
+      dependencies: {
+        a: '^1'
       }
     },
     'package-lock.json': {
       dependencies: {
-        a: {}
+        a: { version: '1.0.0' }
       },
       lockfileVersion: 1
     }
@@ -252,6 +269,7 @@ test('skips lifecycle scripts with ignoreScripts is set', t => {
     'package.json': {
       name: pkgName,
       version: pkgVersion,
+      dependencies: { a: '^1' },
       scripts: {
         preinstall: writeEnvScript,
         install: writeEnvScript,
@@ -260,7 +278,7 @@ test('skips lifecycle scripts with ignoreScripts is set', t => {
     },
     'package-lock.json': {
       dependencies: {
-        a: {}
+        a: { version: '1.0.0' }
       },
       lockfileVersion: 1
     }