Fix package id in shrinkwrap lifecycle step output

Currently all logging related to shrinkwrap steps reports 'undefined'
for the package in output and log messages.

This is due to the package associated with the `idealTree` being
recreated in the `savePackageJson()` method which precedes these
steps. For now, just copy forward the `_id` attribute which lifecycle
logging expects, but note that mutating `package` here is surprising.

Fixes npm/npm#20756

PR-URL: #288
Credit: @bz2
Close: #288
Reviewed-by: @claudiahdz

lib/install.js

@@ -650,6 +650,9 @@ Installer.prototype.saveToDependencies = function (cb) {
   validate('F', arguments)
   if (this.failing) return cb()
   log.silly('install', 'saveToDependencies')
+  // Note idealTree will be mutated during the save operations below as the
+  // package is reloaded from disk to preserve additional details. This means
+  // steps after postInstall will see a slightly different package object.
   if (this.saveOnlyLock) {
     saveShrinkwrap(this.idealTree, cb)
   } else {

lib/install/save.js

@@ -8,6 +8,7 @@ const iferr = require('iferr')
 const log = require('npmlog')
 const moduleName = require('../utils/module-name.js')
 const npm = require('../npm.js')
+const packageId = require('../utils/package-id.js')
 const parseJSON = require('../utils/parse-json.js')
 const path = require('path')
 const stringifyPackage = require('stringify-package')
@@ -131,6 +132,9 @@ function savePackageJson (tree, next) {
     } else {
       writeFileAtomic(saveTarget, json, next)
     }
+
+    // Restore derived id as it was removed when reloading from disk
+    tree.package._id = packageId(tree.package)
   }))
 }
 

test/common-tap.js

@@ -283,3 +283,8 @@ Environment.prototype.extend = function (env) {
   }
   return self
 }
+
+var reEscape = /[\\[\](){}*?+.^$|]/g
+exports.escapeForRe = function (string) {
+  return string.replace(reEscape, '\\$&')
+}

test/tap/install-lifecycle.js

@@ -0,0 +1,35 @@
+var fs = require('graceful-fs')
+var path = require('path')
+var test = require('tap').test
+var common = require('../common-tap.js')
+var pkg = common.pkg
+
+test('npm install execution order', function (t) {
+  const packageJson = {
+    name: 'life-test',
+    version: '0.0.1',
+    description: 'Test for npm install execution order',
+    scripts: {
+      install: 'true',
+      preinstall: 'true',
+      preshrinkwrap: 'true',
+      postinstall: 'true',
+      postshrinkwrap: 'true',
+      shrinkwrap: 'true'
+    }
+  }
+  fs.writeFileSync(path.resolve(pkg, 'package.json'), JSON.stringify(packageJson), 'utf8')
+  common.npm(['install', '--loglevel=error'], { cwd: pkg }, function (err, code, stdout, stderr) {
+    if (err) throw err
+
+    t.comment(stdout)
+    t.comment(stderr)
+
+    const steps = ['preinstall', 'install', 'postinstall', 'preshrinkwrap', 'shrinkwrap', 'postshrinkwrap']
+    const expectedLines = steps.map(function (step) {
+      return '> ' + packageJson.name + '@' + packageJson.version + ' ' + step
+    })
+    t.match(stdout, new RegExp(expectedLines.map(common.escapeForRe).join('(.|\n)*')))
+    t.end()
+  })
+})