From 1961c9369c92bf8fe530cecba9834ca3c7f5567c Mon Sep 17 00:00:00 2001
From: Martin Packman <martin@zegami.com>
Date: Wed, 6 Nov 2019 19:43:59 +0000
Subject: [PATCH] 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: https://github.com/npm/cli/pull/288
Credit: @bz2
Close: #288
Reviewed-by: @claudiahdz
---
 lib/install.js                |  3 +++
 lib/install/save.js           |  4 ++++
 test/common-tap.js            |  5 +++++
 test/tap/install-lifecycle.js | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+)
 create mode 100644 test/tap/install-lifecycle.js

diff --git a/lib/install.js b/lib/install.js
index 378ada7b05c06..ef492063b3a46 100644
--- a/lib/install.js
+++ b/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 {
diff --git a/lib/install/save.js b/lib/install/save.js
index 92b44a108099b..986233a516a7c 100644
--- a/lib/install/save.js
+++ b/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)
   }))
 }
 
diff --git a/test/common-tap.js b/test/common-tap.js
index 9e38c7690fa71..4ba92973ccfb2 100644
--- a/test/common-tap.js
+++ b/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, '\\$&')
+}
diff --git a/test/tap/install-lifecycle.js b/test/tap/install-lifecycle.js
new file mode 100644
index 0000000000000..c5b0fd35a020e
--- /dev/null
+++ b/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()
+  })
+})