Skip to content

Commit

Permalink
config: Restore --dev flag, unify --omit flatteners
Browse files Browse the repository at this point in the history
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: #2942
Credit: @isaacs
Close: #2942
Reviewed-by: @wraithgar
  • Loading branch information
isaacs authored and wraithgar committed Mar 24, 2021
1 parent 62b783a commit a4df2b9
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 45 deletions.
8 changes: 8 additions & 0 deletions docs/content/using-npm/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""
Expand Down
73 changes: 41 additions & 32 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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)
},
})
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -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)
},
})

Expand All @@ -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)
},
})
Expand All @@ -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)
},
})
Expand Down Expand Up @@ -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)
},
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Array [
"commit-hooks",
"depth",
"description",
"dev",
"diff",
"diff-ignore-all-space",
"diff-name-only",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: ""
Expand Down
70 changes: 57 additions & 13 deletions test/lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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()
})

Expand Down

0 comments on commit a4df2b9

Please sign in to comment.