Allow token replacement of .npmrc configuration with env vars #1207

darthtrevino · 2016-10-18T21:03:10Z

Summary

NPM allows for clients to inject environment variables into .npmrc configuration files. e.g.

//registry.npmjs.org/:_authToken=${NPM_TOKEN}
registry=http://registry.npmjs.org

This commit allows yarn to understand .npmrc files using this feature.

Test plan

Build the yarn executable
execute yarn config list to view injected variables

Fixes #1206

…e way npm does

darthtrevino · 2016-10-18T21:10:00Z

holm · 2016-10-19T07:19:20Z

We use this to inject our NPM token into the nvmrc file, so we can avoid checking in the token, and also avoid having to deal with global npm configuration on machines.

…env_vars

samccone · 2016-10-25T21:30:21Z

Thanks for the PR @darthtrevino I will review

darthtrevino · 2016-10-25T21:30:39Z

Sweet, thank you sir!

wyze · 2016-10-25T21:44:20Z

src/registries/npm-registry.js

@@ -36,6 +36,24 @@ function getGlobalPrefix(): string {
  }
 }

+function envReplace(f: string): string {


Should be more descriptive variable here than f.

… 1206_npm_env_vars

darthtrevino · 2016-10-26T18:39:08Z

Word, thanks @wyze; in the last couple of commits I split out the envReplace function into a separate module in utils/ and added a few basic tests

maybeec · 2016-11-01T15:25:20Z

Any chance to get this in the next release? Would love to see this as it is annoying me in my project :)

sebmck · 2016-11-02T14:08:37Z

src/registries/npm-registry.js

