Skip to content

Commit 1961c93

Browse files
bz2claudiahdz
authored andcommitted
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
1 parent 36e6c01 commit 1961c93

File tree

4 files changed

+47
-0
lines changed

4 files changed

+47
-0
lines changed

lib/install.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,9 @@ Installer.prototype.saveToDependencies = function (cb) {
650650
validate('F', arguments)
651651
if (this.failing) return cb()
652652
log.silly('install', 'saveToDependencies')
653+
// Note idealTree will be mutated during the save operations below as the
654+
// package is reloaded from disk to preserve additional details. This means
655+
// steps after postInstall will see a slightly different package object.
653656
if (this.saveOnlyLock) {
654657
saveShrinkwrap(this.idealTree, cb)
655658
} else {

lib/install/save.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const iferr = require('iferr')
88
const log = require('npmlog')
99
const moduleName = require('../utils/module-name.js')
1010
const npm = require('../npm.js')
11+
const packageId = require('../utils/package-id.js')
1112
const parseJSON = require('../utils/parse-json.js')
1213
const path = require('path')
1314
const stringifyPackage = require('stringify-package')
@@ -131,6 +132,9 @@ function savePackageJson (tree, next) {
131132
} else {
132133
writeFileAtomic(saveTarget, json, next)
133134
}
135+
136+
// Restore derived id as it was removed when reloading from disk
137+
tree.package._id = packageId(tree.package)
134138
}))
135139
}
136140

test/common-tap.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,8 @@ Environment.prototype.extend = function (env) {
283283
}
284284
return self
285285
}
286+
287+
var reEscape = /[\\[\](){}*?+.^$|]/g
288+
exports.escapeForRe = function (string) {
289+
return string.replace(reEscape, '\\$&')
290+
}

test/tap/install-lifecycle.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
var fs = require('graceful-fs')
2+
var path = require('path')
3+
var test = require('tap').test
4+
var common = require('../common-tap.js')
5+
var pkg = common.pkg
6+
7+
test('npm install execution order', function (t) {
8+
const packageJson = {
9+
name: 'life-test',
10+
version: '0.0.1',
11+
description: 'Test for npm install execution order',
12+
scripts: {
13+
install: 'true',
14+
preinstall: 'true',
15+
preshrinkwrap: 'true',
16+
postinstall: 'true',
17+
postshrinkwrap: 'true',
18+
shrinkwrap: 'true'
19+
}
20+
}
21+
fs.writeFileSync(path.resolve(pkg, 'package.json'), JSON.stringify(packageJson), 'utf8')
22+
common.npm(['install', '--loglevel=error'], { cwd: pkg }, function (err, code, stdout, stderr) {
23+
if (err) throw err
24+
25+
t.comment(stdout)
26+
t.comment(stderr)
27+
28+
const steps = ['preinstall', 'install', 'postinstall', 'preshrinkwrap', 'shrinkwrap', 'postshrinkwrap']
29+
const expectedLines = steps.map(function (step) {
30+
return '> ' + packageJson.name + '@' + packageJson.version + ' ' + step
31+
})
32+
t.match(stdout, new RegExp(expectedLines.map(common.escapeForRe).join('(.|\n)*')))
33+
t.end()
34+
})
35+
})

0 commit comments

Comments
 (0)