Skip to content
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.

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

fix(npm): Fix NPM_TOKEN usage again #143

Merged
merged 4 commits into from
Dec 3, 2020
Merged

fix(npm): Fix NPM_TOKEN usage again #143

merged 4 commits into from
Dec 3, 2020

Conversation

BYK
Copy link
Member

@BYK BYK commented Dec 2, 2020

This is a retake on #130. Although npm/cli#8 claims to have support
for npm_config_//registry.npmjs.org/:_authToken= usage, my tests
and the reports on the internet says this still doesn't work, even
with the latest npm (7.0.15 at the time).

The only way to pass the token is to have the authToken line in
an .npmrc file. The quick&dirty way would have been to create one
in the project directory but that may collide with a potentially
pre-existing project .npmrc. Trying to merge these seems more
trouble than it is worth:
https://github.com/actions/setup-node/blob/59e61b89511ed136a0b17773f07c349fa5c01e8b/src/authutil.ts
(even worse as you'd need to revert these changes after the fact)

The "better" solution I found is:

  1. Create a temporary file as your npmrc
  2. Put the token/registry line there
  3. Tell npm to use that file as the user config
  4. Use the npm_config_userconfig for the above to support yarn too

@BYK BYK requested review from jan-auer, tonyo and HazAT December 2, 2020 15:09
This is a retake on #130. Although npm/cli#8 claims to have support
for `npm_config_//registry.npmjs.org/:_authToken=` usage, my tests
and the reports on the internet says this still doesn't work, even
with the latest npm (7.0.15 at the time).

The only way to pass the token is to have the `authToken` line in
an `.npmrc` file. The quick&dirty way would have been to create one
in the project directory but that may collide with a potentially
pre-existing project `.npmrc`. Trying to merge these seems more
trouble than it is worth:
https://github.com/actions/setup-node/blob/59e61b89511ed136a0b17773f07c349fa5c01e8b/src/authutil.ts
(even worse as you'd need to revert these changes after the fact)

The "better" solution I found is:

1. Create a temporary file as your npmrc
2. Put the token/registry line there
3. Tell npm to use that file as the user config
4. Use the `npm_config_userconfig` for the above to support yarn too

This may still fail for yarn, see yarnpkg/yarn#4568.
@BYK
Copy link
Member Author

BYK commented Dec 2, 2020

Okay, just manually verified that this works quite well 🥳

Copy link
Contributor

@tonyo tonyo left a comment

Choose a reason for hiding this comment

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

Sweeeeet

@BYK BYK merged commit c6e3dbc into master Dec 3, 2020
@BYK BYK deleted the byk/fix/npm-token branch December 3, 2020 09:31
BYK added a commit that referenced this pull request Dec 15, 2020
BYK added a commit that referenced this pull request Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants