This repository has been archived by the owner. It is now read-only.

Do replace environment in config files (like .npmrc) even for keys #18426

Merged
merged 1 commit into from Mar 8, 2018

Conversation

Projects
None yet
3 participants
@misak113
Contributor

misak113 commented Sep 6, 2017

Current .npmrc will keep keys with not replaced Environment variables. However values are replaced. So it may confuse somebody & has no option to solve this problem

.npmrc example

registry=${NPM_REGISTRY_URL}
//${NPM_REGISTRY_HOST}/:_authToken=${NPM_AUTH_TOKEN}
always-auth=true

With env.vars:

export NPM_REGISTRY_URL=http://my.registry.com/
export NPM_REGISTRY_HOST=my.registry.com
export NPM_AUTH_TOKEN=xxxxxxxxxxxxxxx

NOW it will produce bellow options:

registry=http://my.registry.com/
//${NPM_REGISTRY_HOST}/:_authToken=xxxxxxxxxxxxxxx
always-auth=true

But expected options are:

registry=http://my.registry.com/
//my.registry.com/:_authToken=xxxxxxxxxxxxxxx
always-auth=true

Probably it is expected state & it would be very helpful when it will be merged for private registries which are using CI for publishing npm packages. For example: Then no more "npm login | answer_input $AUTH_TOKEN ... # etc. constructions" are necessary.

Do replace environment in config files even for keys
* as additional replacement for values in .npmrc for example

@misak113 misak113 requested a review from npm/cli-team as a code owner Sep 6, 2017

@misak113

This comment has been minimized.

Show comment
Hide comment
@misak113

misak113 Sep 11, 2017

Contributor

Did anyone get this PR? Just asking. It's my first contribution here.

Contributor

misak113 commented Sep 11, 2017

Did anyone get this PR? Just asking. It's my first contribution here.

@iarna

This comment has been minimized.

Show comment
Hide comment
@iarna

iarna Sep 14, 2017

Member

@misak113, yup we've seen it, we just haven't had an opportunity to comment yet.

Member

iarna commented Sep 14, 2017

@misak113, yup we've seen it, we just haven't had an opportunity to comment yet.

@zkat

zkat approved these changes Mar 6, 2018

I talked with @iarna and while this is pretty darn weird, we think we should accept it because it can be a nice escape valve, and I'm having a hard time figuring out how this would cause huge problems that the current replacement doesn't already.

@zkat zkat changed the base branch from latest to release-next Mar 6, 2018

@zkat zkat added the target-latest label Mar 6, 2018

@zkat zkat merged commit e6b9c1c into npm:release-next Mar 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

zkat added a commit that referenced this pull request Mar 8, 2018

config: Do replace environment in config files even for keys (#18426)
 as additional replacement for values in .npmrc for example

PR-URL: #18426
Credit: @misak113
Reviewed-By: @zkat
Reviewed-By: @iarna

@dependencies dependencies bot referenced this pull request Mar 24, 2018

Merged

Update yarn.lock #40

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.