fix(hooks): run .hooks scripts even if package.json script is not present (#13)

Fixes: npm/npm#19258

Author: mikesherov

index.js

@@ -16,6 +16,7 @@ const byline = require('byline')
 const resolveFrom = require('resolve-from')
 
 const DEFAULT_NODE_GYP_PATH = resolveFrom(__dirname, 'node-gyp/bin/node-gyp')
+const hookStatCache = new Map()
 
 let PATH = 'PATH'
 
@@ -33,6 +34,20 @@ function logid (pkg, stage) {
   return pkg._id + '~' + stage + ':'
 }
 
+function hookStat (dir, stage, cb) {
+  const hook = path.join(dir, '.hooks', stage)
+  const cachedStatError = hookStatCache.get(hook)
+
+  if (cachedStatError === undefined) {
+    return fs.stat(hook, function (statError) {
+      hookStatCache.set(hook, statError)
+      cb(statError)
+    })
+  }
+
+  return setImmediate(() => cb(cachedStatError))
+}
+
 function lifecycle (pkg, stage, wd, opts) {
   return new Promise((resolve, reject) => {
     while (pkg && pkg._data) pkg = pkg._data
@@ -46,32 +61,36 @@ function lifecycle (pkg, stage, wd, opts) {
       delete pkg.scripts.prepublish
     }
 
-    if (!pkg.scripts[stage]) return resolve()
-
-    validWd(wd || path.resolve(opts.dir, pkg.name), function (er, wd) {
-      if (er) return reject(er)
+    hookStat(opts.dir, stage, function (statError) {
+      // makeEnv is a slow operation. This guard clause prevents makeEnv being called
+      // and avoids a ton of unnecessary work, and results in a major perf boost.
+      if (!pkg.scripts[stage] && statError) return resolve()
 
-      if ((wd.indexOf(opts.dir) !== 0 || _incorrectWorkingDirectory(wd, pkg)) &&
-          !opts.unsafePerm && pkg.scripts[stage]) {
-        opts.log.warn('lifecycle', logid(pkg, stage), 'cannot run in wd', pkg._id, pkg.scripts[stage], `(wd=${wd})`)
-        return resolve()
-      }
-
-      // set the env variables, then run scripts as a child process.
-      var env = makeEnv(pkg, opts)
-      env.npm_lifecycle_event = stage
-      env.npm_node_execpath = env.NODE = env.NODE || process.execPath
-      env.npm_execpath = require.main.filename
-      env.INIT_CWD = process.cwd()
-      env.npm_config_node_gyp = env.npm_config_node_gyp || DEFAULT_NODE_GYP_PATH
+      validWd(wd || path.resolve(opts.dir, pkg.name), function (er, wd) {
+        if (er) return reject(er)
 
-      // 'nobody' typically doesn't have permission to write to /tmp
-      // even if it's never used, sh freaks out.
-      if (!opts.unsafePerm) env.TMPDIR = wd
+        if ((wd.indexOf(opts.dir) !== 0 || _incorrectWorkingDirectory(wd, pkg)) &&
+            !opts.unsafePerm && pkg.scripts[stage]) {
+          opts.log.warn('lifecycle', logid(pkg, stage), 'cannot run in wd', pkg._id, pkg.scripts[stage], `(wd=${wd})`)
+          return resolve()
+        }
 
-      lifecycle_(pkg, stage, wd, opts, env, (er) => {
-        if (er) return reject(er)
-        return resolve()
+        // set the env variables, then run scripts as a child process.
+        var env = makeEnv(pkg, opts)
+        env.npm_lifecycle_event = stage
+        env.npm_node_execpath = env.NODE = env.NODE || process.execPath
+        env.npm_execpath = require.main.filename
+        env.INIT_CWD = process.cwd()
+        env.npm_config_node_gyp = env.npm_config_node_gyp || DEFAULT_NODE_GYP_PATH
+
+        // 'nobody' typically doesn't have permission to write to /tmp
+        // even if it's never used, sh freaks out.
+        if (!opts.unsafePerm) env.TMPDIR = wd
+
+        lifecycle_(pkg, stage, wd, opts, env, (er) => {
+          if (er) return reject(er)
+          return resolve()
+        })
       })
     })
   })
@@ -131,8 +150,8 @@ function lifecycle_ (pkg, stage, wd, opts, env, cb) {
 
   chain(
     [
-      packageLifecycle && [runPackageLifecycle, pkg, env, wd, opts],
-      [runHookLifecycle, pkg, env, wd, opts]
+      packageLifecycle && [runPackageLifecycle, pkg, stage, env, wd, opts],
+      [runHookLifecycle, pkg, stage, env, wd, opts]
     ],
     done
   )
@@ -185,9 +204,8 @@ function validWd (d, cb) {
   })
 }
 
-function runPackageLifecycle (pkg, env, wd, opts, cb) {
+function runPackageLifecycle (pkg, stage, env, wd, opts, cb) {
   // run package lifecycle scripts in the package root, or the nearest parent.
-  var stage = env.npm_lifecycle_event
   var cmd = env.npm_lifecycle_script
 
   var note = '\n> ' + pkg._id + ' ' + stage + ' ' + wd +
@@ -329,17 +347,13 @@ function runCmd_ (cmd, pkg, env, wd, opts, stage, unsafe, uid, gid, cb_) {
   }
 }
 
-function runHookLifecycle (pkg, env, wd, opts, cb) {
-  // check for a hook script, run if present.
-  var stage = env.npm_lifecycle_event
-  var hook = path.join(opts.dir, '.hooks', stage)
-  var cmd = hook
-
-  fs.stat(hook, function (er) {
+function runHookLifecycle (pkg, stage, env, wd, opts, cb) {
+  hookStat(opts.dir, stage, function (er) {
     if (er) return cb()
+    var cmd = path.join(opts.dir, '.hooks', stage)
     var note = '\n> ' + pkg._id + ' ' + stage + ' ' + wd +
                '\n> ' + cmd
-    runCmd(note, hook, pkg, env, stage, wd, opts, cb)
+    runCmd(note, cmd, pkg, env, stage, wd, opts, cb)
   })
 }
 

test/fixtures/has-hooks/node_modules/.hooks/postinstall

@@ -0,0 +1,4 @@
+{
+  "name": "has-hooks",
+  "version": "1.0.0"
+}

test/fixtures/has-hooks/package.json

@@ -74,6 +74,48 @@ test('_incorrectWorkingDirectory: rejects wd from other packages', function (t)
   t.end()
 })
 
+test('runs scripts from .hooks directory even if no script is present in package.json', function (t) {
+  const fixture = path.join(__dirname, 'fixtures', 'has-hooks')
+
+  const verbose = sinon.spy()
+  const silly = sinon.spy()
+  const log = {
+    level: 'silent',
+    info: noop,
+    warn: noop,
+    silly,
+    verbose,
+    pause: noop,
+    resume: noop,
+    clearProgress: noop,
+    showProgress: noop
+  }
+  const dir = path.join(fixture, 'node_modules')
+
+  const pkg = require(path.join(fixture, 'package.json'))
+
+  lifecycle(pkg, 'postinstall', fixture, {
+    stdio: 'pipe',
+    log,
+    dir,
+    config: {}
+  })
+  .then(() => {
+    t.ok(
+      verbose.calledWithMatch(
+        'lifecycle',
+        'undefined~postinstall:',
+        'stdout',
+        'ran hook'
+      ),
+      'ran postinstall hook'
+    )
+
+    t.end()
+  })
+  .catch(t.end)
+})
+
 test("reports child's output", function (t) {
   const fixture = path.join(__dirname, 'fixtures', 'count-to-10')