Skip to content

Commit

Permalink
validate-tree: Warn on deps in both dependencies and devDependencies
Browse files Browse the repository at this point in the history
Fixes: npm#6725
Credit: @TedYav
PR-URL: npm/npm#15772
Reviewed-By: @iarna
  • Loading branch information
TedYav authored and iarna committed Feb 23, 2017
1 parent 148ee66 commit 53412eb
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 1 deletion.
1 change: 1 addition & 0 deletions .mailmap
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Ryan Emery <[email protected]>
Sam Mikes <[email protected]>
Stephanie Snopek <[email protected]>
Takaya Kobayashi <[email protected]>
Ted Yavuzkurt <[email protected]> <[email protected]>
Thomas Reggi <[email protected]>
Timo Weiß <[email protected]>
Tony <[email protected]>
Expand Down
20 changes: 19 additions & 1 deletion lib/install/validate-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ module.exports = function (idealTree, log, next) {
], done)
}],
[thenValidateAllPeerDeps, idealTree],
[thenCheckTop, idealTree]
[thenCheckTop, idealTree],
[thenCheckDuplicateDeps, idealTree]
], andFinishTracker(log, next))
}

Expand Down Expand Up @@ -73,5 +74,22 @@ function thenCheckTop (idealTree, next) {
warnObj.code = 'ENODEPRE'
idealTree.warnings.push(warnObj)
}

next()
}

// check for deps duplciated between devdeps and regular deps
function thenCheckDuplicateDeps (idealTree, next) {
var deps = idealTree.package.dependencies || {}
var devDeps = idealTree.package.devDependencies || {}

for (var pkg in devDeps) {
if (pkg in deps) {
var warnObj = new Error('The package ' + pkg + ' is included as both a dev and production dependency.')
warnObj.code = 'EDUPLICATEDEP'
idealTree.warnings.push(warnObj)
}
}

next()
}
69 changes: 69 additions & 0 deletions test/tap/install-duplicate-deps-warning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
var fs = require('graceful-fs')
var path = require('path')

var mkdirp = require('mkdirp')
var mr = require('npm-registry-mock')
var osenv = require('osenv')
var rimraf = require('rimraf')
var test = require('tap').test

var common = require('../common-tap.js')
var npm = npm = require('../../')

var pkg = path.resolve(__dirname, path.basename(__filename, '.js'))

var json = {
dependencies: {
underscore: '1.5.1'
},
devDependencies: {
underscore: '1.3.1'
}
}

test('setup', function (t) {
t.comment('test for https://github.com/npm/npm/issues/6725')
cleanup()
mkdirp.sync(pkg)
fs.writeFileSync(
path.join(pkg, 'package.json'),
JSON.stringify(json, null, 2)
)
process.chdir(pkg)
console.dir(pkg)
t.end()
})

test('npm install with duplicate dependencies, different versions', function (t) {
t.plan(1)
mr({ port: common.port }, function (er, s) {
var opts = {
cache: path.resolve(pkg, 'cache'),
registry: common.registry
}

npm.load(opts, function (err) {
if (err) return t.fail(err)

npm.install('.', function (err, additions, result) {
if (err) return t.fail(err)

var invalid = result.warnings.filter(function (warning) { return warning.code === 'EDUPLICATEDEP' })
t.is(invalid.length, 1, 'got a warning (EDUPLICATEDEP) for duplicate dev/production dependencies')

s.close()
t.end()
})
})
})
})

test('cleanup', function (t) {
cleanup()
t.end()
})

function cleanup () {
process.chdir(osenv.tmpdir())
rimraf.sync(pkg)
}

0 comments on commit 53412eb

Please sign in to comment.