save: If a shrinkwrap already has dev deps, make sure they're kept

When running `install --save` in a folder with a shrinkwrap that has
dev-only dependencies in it, it's important that we don't then throw
away the dev dependencies just because they didn't say
`install --save --also=dev`.

PR-URL: #9992
Fixes: #9647

lib/install/save.js

@@ -46,9 +46,28 @@ function saveShrinkwrap (tree, next) {
     var save = npm.config.get('save')
     var saveDev = npm.config.get('save-dev')
     var saveOptional = npm.config.get('save-optional')
-    if (!saveOptional && saveDev) return next()
+
+    var shrinkwrap = tree.package._shrinkwrap || {dependencies: {}}
+    var hasDevOnlyDeps = tree.requires.filter(function (dep) {
+      var devReqs = dep.package._requiredBy.filter(function (name) { return name.substr(0, 4) === '#DEV' })
+      return devReqs.length === dep.package._requiredBy.length
+    }).some(function (dep) {
+      return shrinkwrap.dependencies[dep.package.name] != null
+    })
+
+    if (!saveOptional && saveDev && !hasDevOnlyDeps) return next()
     if (saveOptional || !save) return next()
-    npm.commands.shrinkwrap([], true, next)
+
+    if (hasDevOnlyDeps) {
+      var dev = npm.config.get('dev')
+      npm.config.set('dev', true)
+      npm.commands.shrinkwrap([], true, function () {
+        npm.config.set('dev', dev)
+        next.apply(this, arguments)
+      })
+    } else {
+      npm.commands.shrinkwrap([], true, next)
+    }
   })
 }
 

test/tap/shrinkwrap-save-with-existing-dev-deps.js

@@ -0,0 +1,87 @@
+var fs = require('fs')
+var path = require('path')
+
+var mkdirp = require('mkdirp')
+var osenv = require('osenv')
+var rimraf = require('rimraf')
+var test = require('tap').test
+
+var common = require('../common-tap.js')
+
+var base = path.resolve(__dirname, path.basename(__filename, '.js'))
+var installme = path.join(base, 'installme')
+var installme_pkg = path.join(installme, 'package.json')
+var example = path.join(base, 'example')
+var example_shrinkwrap = path.join(example, 'npm-shrinkwrap.json')
+var example_pkg = path.join(example, 'package.json')
+var installed = path.join(example, 'node_modules', 'installed')
+var installed_pkg = path.join(installed, 'package.json')
+
+var EXEC_OPTS = { cwd: example }
+
+var installme_pkg_json = {
+  name: 'installme',
+  version: '1.0.0',
+  dependencies: {}
+}
+
+var example_pkg_json = {
+  name: 'example',
+  version: '1.0.0',
+  dependencies: {},
+  devDependencies: {
+    'installed': '1.0'
+  }
+}
+
+var example_shrinkwrap_json = {
+  name: 'example',
+  version: '1.0.0',
+  dependencies: {
+    installed: {
+      version: '1.0.0'
+    }
+  }
+}
+
+var installed_pkg_json = {
+  _id: 'installed@1.0.0',
+  name: 'installed',
+  version: '1.0.0'
+}
+
+function writeJson (filename, obj) {
+  mkdirp.sync(path.dirname(filename))
+  fs.writeFileSync(filename, JSON.stringify(obj, null, 2))
+}
+
+test('setup', function (t) {
+  cleanup()
+  writeJson(installme_pkg, installme_pkg_json)
+  writeJson(example_pkg, example_pkg_json)
+  writeJson(example_shrinkwrap, example_shrinkwrap_json)
+  writeJson(installed_pkg, installed_pkg_json)
+  t.end()
+})
+
+test('install --save leaves dev deps alone', function (t) {
+  common.npm(['install', '--save', 'file://' + installme], EXEC_OPTS, function (er, code, stdout, stderr) {
+    t.ifError(er, "spawn didn't catch fire")
+    t.is(code, 0, 'install completed ok')
+    t.is(stderr, '', 'install completed without error output')
+    var shrinkwrap = JSON.parse(fs.readFileSync(example_shrinkwrap))
+    t.ok(shrinkwrap.dependencies.installed, "save new install didn't remove dev dep")
+    t.ok(shrinkwrap.dependencies.installme, 'save new install DID add new dep')
+    t.end()
+  })
+})
+
+test('cleanup', function (t) {
+  cleanup()
+  t.end()
+})
+
+function cleanup () {
+  process.chdir(osenv.tmpdir())
+  rimraf.sync(base)
+}