config: Restore --dev flag, unify --omit flatteners

This consolidates all the various --include and --omit configuration
flatteners into a single function, since they have to be interdependent
in order to function properly.

Fix: #2936

PR-URL: https://github.com/npm/cli/pull/2942
Credit: @isaacs
Close: #2942
Reviewed-by: @wraithgar

5 files changed, 115 insertions, 45 deletions

diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md
index da69ad463..6bbe58d91 100644
--- a/docs/content/using-npm/config.md
+++ b/docs/content/using-npm/config.md
@@ -1382,6 +1382,14 @@ What authentication strategy to use with `adduser`/`login`.
 
 `--cache-min=9999 (or bigger)` is an alias for `--prefer-offline`.
 
+#### `dev`
+
+* Default: false
+* Type: Boolean
+* DEPRECATED: Please use --include=dev instead.
+
+Alias for `--include=dev`.
+
 #### `init.author.email`
 
 * Default: ""
diff --git a/lib/utils/config/definitions.js b/lib/utils/config/definitions.js
index 5001efb46..b8e021c52 100644
--- a/lib/utils/config/definitions.js
+++ b/lib/utils/config/definitions.js
@@ -22,6 +22,34 @@ const maybeReadFile = file => {
   }
 }
 
+const buildOmitList = obj => {
+  const include = obj.include || []
+  const omit = obj.omit || []
+
+  const only = obj.only
+  if (/^prod(uction)?$/.test(only) || obj.production)
+    omit.push('dev')
+
+  if (/^dev/.test(obj.also))
+    include.push('dev')
+
+  if (obj.dev)
+    include.push('dev')
+
+  if (obj.optional === false)
+    omit.push('optional')
+  else if (obj.optional === true)
+    include.push('optional')
+
+  obj.omit = [...new Set(omit)].filter(type => !include.includes(type))
+  obj.include = [...new Set(include)]
+
+  if (obj.omit.includes('dev'))
+    process.env.NODE_ENV = 'production'
+
+  return obj.omit
+}
+
 const editor = process.env.EDITOR ||
   process.env.VISUAL ||
   (isWindows ? 'notepad.exe' : 'vi')
@@ -164,12 +192,6 @@ define('also', {
   `,
   deprecated: 'Please use --include=dev instead.',
   flatten (key, obj, flatOptions) {
-    if (!/^dev(elopment)?$/.test(obj.also))
-      return
-
-    // add to include, and call the omit flattener
-    obj.include = obj.include || []
-    obj.include.push('dev')
     definitions.omit.flatten('omit', obj, flatOptions)
   },
 })
@@ -477,6 +499,18 @@ define('description', {
   },
 })
 
+define('dev', {
+  default: false,
+  type: Boolean,
+  description: `
+    Alias for \`--include=dev\`.
+  `,
+  deprecated: 'Please use --include=dev instead.',
+  flatten (key, obj, flatOptions) {
+    definitions.omit.flatten('omit', obj, flatOptions)
+  },
+})
+
 define('diff', {
   default: [],
   type: [String, Array],
@@ -1218,10 +1252,7 @@ define('omit', {
     scripts.
   `,
   flatten (key, obj, flatOptions) {
-    const include = obj.include || []
-    const omit = flatOptions.omit || []
-    flatOptions.omit = omit.concat(obj[key])
-      .filter(type => type && !include.includes(type))
+    flatOptions.omit = buildOmitList(obj)
   },
 })
 
@@ -1236,12 +1267,6 @@ define('only', {
     \`--omit=dev\`.
   `,
   flatten (key, obj, flatOptions) {
-    const value = obj[key]
-    if (!/^prod(uction)?$/.test(value))
-      return
-
-    obj.omit = obj.omit || []
-    obj.omit.push('dev')
     definitions.omit.flatten('omit', obj, flatOptions)
   },
 })
@@ -1259,16 +1284,6 @@ define('optional', {
     Alias for --include=optional or --omit=optional
   `,
   flatten (key, obj, flatOptions) {
-    const value = obj[key]
-    if (value === null)
-      return
-    else if (value === true) {
-      obj.include = obj.include || []
-      obj.include.push('optional')
-    } else {
-      obj.omit = obj.omit || []
-      obj.omit.push('optional')
-    }
     definitions.omit.flatten('omit', obj, flatOptions)
   },
 })
@@ -1385,12 +1400,6 @@ define('production', {
   deprecated: 'Use `--omit=dev` instead.',
   description: 'Alias for `--omit=dev`',
   flatten (key, obj, flatOptions) {
-    const value = obj[key]
-    if (!value)
-      return
-
-    obj.omit = obj.omit || []
-    obj.omit.push('dev')
     definitions.omit.flatten('omit', obj, flatOptions)
   },
 })
diff --git a/tap-snapshots/test-lib-utils-config-definitions.js-TAP.test.js b/tap-snapshots/test-lib-utils-config-definitions.js-TAP.test.js
index 2ed810da8..b3d920a0c 100644
--- a/tap-snapshots/test-lib-utils-config-definitions.js-TAP.test.js
+++ b/tap-snapshots/test-lib-utils-config-definitions.js-TAP.test.js
@@ -32,6 +32,7 @@ Array [
   "commit-hooks",
   "depth",
   "description",
+  "dev",
   "diff",
   "diff-ignore-all-space",
   "diff-name-only",
diff --git a/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js b/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js
index 8323e793e..8af8c1edd 100644
--- a/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js
+++ b/tap-snapshots/test-lib-utils-config-describe-all.js-TAP.test.js
@@ -1261,6 +1261,14 @@ What authentication strategy to use with \`adduser\`/\`login\`.
 
 \`--cache-min=9999 (or bigger)\` is an alias for \`--prefer-offline\`.
 
+#### \`dev\`
+
+* Default: false
+* Type: Boolean
+* DEPRECATED: Please use --include=dev instead.
+
+Alias for \`--include=dev\`.
+
 #### \`init.author.email\`
 
 * Default: ""
diff --git a/test/lib/utils/config/definitions.js b/test/lib/utils/config/definitions.js
index 830468c43..75819dcbc 100644
--- a/test/lib/utils/config/definitions.js
+++ b/test/lib/utils/config/definitions.js
@@ -194,18 +194,26 @@ t.test('flatteners that populate flat.omit array', t => {
     // ignored if setting is not dev or development
     obj.also = 'ignored'
     definitions.also.flatten('also', obj, flat)
-    t.strictSame(obj, {also: 'ignored'}, 'nothing done')
-    t.strictSame(flat, {}, 'nothing done')
+    t.strictSame(obj, {also: 'ignored', omit: [], include: []}, 'nothing done')
+    t.strictSame(flat, {omit: []}, 'nothing done')
 
     obj.also = 'development'
     definitions.also.flatten('also', obj, flat)
-    t.strictSame(obj, { also: 'development', include: ['dev'] }, 'marked dev as included')
+    t.strictSame(obj, {
+      also: 'development',
+      omit: [],
+      include: ['dev'],
+    }, 'marked dev as included')
     t.strictSame(flat, { omit: [] }, 'nothing omitted, so nothing changed')
 
     obj.omit = ['dev', 'optional']
     obj.include = []
     definitions.also.flatten('also', obj, flat)
-    t.strictSame(obj, { also: 'development', omit: ['dev', 'optional'], include: ['dev'] }, 'marked dev as included')
+    t.strictSame(obj, {
+      also: 'development',
+      omit: ['optional'],
+      include: ['dev'],
+    }, 'marked dev as included')
     t.strictSame(flat, { omit: ['optional'] }, 'removed dev from omit')
     t.end()
   })
@@ -237,7 +245,7 @@ t.test('flatteners that populate flat.omit array', t => {
     const flat = {}
     const obj = { only: 'asdf' }
     definitions.only.flatten('only', obj, flat)
-    t.strictSame(flat, {}, 'ignored if value is not production')
+    t.strictSame(flat, { omit: [] }, 'ignored if value is not production')
 
     obj.only = 'prod'
     definitions.only.flatten('only', obj, flat)
@@ -256,18 +264,30 @@ t.test('flatteners that populate flat.omit array', t => {
     const obj = { optional: null }
 
     definitions.optional.flatten('optional', obj, flat)
-    t.strictSame(obj, { optional: null }, 'do nothing by default')
-    t.strictSame(flat, {}, 'do nothing by default')
+    t.strictSame(obj, {
+      optional: null,
+      omit: [],
+      include: [],
+    }, 'do nothing by default')
+    t.strictSame(flat, { omit: [] }, 'do nothing by default')
 
     obj.optional = true
     definitions.optional.flatten('optional', obj, flat)
-    t.strictSame(obj, {include: ['optional'], optional: true}, 'include optional when set')
+    t.strictSame(obj, {
+      omit: [],
+      optional: true,
+      include: ['optional'],
+    }, 'include optional when set')
     t.strictSame(flat, {omit: []}, 'nothing to omit in flatOptions')
 
     delete obj.include
     obj.optional = false
     definitions.optional.flatten('optional', obj, flat)
-    t.strictSame(obj, {omit: ['optional'], optional: false}, 'omit optional when set false')
+    t.strictSame(obj, {
+      omit: ['optional'],
+      optional: false,
+      include: [],
+    }, 'omit optional when set false')
     t.strictSame(flat, {omit: ['optional']}, 'omit optional when set false')
 
     t.end()
@@ -277,25 +297,49 @@ t.test('flatteners that populate flat.omit array', t => {
     const flat = {}
     const obj = {production: true}
     definitions.production.flatten('production', obj, flat)
-    t.strictSame(obj, {production: true, omit: ['dev']}, '--production sets --omit=dev')
+    t.strictSame(obj, {
+      production: true,
+      omit: ['dev'],
+      include: [],
+    }, '--production sets --omit=dev')
     t.strictSame(flat, {omit: ['dev']}, '--production sets --omit=dev')
 
     delete obj.omit
     obj.production = false
     delete flat.omit
     definitions.production.flatten('production', obj, flat)
-    t.strictSame(obj, {production: false}, '--no-production has no effect')
-    t.strictSame(flat, {}, '--no-production has no effect')
+    t.strictSame(obj, {
+      production: false,
+      include: [],
+      omit: [],
+    }, '--no-production has no effect')
+    t.strictSame(flat, { omit: [] }, '--no-production has no effect')
 
     obj.production = true
     obj.include = ['dev']
     definitions.production.flatten('production', obj, flat)
-    t.strictSame(obj, {production: true, include: ['dev'], omit: ['dev']}, 'omit and include dev')
+    t.strictSame(obj, {
+      production: true,
+      include: ['dev'],
+      omit: [],
+    }, 'omit and include dev')
     t.strictSame(flat, {omit: []}, 'do not omit dev when included')
 
     t.end()
   })
 
+  t.test('dev', t => {
+    const flat = {}
+    const obj = {dev: true}
+    definitions.dev.flatten('dev', obj, flat)
+    t.strictSame(obj, {
+      dev: true,
+      omit: [],
+      include: ['dev'],
+    })
+    t.end()
+  })
+
   t.end()
 })