Skip to content

feat: add support for optional env var replacements in .npmrc #8359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from
+15 −4
Open

feat: add support for optional env var replacements in .npmrc #8359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/lib/content/configuring-npm/npmrc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@ The four relevant files are:
* npm builtin config file (`/path/to/npm/npmrc`)

All npm config files are an ini-formatted list of `key = value` parameters.
Environment variables can be replaced using `${VARIABLE_NAME}`. For
Environment variables can be replaced using `${VARIABLE_NAME}`. By default
if the variable is not defined, it is left unreplaced. By adding `?` after
variable name they can be forced to evaluate to an empty string instead.For
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
variable name they can be forced to evaluate to an empty string instead.For
variable name they can be forced to evaluate to an empty string instead. For

example:

```bash
cache = ${HOME}/.npm-packages
node-options = "${NODE_OPTIONS?} --use-system-ca"
```

Each of these files is loaded, and config options are resolved in priority
Expand Down
8 changes: 5 additions & 3 deletions workspaces/config/lib/env-replace.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// replace any ${ENV} values with the appropriate environ.
// optional "?" modifier can be used like this: ${ENV?} so in case of the variable being not defined, it evaluates into empty string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// optional "?" modifier can be used like this: ${ENV?} so in case of the variable being not defined, it evaluates into empty string
// optional "?" modifier can be used like this: ${ENV?} so in case of the variable being not defined, it evaluates into empty string.


const envExpr = /(?<!\\)(\\*)\$\{([^${}]+)\}/g
const envExpr = /(?<!\\)(\\*)\$\{([^${}?]+)(\?)?\}/g

module.exports = (f, env) => f.replace(envExpr, (orig, esc, name) => {
const val = env[name] !== undefined ? env[name] : `$\{${name}}`
module.exports = (f, env) => f.replace(envExpr, (orig, esc, name, modifier) => {
const fallback = modifier === '?' ? '' : `$\{${name}}`
const val = env[name] !== undefined ? env[name] : fallback

// consume the escape chars that are relevant.
if (esc.length % 2) {
Expand Down
6 changes: 6 additions & 0 deletions workspaces/config/test/env-replace.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ const env = {

t.equal(envReplace('\\${foo}', env), '${foo}')
t.equal(envReplace('\\\\${foo}', env), '\\bar')
t.equal(envReplace('\\\\\\${foo}', env), '\\${foo}')
t.equal(envReplace('${baz}', env), '${baz}')
t.equal(envReplace('\\${baz}', env), '${baz}')
t.equal(envReplace('\\\\${baz}', env), '\\${baz}')
t.equal(envReplace('\\${foo?}', env), '${foo?}')
t.equal(envReplace('\\\\${foo?}', env), '\\bar')
t.equal(envReplace('${baz?}', env), '')
t.equal(envReplace('\\${baz?}', env), '${baz?}')
t.equal(envReplace('\\\\${baz?}', env), '\\')
Comment on lines 9 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion for a couple more tests here - and what do you think about adding a description, I find it way easier to read:

Suggested change
t.equal(envReplace('\\${foo}', env), '${foo}')
t.equal(envReplace('\\\\${foo}', env), '\\bar')
t.equal(envReplace('\\\\\\${foo}', env), '\\${foo}')
t.equal(envReplace('${baz}', env), '${baz}')
t.equal(envReplace('\\${baz}', env), '${baz}')
t.equal(envReplace('\\\\${baz}', env), '\\${baz}')
t.equal(envReplace('\\${foo?}', env), '${foo?}')
t.equal(envReplace('\\\\${foo?}', env), '\\bar')
t.equal(envReplace('${baz?}', env), '')
t.equal(envReplace('\\${baz?}', env), '${baz?}')
t.equal(envReplace('\\\\${baz?}', env), '\\')
t.equal(envReplace('${foo}', env), 'bar', 'replaces defined variable')
t.equal(envReplace('${foo?}', env), 'bar', 'replaces defined variable with ? modifier')
t.equal(envReplace('${foo}${bar}', env), 'barbaz', 'replaces multiple defined variables')
t.equal(envReplace('${foo?}${baz?}', env), 'bar', 'replaces mixed defined/undefined variables with ? modifier')
t.equal(envReplace('\\${foo}', env), '${foo}', 'escapes normal variable')
t.equal(envReplace('\\\\${foo}', env), '\\bar', 'double escape allows replacement')
t.equal(envReplace('\\\\\\${foo}', env), '\\${foo}', 'triple escape prevents replacement')
t.equal(envReplace('${baz}', env), '${baz}', 'leaves undefined variable unreplaced')
t.equal(envReplace('\\${baz}', env), '${baz}', 'escapes undefined variable')
t.equal(envReplace('\\\\${baz}', env), '\\${baz}', 'double escape with undefined variable')
t.equal(envReplace('\\${foo?}', env), '${foo?}', 'escapes optional variable')
t.equal(envReplace('\\\\${foo?}', env), '\\bar', 'double escape allows optional replacement')
t.equal(envReplace('${baz?}', env), '', 'replaces undefined variable with empty string when using ? modifier')
t.equal(envReplace('\\${baz?}', env), '${baz?}', 'escapes undefined optional variable')
t.equal(envReplace('\\\\${baz?}', env), '\\', 'double escape with undefined optional variable results in empty replacement')