feat(lifecycle): actually run lifecycle scripts correctly

.travis.yml

@@ -1,3 +1,6 @@
+# cipm currently relies on npm >5.4.0 to retrieve config
+before_install:
+  - npm i -g [email protected]
 language: node_js
 sudo: false
 node_js:

index.js

@@ -11,12 +11,12 @@ const lifecycle = require('npm-lifecycle')
 
 module.exports = main
 
-let pkgCount
+function main (opts) {
+  let prefix = path.resolve(opts.prefix)
 
-function main ({ prefix = '.' }) {
   const startTime = Date.now()
   const nodeModulesPath = path.join(prefix, 'node_modules')
-  pkgCount = 0
+  let pkgCount = 0
 
   extract.startWorkers()
 
@@ -52,53 +52,55 @@ function main ({ prefix = '.' }) {
       time: Date.now() - startTime
     }
   })
-}
 
-function runScript (stage, pkg, pkgPath) {
-  if (pkg.scripts && pkg.scripts[stage]) {
-    return config().then(config => lifecycle(pkg, stage, pkgPath, config))
+  function runScript (stage, pkg, pkgPath) {
+    if (pkg.scripts && pkg.scripts[stage]) {
+      // TODO(mikesherov): remove pkg._id when npm-lifecycle no longer relies on it
+      pkg._id = pkg.name + '@' + pkg.version
+      return config(prefix).then(config => lifecycle(pkg, stage, pkgPath, config))
+    }
+    return BB.resolve()
   }
-  return BB.resolve()
-}
 
-function 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)
-    .then(() => {
-      return readJson(childPath, 'package.json')
-    }).tap(pkg => {
-      return runScript('preinstall', pkg, childPath)
-    }).then(pkg => {
-      return 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
-          }, {})
-        }
+  function 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)
+      .then(() => {
+        return readJson(childPath, 'package.json')
+      }).tap(pkg => {
+        return runScript('preinstall', pkg, childPath)
+      }).then(pkg => {
+        return 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
+            }, {})
+          }
+        })
+      }).tap(full => {
+        pkgCount++
+        return runScript('install', full.package, childPath)
+      }).tap(full => {
+        return runScript('postinstall', full.package, childPath)
       })
-    }).tap(full => {
-      pkgCount++
-      return runScript('install', full.package, childPath)
-    }).tap(full => {
-      return runScript('postinstall', full.package, childPath)
-    })
-  }, {concurrency: 50})
-}
+    }, {concurrency: 50})
+  }
 
-function readJson (jsonPath, name, ignoreMissing) {
-  return fs.readFileAsync(path.join(jsonPath, name), 'utf8')
-  .then(str => JSON.parse(str))
-  .catch({code: 'ENOENT'}, err => {
-    if (!ignoreMissing) {
-      throw err
-    }
-  })
+  function readJson (jsonPath, name, ignoreMissing) {
+    return fs.readFileAsync(path.join(jsonPath, name), 'utf8')
+    .then(str => JSON.parse(str))
+    .catch({code: 'ENOENT'}, err => {
+      if (!ignoreMissing) {
+        throw err
+      }
+    })
+  }
 }

lib/config.js

@@ -46,11 +46,24 @@ function _resetConfig () {
   _config = undefined
 }
 
