Skip to content

Commit

Permalink
Add a 'envExport' flag for config definitions
Browse files Browse the repository at this point in the history
It defaults to true, but setting it to any falsey value will tell
`@npmcli/config` not to export it into the environment, and will also
put a remark in the documentation that it is not exported.

PR-URL: #3014
Credit: @isaacs
Close: #3014
Reviewed-by: @nlf
  • Loading branch information
isaacs committed Apr 1, 2021
1 parent 61da39b commit 7b177e4
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 1 deletion.
4 changes: 4 additions & 0 deletions docs/content/using-npm/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,8 @@ Valid values for the `workspace` config are either: - Workspace names - Path
to a workspace directory - Path to a parent workspace directory (will result
to selecting all of the nested workspaces)

This value is not exported to the environment for child processes.

#### `workspaces`

* Default: false
Expand All @@ -1341,6 +1343,8 @@ to selecting all of the nested workspaces)
Enable running a command in the context of **all** the configured
workspaces.

This value is not exported to the environment for child processes.

#### `yes`

* Default: null
Expand Down
8 changes: 7 additions & 1 deletion lib/utils/config/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const allowed = [
'type',
'typeDescription',
'usage',
'envExport',
]

const {
Expand All @@ -39,6 +40,8 @@ const {
class Definition {
constructor (key, def) {
this.key = key
// if it's set falsey, don't export it, otherwise we do by default
this.envExport = true
Object.assign(this, def)
this.validate()
if (!this.defaultDescription)
Expand Down Expand Up @@ -68,6 +71,9 @@ class Definition {
// a textual description of this config, suitable for help output
describe () {
const description = unindent(this.description)
const noEnvExport = this.envExport ? '' : `
This value is not exported to the environment for child processes.
`
const deprecated = !this.deprecated ? ''
: `* DEPRECATED: ${unindent(this.deprecated)}\n`
return wrapAll(`#### \`${this.key}\`
Expand All @@ -76,7 +82,7 @@ class Definition {
* Type: ${unindent(this.typeDescription)}
${deprecated}
${description}
`)
${noEnvExport}`)
}
}

Expand Down
2 changes: 2 additions & 0 deletions lib/utils/config/definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -2044,6 +2044,7 @@ define('workspace', {
type: [String, Array],
hint: '<workspace-name>',
short: 'w',
envExport: false,
description: `
Enable running a command in the context of the configured workspaces of the
current project while filtering by running only the workspaces defined by
Expand All @@ -2061,6 +2062,7 @@ define('workspaces', {
default: false,
type: Boolean,
short: 'ws',
envExport: false,
description: `
Enable running a command in the context of **all** the configured
workspaces.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,8 @@ Valid values for the \`workspace\` config are either: - Workspace names - Path
to a workspace directory - Path to a parent workspace directory (will result
to selecting all of the nested workspaces)
This value is not exported to the environment for child processes.
#### \`workspaces\`
* Default: false
Expand All @@ -1220,6 +1222,8 @@ to selecting all of the nested workspaces)
Enable running a command in the context of **all** the configured
workspaces.
This value is not exported to the environment for child processes.
#### \`yes\`
* Default: null
Expand Down
20 changes: 20 additions & 0 deletions test/lib/utils/config/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ t.test('basic definition', async t => {
usage: '--key <key>',
typeDescription: 'Number or String',
description: 'just a test thingie',
envExport: true,
})
t.matchSnapshot(def.describe(), 'human-readable description')

Expand Down Expand Up @@ -119,6 +120,25 @@ t.test('basic definition', async t => {
description: 'asdf',
})
t.equal(optionalBool.usage, '--key')

const noExported = new Definition('methane', {
envExport: false,
type: String,
typeDescription: 'Greenhouse Gas',
default: 'CH4',
description: `
This is bad for the environment, for our children, do not put it there.
`,
})
t.equal(noExported.envExport, false, 'envExport flag is false')
t.equal(noExported.describe(), `#### \`methane\`
* Default: "CH4"
* Type: Greenhouse Gas
This is bad for the environment, for our children, do not put it there.
This value is not exported to the environment for child processes.`)
})

t.test('missing fields', async t => {
Expand Down

0 comments on commit 7b177e4

Please sign in to comment.