shrinkwrap: save-dev updates shrinkwrap file

Running 'npm install --save-dev' will update shrinkwrap file, but only
if there already are any dev dependencies in this file.

Fixes: #11735
PR-URL: https://github.com/npm/npm/pull/13860
Credit: @szimek
Reviewed-By: @iarna

4 files changed, 195 insertions, 6 deletions

diff --git a/doc/cli/npm-shrinkwrap.md b/doc/cli/npm-shrinkwrap.md
index 6fc7d96f7..f89c3b837 100644
--- a/doc/cli/npm-shrinkwrap.md
+++ b/doc/cli/npm-shrinkwrap.md
@@ -137,11 +137,11 @@ To add or update a dependency in a shrinkwrapped package:
 
 1. Run `npm install` in the package root to install the current
    versions of all dependencies.
-2. Add or update dependencies. `npm install --save` each new or updated
-   package individually to update the `package.json` and the shrinkwrap.
-   Note that they must be explicitly named in order to be installed: running
-   `npm install` with no arguments will merely reproduce the existing
-   shrinkwrap.
+2. Add or update dependencies. `npm install --save` or `npm install --save-dev`
+   each new or updated package individually to update the `package.json` and
+   the shrinkwrap. Note that they must be explicitly named in order to be
+   installed: running `npm install` with no arguments will merely reproduce
+   the existing shrinkwrap.
 3. Validate that the package works as expected with the new
    dependencies.
 4. Commit the new `npm-shrinkwrap.json`, and publish your package.
diff --git a/lib/install/save.js b/lib/install/save.js
index 18e04f6de..d5c97bfaa 100644
--- a/lib/install/save.js
+++ b/lib/install/save.js
@@ -57,7 +57,7 @@ function saveShrinkwrap (tree, next) {
     })
 
     if (!saveOptional && saveDev && !shrinkwrapHasAnyDevOnlyDeps) return next()
-    if (saveOptional || !save) return next()
+    if (saveOptional || !(save || saveDev)) return next()
 
     var silent = false
     createShrinkwrap(tree.path, tree.package, shrinkwrapHasAnyDevOnlyDeps, silent, next)
diff --git a/test/tap/shrinkwrap-save-dev-with-existing-deps.js b/test/tap/shrinkwrap-save-dev-with-existing-deps.js
new file mode 100644
index 000000000..eacea9e3c
--- /dev/null
+++ b/test/tap/shrinkwrap-save-dev-with-existing-deps.js
@@ -0,0 +1,102 @@
+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_prod = path.join(example, 'node_modules', 'installed-prod')
+var installed_prod_pkg = path.join(installed_prod, 'package.json')
+var installed_dev = path.join(example, 'node_modules', 'installed-dev')
+var installed_dev_pkg = path.join(installed_dev, '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: {
+    'installed-prod': '1.0'
+  },
+  devDependencies: {
+    'installed-dev': '1.0'
+  }
+}
+
+var example_shrinkwrap_json = {
+  name: 'example',
+  version: '1.0.0',
+  dependencies: {
+    'installed-prod': {
+      version: '1.0.0'
+    },
+    'installed-dev': {
+      version: '1.0.0'
+    }
+  }
+}
+
+var installed_prod_pkg_json = {
+  _id: 'installed-prod@1.0.0',
+  name: 'installed-prod',
+  version: '1.0.0'
+}
+
+var installed_dev_pkg_json = {
+  _id: 'installed-dev@1.0.0',
+  name: 'installed-dev',
+  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_prod_pkg, installed_prod_pkg_json)
+  writeJson(installed_dev_pkg, installed_dev_pkg_json)
+  t.end()
+})
+
+test('install --save-dev leaves prod deps alone', function (t) {
+  common.npm(['install', '--save-dev', '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-prod'], "save-dev new install didn't remove prod dep")
+    t.ok(shrinkwrap.dependencies['installed-dev'], "save-dev new install didn't remove dev dep")
+    t.ok(shrinkwrap.dependencies.installme, 'save-dev new install DID add new dev dep')
+    t.end()
+  })
+})
+
+test('cleanup', function (t) {
+  cleanup()
+  t.end()
+})
+
+function cleanup () {
+  process.chdir(osenv.tmpdir())
+  rimraf.sync(base)
+}
diff --git a/test/tap/shrinkwrap-save-dev-without-existing-dev-deps.js b/test/tap/shrinkwrap-save-dev-without-existing-dev-deps.js
new file mode 100644
index 000000000..436b17895
--- /dev/null
+++ b/test/tap/shrinkwrap-save-dev-without-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: {
+    'installed': '1.0'
+  },
+  devDependencies: {}
+}
+
+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-dev leaves dev deps alone', function (t) {
+  common.npm(['install', '--save-dev', '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-dev new install didn't remove dep")
+    t.notOk(shrinkwrap.dependencies.installme, 'save-dev new install DID NOT add new dev dep')
+    t.end()
+  })
+})
+
+test('cleanup', function (t) {
+  cleanup()
+  t.end()
+})
+
+function cleanup () {
+  process.chdir(osenv.tmpdir())
+  rimraf.sync(base)
+}