@@ -118,6 +119,9 @@ export default class NpmRegistry extends Registry {

    for (const [, loc, file] of await this.getPossibleConfigLocations('.npmrc')) {
      const config = ini.parse(file);
+      Object.keys(config).forEach((key: string) => {


Use for...in here please.

sebmck · 2016-11-02T14:08:44Z

__tests__/util/env-replace.js

+import assert from 'assert';
+
+describe('environment variable replacement', () => {
+


Remove empty newline.

sebmck · 2016-11-02T14:09:13Z

src/util/env-replace.js

+    return value;
+  }
+
+  const envExpr = /(\\*)\$\{([^}]+)\}/g;


Put this at the top level and capitalise it as ENV_EXPR.

darthtrevino · 2016-11-02T17:22:12Z

~~The CircleCI issue looks like it's affecting newer PRs too. Let me know when I can pull a fix for it.~~

maybeec · 2016-11-03T10:33:37Z

Does this PR also include the ability to replace any env variable? NPM, e.g., does not replace env vars in keys like this:

@scope:registry=${ARTIFACTORY_REGISTRY}
//${ARTIFACTORY_REGISTRY}:_password=${PASSWORD}
//${ARTIFACTORY_REGISTRY}:username=${USERNAME}

So ${ARTIFACTORY_REGISTRY} is replaced in a value expression but not in a key expression, which is quite odd.

darthtrevino · 2016-11-03T17:46:18Z

@maybeec Nope, this change acts the same way that NPM does. It performs token replacement in value expressions

maybeec · 2016-11-04T06:31:49Z

@darthtrevino maybe not in near future: npm/npm#14513
So you might want to think of supporting this directly. 👍

But might be also in a separate PR by another guy if this is merged here. As you prefer.

darthtrevino · 2016-11-04T16:43:04Z

@maybeec When NPM changes, I'll gladly implement it here (provided this PR actually makes it in)

darthtrevino · 2016-11-04T16:54:03Z

@kittens The changes you requested have been implemented and the CI looks good now.

darthtrevino · 2016-11-04T18:25:15Z

PAkerstrand · 2016-11-09T16:41:54Z

Does this PR require further work/changes? Would love to see it merged...

darthtrevino · 2016-11-09T18:54:14Z

¯_(ツ)_/¯ - I've addressed the review issues; it's up to the yarn team now

darthtrevino · 2016-11-14T18:17:33Z

💃

Daniel15 and others added 2 commits October 10, 2016 22:40

test IGNORE ME

29e9ad4

Update npm-registry to inject env vars into npm configuration the sam…

2614029

…e way npm does

Add type annotation to config object iteration

d0e71d0

darthtrevino added 2 commits October 19, 2016 01:13

Merge branch 'master' into 1206_npm_env_vars

8470b43

Merge branch 'master' into 1206_npm_env_vars

36ebff6

darthtrevino changed the title ~~Update NPM-Registry Class to inject Env Vars into NPM Config~~ Allow token replacement of .npmrc configuration with env vars Oct 19, 2016

darthtrevino and others added 3 commits October 19, 2016 12:29

Merge branch 'master' into 1206_npm_env_vars

936b403

Merge remote-tracking branch 'origin/Daniel15-patch-1' into 1206_npm_…

85e6c44

…env_vars

Merge branch 'master' into 1206_npm_env_vars

8edfb68

samccone self-assigned this Oct 25, 2016

wyze reviewed Oct 25, 2016

View reviewed changes

darthtrevino added 4 commits October 25, 2016 14:57

Update envReplace function

123ddaa

Merge branch '1206_npm_env_vars' of github.com:darthtrevino/yarn into…

b3bd853

… 1206_npm_env_vars

Move env-replace function to a separate util module. Add unit tests

2edadcc

Correct the env-replace tests

89e5729

samccone approved these changes Nov 1, 2016

View reviewed changes

sebmck suggested changes Nov 2, 2016

View reviewed changes

darthtrevino added 2 commits November 2, 2016 09:32

Updates per @kittens' comments

c452e21

Merge in master branch

fc30d3a

darthtrevino closed this Nov 2, 2016

darthtrevino reopened this Nov 2, 2016

Merge branch 'master' into 1206_npm_env_vars

77042e5

Merge branch 'master' into 1206_npm_env_vars

2e175a0

sebmck merged commit b4e42e3 into yarnpkg:master Nov 14, 2016

darthtrevino deleted the 1206_npm_env_vars branch November 14, 2016 18:17

LittleColin mentioned this pull request Jul 18, 2018

Can a custom npm registry be used with brigade.json? brigadecore/brigade#558

Closed

stoically mentioned this pull request Mar 20, 2019

HTTPError: Response code 401 (Unauthorized) on private registry raineorshine/npm-check-updates#519

Closed

2 tasks

robertknight mentioned this pull request Nov 6, 2020

Remove npm dependency hypothesis/client#2670

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow token replacement of .npmrc configuration with env vars #1207

Allow token replacement of .npmrc configuration with env vars #1207

darthtrevino commented Oct 18, 2016 •

edited

Loading

darthtrevino commented Oct 18, 2016

holm commented Oct 19, 2016

samccone commented Oct 25, 2016

darthtrevino commented Oct 25, 2016

wyze Oct 25, 2016

darthtrevino commented Oct 26, 2016

maybeec commented Nov 1, 2016

sebmck Nov 2, 2016

sebmck Nov 2, 2016

sebmck Nov 2, 2016

darthtrevino commented Nov 2, 2016 •

edited

Loading

maybeec commented Nov 3, 2016

darthtrevino commented Nov 3, 2016

maybeec commented Nov 4, 2016 •

edited

Loading

darthtrevino commented Nov 4, 2016

darthtrevino commented Nov 4, 2016

darthtrevino commented Nov 4, 2016

PAkerstrand commented Nov 9, 2016

darthtrevino commented Nov 9, 2016 •

edited

Loading

darthtrevino commented Nov 14, 2016

		import assert from 'assert';

		describe('environment variable replacement', () => {

Allow token replacement of .npmrc configuration with env vars #1207

Allow token replacement of .npmrc configuration with env vars #1207

Conversation

darthtrevino commented Oct 18, 2016 • edited Loading

darthtrevino commented Oct 18, 2016

holm commented Oct 19, 2016

samccone commented Oct 25, 2016

darthtrevino commented Oct 25, 2016

wyze Oct 25, 2016

Choose a reason for hiding this comment

darthtrevino commented Oct 26, 2016

maybeec commented Nov 1, 2016

sebmck Nov 2, 2016

Choose a reason for hiding this comment

sebmck Nov 2, 2016

Choose a reason for hiding this comment

sebmck Nov 2, 2016

Choose a reason for hiding this comment

darthtrevino commented Nov 2, 2016 • edited Loading

maybeec commented Nov 3, 2016

darthtrevino commented Nov 3, 2016

maybeec commented Nov 4, 2016 • edited Loading

darthtrevino commented Nov 4, 2016

darthtrevino commented Nov 4, 2016

darthtrevino commented Nov 4, 2016

PAkerstrand commented Nov 9, 2016

darthtrevino commented Nov 9, 2016 • edited Loading

darthtrevino commented Nov 14, 2016

darthtrevino commented Oct 18, 2016 •

edited

Loading

darthtrevino commented Nov 2, 2016 •

edited

Loading

maybeec commented Nov 4, 2016 •

edited

Loading

darthtrevino commented Nov 9, 2016 •

edited

Loading