-function getConfig () {
+function getConfig (dir) {
   if (_config) return BB.resolve(_config)
   return readConfig().then(config => {
-    _config = config
-    _config.log = log
+    _config = {
+      config,
+      dir,
+      failOk: false,
+      force: config.force,
+      group: config.group,
+      ignorePrepublish: config['ignore-prepublish'],
+      ignoreScripts: config['ignore-scripts'],
+      log,
+      production: config.production,
+      scriptShell: config['script-shell'],
+      scriptsPrependNodePath: config['scripts-prepend-node-path'],
+      unsafePerm: config['unsafe-perm'],
+      user: config['user']
+    }
 
     return _config
   })

test/specs/index.js

@@ -4,24 +4,23 @@ const test = require('tap').test
 const requireInject = require('require-inject')
 const fixtureHelper = require('../lib/fixtureHelper.js')
 
-let config = {}
 let extract = () => {}
-const dir = 'index'
+const pkgName = 'hark-a-package'
+const pkgVersion = '1.0.0'
+const writeEnvScript = 'node -e \'const fs = require("fs"); fs.writeFileSync(process.cwd() + "/" + process.env.npm_lifecycle_event, process.env.npm_lifecycle_event);\''
+
 const main = requireInject('../../index.js', {
-  '../../lib/config': () => {
-    return config
-  },
   '../../lib/extract': {
     startWorkers () {},
     stopWorkers () {},
-    child (...args) {
-      return extract(...args)
+    child () {
+      return extract.apply(null, arguments)
     }
   }
 })
 
 test('throws error when no package.json is found', t => {
-  const prefix = fixtureHelper.write(dir, {
+  const prefix = fixtureHelper.write(pkgName, {
     'index.js': 'var a = 1;'
   })
 
@@ -34,8 +33,11 @@ test('throws error when no package.json is found', t => {
 })
 
 test('throws error when no package-lock nor shrinkwrap is found', t => {
-  const prefix = fixtureHelper.write(dir, {
-    'package.json': {}
+  const prefix = fixtureHelper.write(pkgName, {
+    'package.json': {
+      name: pkgName,
+      version: pkgVersion
+    }
   })
 
   main({ prefix: prefix }).catch(err => {
@@ -47,8 +49,11 @@ test('throws error when no package-lock nor shrinkwrap is found', t => {
 })
 
 test('throws error when old shrinkwrap is found', t => {
-  const prefix = fixtureHelper.write(dir, {
-    'package.json': {},
+  const prefix = fixtureHelper.write(pkgName, {
+    'package.json': {
+      name: pkgName,
+      version: pkgVersion
+    },
     'npm-shrinkwrap.json': {}
   })
 
@@ -61,8 +66,11 @@ test('throws error when old shrinkwrap is found', t => {
 })
 
 test('handles empty dependency list', t => {
-  const prefix = fixtureHelper.write(dir, {
-    'package.json': {},
+  const prefix = fixtureHelper.write(pkgName, {
+    'package.json': {
+      name: pkgName,
+      version: pkgVersion
+    },
     'package-lock.json': {
       dependencies: {},
       lockfileVersion: 1
@@ -78,8 +86,11 @@ test('handles empty dependency list', t => {
 })
 
 test('handles dependency list with only shallow subdeps', t => {
-  const prefix = fixtureHelper.write(dir, {
-    'package.json': {},
+  const prefix = fixtureHelper.write(pkgName, {
+    'package.json': {
+      name: pkgName,
+      version: pkgVersion
+    },
     'package-lock.json': {
       dependencies: {
         a: {}
@@ -90,9 +101,12 @@ test('handles dependency list with only shallow subdeps', t => {
 
   const aContents = 'var a = 1;'
 
-  extract = fixtureHelper.getWriter(dir, {
+  extract = fixtureHelper.getWriter(pkgName, {
     '/node_modules/a': {
-      'package.json': {},
+      'package.json': {
+        name: pkgName,
+        version: pkgVersion
+      },
       'index.js': aContents
     }
   })
@@ -107,8 +121,11 @@ test('handles dependency list with only shallow subdeps', t => {
 })
 
 test('handles dependency list with only deep subdeps', t => {
-  const prefix = fixtureHelper.write(dir, {
-    'package.json': {},
+  const prefix = fixtureHelper.write(pkgName, {
+    'package.json': {
+      name: pkgName,
+      version: pkgVersion
+    },
     'package-lock.json': {
       dependencies: {
         a: {
@@ -124,13 +141,19 @@ test('handles dependency list with only deep subdeps', t => {
   const aContents = 'var a = 1;'
   const bContents = 'var b = 2;'
 
-  extract = fixtureHelper.getWriter(dir, {
+  extract = fixtureHelper.getWriter(pkgName, {
     '/node_modules/a': {
-      'package.json': {},
+      'package.json': {
+        name: pkgName,
+        version: pkgVersion
+      },
       'index.js': aContents
     },
     '/node_modules/a/node_modules/b': {
-      'package.json': {},
+      'package.json': {
+        name: pkgName,
+        version: pkgVersion
+      },
       'index.js': bContents
     }
   })
@@ -144,3 +167,54 @@ test('handles dependency list with only deep subdeps', t => {
     t.end()
   })
 })
+
+test('runs lifecycle hooks of packages with env variables', t => {
+  const originalConsoleLog = console.log
+  console.log = () => {}
+
+  const prefix = fixtureHelper.write(pkgName, {
+    'package.json': {
+      name: pkgName,
+      version: pkgVersion,
+      scripts: {
+        preinstall: writeEnvScript,
+        install: writeEnvScript,
+        postinstall: writeEnvScript
+      }
+    },
+    'package-lock.json': {
+      dependencies: {
+        a: {}
+      },
+      lockfileVersion: 1
+    }
+  })
+
+  extract = fixtureHelper.getWriter(pkgName, {
+    '/node_modules/a': {
+      'package.json': {
+        name: 'a',
+        version: '1.0.0',
+        scripts: {
+          preinstall: writeEnvScript,
+          install: writeEnvScript,
+          postinstall: writeEnvScript
+        }
+      }
+    }
+  })
+
+  main({ prefix: prefix }).then(details => {
+    t.equal(details.count, 1)
+    t.ok(fixtureHelper.equals(prefix, 'preinstall', 'preinstall'))
+    t.ok(fixtureHelper.equals(prefix, 'install', 'install'))
+    t.ok(fixtureHelper.equals(prefix, 'postinstall', 'postinstall'))
+    t.ok(fixtureHelper.equals(prefix + '/node_modules/a', 'preinstall', 'preinstall'))
+    t.ok(fixtureHelper.equals(prefix + '/node_modules/a', 'install', 'install'))
+    t.ok(fixtureHelper.equals(prefix + '/node_modules/a', 'postinstall', 'postinstall'))
+
+    fixtureHelper.teardown()
+    console.log = originalConsoleLog
+    t.end()
+  })
+})

test/specs/lib/config.js

@@ -5,6 +5,8 @@ const requireInject = require('require-inject')
 const npmlog = require('npmlog')
 const childProcessFactory = require('../../lib/childProcessFactory.js')
 
+const dir = 'dir'
+
 let child
 const config = requireInject('../../../lib/config.js', {
   child_process: {
@@ -20,7 +22,7 @@ function cleanup () {
 test('config: errors if npm is not found', t => {
   cleanup()
 
-  config().catch(err => {
+  config(dir).catch(err => {
     t.equal(err.message, '`npm` command not found. Please ensure you have [email protected] or later installed.')
     t.end()
   })
@@ -31,7 +33,7 @@ test('config: errors if npm is not found', t => {
 test('config: errors if npm config ls --json cant output json', t => {
   cleanup()
 
-  config().catch(err => {
+  config(dir).catch(err => {
     t.equal(err.message, '`npm config ls --json` failed to output json. Please ensure you have [email protected] or later installed.')
     t.end()
   })
@@ -47,7 +49,7 @@ test('config: errors if npm errors for any reason', t => {
 
   const errorMessage = 'failed to reticulate splines'
 
-  config().catch(err => {
+  config(dir).catch(err => {
     t.equal(err, errorMessage)
     t.end()
   })
@@ -60,9 +62,11 @@ test('config: parses configs from npm', t => {
 
   const expectedConfig = { a: 1, b: 2 }
 
-  config().then(config => {
-    expectedConfig.log = npmlog
-    t.same(config, expectedConfig)
+  config(dir).then(config => {
+    t.same(config.config.a, expectedConfig.a)
+    t.same(config.config.b, expectedConfig.b)
+    t.same(config.dir, dir)
+    t.same(config.log, npmlog)
     t.end()
   })