Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

When replacing an existing module with a bundled one, remove it first #10192

Closed
wants to merge 1 commit into from
Closed

When replacing an existing module with a bundled one, remove it first #10192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/install/diff-trees.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ function diffTrees (oldTree, newTree) {
requiredByAllLinked(pkg)
if (pkg.fromBundle) {
if (npm.config.get('rebuild-bundle')) differences.push(['rebuild', pkg])
if (pkg.oldPkg) differences.push(['remove', pkg])
} else if (pkg.oldPkg) {
if (!pkg.directlyRequested && pkgAreEquiv(pkg.oldPkg.package, pkg.package)) return
if (!pkg.isInLink && (isLink(pkg.oldPkg) || isLink(pkg))) {
Expand Down
75 changes: 47 additions & 28 deletions test/tap/override-bundled.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,31 @@ var testmod = path.resolve(testdir, 'top-test')
var testmodjson = {
name: 'top-test',
version: '1.0.0',
dependencies: {'bundle-test': 'file:bundle-test/'},
bundledDependencies: ['bundle-test']
dependencies: {
'bundle-update': 'file:bundle-update/',
'bundle-keep': 'file:bundle-keep/'
},
bundledDependencies: ['bundle-update', 'bundle-keep']
}

var bundlenew = path.resolve(testmod, 'bundle-test')
var newsource = path.resolve(bundlenew, 'NEW')
var newdest = path.resolve(testdir, 'node_modules', 'top-test', 'node_modules', 'bundle-test', 'NEW')
var bundlebad = path.resolve(testmod, 'node_modules', 'bundle-test')
var bundleupdatesrc = path.resolve(testmod, 'bundle-update')
var bundleupdateNEW = path.resolve(bundleupdatesrc, 'NEW')
var bundleupdateNEWpostinstall = path.resolve(testdir, 'node_modules', 'top-test', 'node_modules', 'bundle-update', 'NEW')
var bundleupdatebad = path.resolve(testmod, 'node_modules', 'bundle-update')
var bundlekeepsrc = path.resolve(testmod, 'bundle-keep')
var bundlekeep = path.resolve(testmod, 'node_modules', 'bundle-keep')
var bundlekeepOLD = path.resolve(bundlekeep, 'OLD')
var bundlekeepOLDpostinstall = path.resolve(testdir, 'node_modules', 'top-test', 'node_modules', 'bundle-keep', 'OLD')
var bundlejson = {
name: 'bundle-test',
name: 'bundle-update',
version: '1.0.0'
}
var bundlekeepjson = {
name: 'bundle-keep',
_requested: {
rawSpec: 'file:bundle-keep/'
}
}

function writepjs (dir, content) {
fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify(content, null, 2))
Expand All @@ -37,11 +50,16 @@ function setup () {
writepjs(testdir, testjson)
mkdirp.sync(testmod)
writepjs(testmod, testmodjson)
mkdirp.sync(bundlenew)
writepjs(bundlenew, bundlejson)
fs.writeFileSync(newsource, '')
mkdirp.sync(bundlebad)
writepjs(bundlebad, bundlejson)
mkdirp.sync(bundleupdatesrc)
writepjs(bundleupdatesrc, bundlejson)
fs.writeFileSync(bundleupdateNEW, '')
mkdirp.sync(bundleupdatebad)
writepjs(bundleupdatebad, bundlejson)
mkdirp.sync(bundlekeepsrc)
writepjs(bundlekeepsrc, bundlekeepjson)
mkdirp.sync(bundlekeep)
writepjs(bundlekeep, bundlekeepjson)
fs.writeFileSync(bundlekeepOLD, '')
}

function cleanup () {
Expand All @@ -54,29 +72,30 @@ test('setup', function (t) {
t.end()
})

/*
finalize: Make finalize ok w/ package.json resulting in bundled dep overrides

PR-URL: https://github.com/npm/npm/pull/10147

*/
test('bundled', function (t) {
// This tests that after the install we have a freshly installed version
// of `bundle-test` (in alignment with the package.json), instead of the
// version that was bundled with `top-test`.
// If npm doesn't do this, and selects the bundled version, things go very
// wrong because npm thinks it has a different module (with different
// metadata) installed in that location and will go off and try to do
// _things_ to it. Things like chmod in particular, which in turn results
// in the dreaded ENOENT errors.
common.npm(['install', '--loglevel=warn'], {cwd: testdir}, function (err, code, stdout, stderr) {
if (err) throw err
t.plan(5)
t.is(code, 0, 'npm itself completed ok')

// This tests that after the install we have a freshly installed version
// of `bundle-update` (in alignment with the package.json), instead of the
// version that was bundled with `top-test`.
// If npm doesn't do this, and selects the bundled version, things go very
// wrong because npm thinks it has a different module (with different
// metadata) installed in that location and will go off and try to do
// _things_ to it. Things like chmod in particular, which in turn results
// in the dreaded ENOENT errors.
t.like(stderr, /EPACKAGEJSON override-bundled/, "didn't stomp on other warnings")
t.like(stderr, /EBUNDLEOVERRIDE/, 'included warning about bundled dep')
fs.stat(newdest, function (missing) {
fs.stat(bundleupdateNEWpostinstall, function (missing) {
t.ok(!missing, 'package.json overrode bundle')
})

// Relatedly, when upgrading, if a bundled module is replacing an existing
// module we want to choose the bundled version, not the version we're replacing.
fs.stat(bundlekeepOLDpostinstall, function (missing) {
t.ok(!missing, 'package.json overrode bundle')
t.end()
})
})
})
Expand Down