un-ts / changesets-gitlab

GitLab CI cli for changesets like its GitHub Action.
https://opencollective.com/unts/projects/changesets-gitlab
MIT License
86 stars 33 forks source link

fix: don't fail if .npmrc exists in project root #161

Closed h3rmanj closed 6 months ago

h3rmanj commented 7 months ago

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.

changeset-bot[bot] commented 7 months ago

🦋 Changeset detected

Latest commit: 52c263ee30e07f1efb07d02813f3abb66dbd424f

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 commented 7 months ago

see also https://github.com/un-ts/changesets-gitlab/pull/78#issuecomment-1776524955

h3rmanj commented 7 months ago

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 commented 7 months ago

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

h3rmanj commented 6 months ago

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 commented 6 months ago

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

h3rmanj commented 6 months ago

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 commented 6 months ago

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.