fix(add): properly save new deps where they go

[wraithgar]

This consolidates and standardizes the logic around where in the package.json we save added packages. We explicitly obey the saveType if it's given, properly infer it now if it isn't, and enforce exclusivity between things like dependencies and devDependencies

We also now log a warning when we've ignored one for the other.

References

Closes: npm/cli#2647
Related to: npm/cli#3096
Related to: npm/cli#2647

[isaacs]

Did you mean to include an empty env file? https://github.com/npm/arborist/pull/273/files#diff-b77349bf1fa9b26d45d545d7d64dad96737e9cfbb3cbb5f5d10fb77b0fcc44ad

[isaacs]

Everything here looks good, but I'd recommend adding this:

diff --git a/tap-snapshots/test/arborist/reify.js.test.cjs b/tap-snapshots/test/arborist/reify.js.test.cjs
index 513c7485..8a69c358 100644
--- a/tap-snapshots/test/arborist/reify.js.test.cjs
+++ b/tap-snapshots/test/arborist/reify.js.test.cjs
@@ -14982,6 +14982,10 @@ exports[`test/arborist/reify.js TAP no saveType: peer only > must match snapshot
 {"peerDependencies":{"abbrev":"^1.1.1"}}
 `
 
+exports[`test/arborist/reify.js TAP no saveType: prod w/ compatible peer, saves prod only > must match snapshot 1`] = `
+{"dependencies":{"abbrev":"^1.1.1"}}
+`
+
 exports[`test/arborist/reify.js TAP omit peer deps > finished timers 1`] = `
 Array [
   "arborist:ctor",
diff --git a/test/arborist/reify.js b/test/arborist/reify.js
index b8323a8b..e1eb5361 100644
--- a/test/arborist/reify.js
+++ b/test/arborist/reify.js
@@ -1752,17 +1752,17 @@ t.test('no saveType: dev w/ incompatible optional', async t => {
   t.matchSnapshot(fs.readFileSync(path + '/package.json', 'utf8'))
 })
 
-// t.test('no saveType: prod w/ compatible peer', async t => {
-//   const path = t.testdir({
-//     'package.json': JSON.stringify({
-//       peerDependencies: { abbrev: '*' },
-//       dependencies: { abbrev: '*' },
-//     }),
-//   })
-//   const arb = newArb({ path })
-//   await arb.reify({ add: ['abbrev'] })
-//   t.matchSnapshot(fs.readFileSync(path + '/package.json', 'utf8'))
-// })
+t.test('no saveType: prod w/ compatible peer, saves prod only', async t => {
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      peerDependencies: { abbrev: '*' },
+      dependencies: { abbrev: '*' },
+    }),
+  })
+  const arb = newArb({ path })
+  await arb.reify({ add: ['abbrev'] })
+  t.matchSnapshot(fs.readFileSync(path + '/package.json', 'utf8'))
+})
 
 t.test('no saveType: peer only', async t => {
   const path = t.testdir({

Ie, uncomment the test, and name it based on what is happening, so if it changes we notice. Or just delete the test if it's not necessary.