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: don't fail if .npmrc exists in project root #161

Closed
wants to merge 2 commits into from

Conversation

h3rmanj
Copy link

@h3rmanj h3rmanj commented Jan 31, 2024

Currently, changesets-gitlab will only check the the ${HOME}/.npmrc path, or use the NPM_TOKEN variable assuming its for the public npm registry. While it's showed in the example in the README, it's not explicitly mentioned.

A common pattern used in GitLab CI/CD examples, is to append auth to the project roots .npmrc file. When I set up the pipeline like I normally would, I was surprised to see it output that no .npmrc file was found. I'd imagine someone migrating from an old setup or following what they know from multiple guides could run into this as well.

Copy link

changeset-bot bot commented Jan 31, 2024

🦋 Changeset detected

Latest commit: 52c263e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
changesets-gitlab Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@JounQin
Copy link
Member

JounQin commented Jan 31, 2024

see also #78 (comment)

@h3rmanj
Copy link
Author

h3rmanj commented Jan 31, 2024

Ah, I'm sorry I didn't see that. It was a little confusing running into though.

Setting a dummy NPM_TOKEN seems like a workaround though, and knowing the implementation actually uses it to set auth for only the default npm registry globally seems like it shouldn't be used.
Modifying a .npmrc outside of the project on shared job runners feels a little weird. It should perhaps be more explicitly stated in the README if this is the intended way forward.

I agree it would be breaking, but changesets-gitlab is still in semver 0.x.

An alternative implementation could perhaps leverage npm whoami or similar to determine if there is a valid login, which would check both project and home for auth specifically. But then changesets-gitlab would need to know the registry that's needed as well.

@JounQin
Copy link
Member

JounQin commented Jan 31, 2024

I think a new environment variable like PREFER_PROJECT_LEVEL_NPMRC can be added so no breaking change required.

@h3rmanj
Copy link
Author

h3rmanj commented Mar 11, 2024

Sorry for leaving this open so long. Maybe we should introduce a variable like NPMRC_AUTH_DIR that defaults to HOME, and people can set to CI_PROJECT_DIR if they want? I feel that might be more explicit, and make it more configurable if someone would want their npmrc in another location aswell.

@JounQin
Copy link
Member

JounQin commented Mar 11, 2024

Is there any usage case for NPMRC_AUTH_DIR? Configurable is not always correct.

@h3rmanj
Copy link
Author

h3rmanj commented Mar 11, 2024

Not that I know of no, other than the publish-command works with an .npmrc with auth in any parent directory.

To be honest though I'm not sure why this cli-tool even checks and fails if there is no .npmrc in a specific directory. It's not a guarantee of auth, and it can exist in any parent directory above the dir npm publish is run in. Auth is expected when publishing, and npm publish will fail with an appropriate message if it doesn't have what it needs as well. Removing the check should at least be considered.

I can try to make an MR with PREFER_PROJECT_LEVEL_NPMRC if that's the only other option you would accept, as it is still better than the NPM_TOKEN workaround.

Edit: Seems like I'm misremembering npm walking up parent dirs checking for .npmrc. But the docs still states it can reside in 4 different locations

@JounQin
Copy link
Member

JounQin commented Mar 11, 2024

It's not a guarantee of auth, and it can exist in any parent directory above the dir npm publish is run in.

Edit: Seems like I'm misremembering npm walking up parent dirs checking for .npmrc. But the docs still states it can reside in 4 different locations

I'm not quite sure for this feature, I'd like to keep this tool as simple as possible, and sync with upstream. Custom auth directories may be great but I'd not recommend to use non-standard .npmrc files. I cannot imagine when should we use other .npmrc locations in such a simple tool.

So, I still think PREFER_PROJECT_LEVEL_NPMRC could be acceptable for trading-off.

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.

None yet

2 participants