From a4df2b98d89429b19cd29b5fc895cdbfc0a6bd78 Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 24 Mar 2021 09:29:21 -0700 Subject: [PATCH] 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 --- docs/content/using-npm/config.md | 8 ++ lib/utils/config/definitions.js | 73 +++++++++++-------- ...ib-utils-config-definitions.js-TAP.test.js | 1 + ...b-utils-config-describe-all.js-TAP.test.js | 8 ++ test/lib/utils/config/definitions.js | 70 ++++++++++++++---- 5 files changed, 115 insertions(+), 45 deletions(-) diff --git a/docs/content/using-npm/config.md b/docs/content/using-npm/config.md index da69ad4632530..6bbe58d91ef42 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 5001efb461b9c..b8e021c5212bf 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 2ed810da8a284..b3d920a0ca284 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 8323e793e075f..8af8c1edd3e52 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 830468c4391ac..75819dcbc671e 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() })